8000 [automatic failover] Make MultiDbConfig.DatabaseConfig.Builder agnostic to call order by atakavci · Pull Request #4456 · redis/jedis · GitHub
[go: up one dir, main page]

Skip to content

[automatic failover] Make MultiDbConfig.DatabaseConfig.Builder agnostic to call order#4456

Merged
atakavci merged 5 commits intoredis:masterfrom
atakavci:failover/healthCheckEnabled
Mar 11, 2026
Merged

[automatic failover] Make MultiDbConfig.DatabaseConfig.Builder agnostic to call order#4456
atakavci merged 5 commits intoredis:masterfrom
atakavci:failover/healthCheckEnabled

Conversation

@atakavci
Copy link
Contributor
@atakavci atakavci commented Mar 3, 2026

When configuring A-A failover, the order in which healthCheckEnabled and healthCheckStrategy were called affected the outcome — healthCheckEnabled(false) had no effect if set first.
Fixed by ensuring healthCheckEnabled always takes explicit priority regardless of call order.


Note

Medium Risk
Touches health-check enablement logic used by automatic failover/failback; misconfiguration could change whether background health checks run or which strategy is used.

Overview
Makes MultiDbConfig.DatabaseConfig.Builder call-order agnostic for health checks. A new healthCheckEnabled flag (default true) now gates getHealthCheckStrategySupplier() (returns null when disabled), while healthCheckStrategySupplier consistently defaults to PingStrategy.DEFAULT and is no longer nulled out by healthCheckEnabled(false).

Removes the DatabaseConfig constructor overload that accepted a pool config (tests now build configs via the builder), and updates integration tests to explicitly disable health checks using the builder where needed (e.g., AutomaticFailoverTest).

Written by Cursor Bugbot for commit 915421b. This will update automatically on new commits. Configure here.

@atakavci atakavci requested review from a-TODO-rov and ggivo March 3, 2026 08:51
@atakavci atakavci self-assigned this Mar 3, 2026
@atakavci atakavci added the techdebt Things that can be improved or refactored label Mar 3, 2026
@atakavci atakavci changed the title [automatic failover] Make MultiDbConfig.Builder agnostic to call order [automatic failover] Make MultiDbConfig.DatabaseConfig.Builder agnostic to call order Mar 3, 2026
Copy link
@cursor cursor bot left a comment

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.

ggivo
ggivo previously approved these changes Mar 5, 2026
Copy link
Collaborator
@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

LGTM,
need to adress failing tests

@ggivo ggivo dismissed their stale review March 5, 2026 15:48

need to address failing tests

Copy link
Collaborator
@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

LGTM,

Note: As discussed offline, removing a construcor is a breakign change but, sicne this is still experimental feature we are not marking it as such

@ggivo ggivo added this to the 7.4.0 milestone Mar 11, 2026
@atakavci atakavci merged commit c37b259 into redis:master Mar 11, 2026
14 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

techdebt Things that can be improved or refactored

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0