Skip to content

Enable command retries for RedisClient and RedisSentinelClient by default - #4490

Open
uglide wants to merge 9 commits into
masterfrom
im/retries-by-default
Open

Enable command retries for RedisClient and RedisSentinelClient by default#4490
uglide wants to merge 9 commits into
masterfrom
im/retries-by-default

Conversation

@uglide

@uglide uglide commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Add retries to DefaultCommandExecutor and consolidate all client classes to support command retries by default.


Note

High Risk
Default retry behavior and exception types change for all standalone/sentinel clients; UnifiedJedis is no longer directly instantiable, which is a breaking migration for custom construction paths.

Overview
Standalone and sentinel clients now retry transient connection failures by default (5 attempts, time budget socketTimeout × maxAttempts), matching cluster behavior. DefaultCommandExecutor implements the retry loop; shared ResilientCommandExecutor centralizes backoff with ClusterCommandExecutor. maxAttempts / maxTotalRetriesDuration move to AbstractClientBuilder for all RedisClient* builders; exhausted retries throw JedisException (with suppressed JedisConnectionException) instead of failing fast on the first blip.

API tightening: UnifiedJedis is abstract; DEFAULT_TIMEOUT and DEFAULT_MAX_ATTEMPTS live on UnifiedJedis (removed from RedisClusterClient). The UnifiedJedis(provider, maxAttempts, duration) constructor is removed; use builders or DefaultCommandExecutor. RetryableCommandExecutor is deprecated in favor of DefaultCommandExecutor.

The v7→v8 migration guide documents opt-out via maxAttempts(1) and exception-handling updates. Tests and examples were updated accordingly.

Reviewed by Cursor Bugbot for commit b4bab31. Bugbot is set up for automated code reviews on this repo. Configure here.

@uglide
uglide requested a review from ggivo April 16, 2026 11:31
@uglide uglide added the breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes. label Apr 16, 2026
@uglide uglide added this to the 8.0.0 milestone Apr 16, 2026
@uglide
uglide force-pushed the im/retries-by-default branch from 077aa3c to a38f383 Compare April 16, 2026 11:34
@github-actions

github-actions Bot commented Apr 16, 2026

Copy link
Copy Markdown

Test Results

0 files   -   203  0 suites   - 203   0s ⏱️ - 9m 52s
0 tests  - 7 767  0 ✅  - 7 705  0 💤  - 62  0 ❌ ±0 
0 runs   - 7 787  0 ✅  - 7 725  0 💤  - 62  0 ❌ ±0 

Results for commit b4bab31. ± Comparison against base commit 53da4a4.

♻️ This comment has been updated with latest results.

@jit-ci

jit-ci Bot commented Apr 16, 2026

Copy link
Copy Markdown

🛡️ Jit Security Scan Results

CRITICAL HIGH MEDIUM

✅ No security findings were detected in this PR


Security scan by Jit

@uglide
uglide force-pushed the im/retries-by-default branch from a38f383 to a149603 Compare April 16, 2026 11:39
@uglide
uglide requested review from atakavci and tishun April 16, 2026 11:40
Comment thread src/test/java/redis/clients/jedis/scenario/ConnectionInterruptionIT.java Outdated
@uglide
uglide force-pushed the im/retries-by-default branch from 40348e3 to 2648aad Compare April 21, 2026 15:18
@uglide
uglide force-pushed the im/retries-by-default branch from c460594 to 76ac06c Compare May 28, 2026 09:32

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b4bab31. Configure here.

} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new JedisException(e);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cluster sleep interrupt now throws wrong exception type

Medium Severity

The sleep method in ResilientCommandExecutor throws JedisException on interrupt, but the old ClusterCommandExecutor.sleep threw JedisClusterOperationException. Since ClusterCommandExecutor now inherits this sleep, any code catching JedisClusterOperationException specifically (including internal retry logic or user code) will miss the interrupt-triggered exception. This is a behavioral regression for cluster clients experiencing thread interruption during backoff.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b4bab31. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breakingchange Pull request that has breaking changes. Must include the breaking behavior in release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant