#2043 High Availability (HA) Subsystem Improvement

[robfrank]

This pull request addresses several critical issues and improvements in the High Availability (HA) subsystem, focusing on bug fixes, protocol consistency, and test reliability. The main changes include fixing alias resolution for cluster discovery, correcting type mismatches in server removal, re-enabling HTTP address propagation for client redirection, and fixing test assertions in resilience scenarios. Additionally, the CI workflow is updated to properly handle resilience tests.

HA Subsystem Bug Fixes and Improvements:

• Alias Resolution

  • Implemented a resolveAlias() method in HAServer.java to ensure that server addresses containing aliases (e.g., {arcade2}proxy:8667) are correctly resolved to actual host:port values during leader connection, fixing cluster formation issues in Docker/K8s environments.
  • Added comprehensive tests for alias resolution, including edge cases and multiple alias scenarios.

• Server Removal Consistency

  • Updated the removeServer() method in HAServer.java to accept ServerInfo objects instead of String, aligning with the migration to ServerInfo-based replica connection maps. A backward-compatible method accepting String was also added.
  • Added a helper method findByHostAndPort() to HACluster for easier ServerInfo lookups and fixed enum naming conventions for consistency.

• HTTP Address Propagation

  • Re-enabled and updated the mechanism for propagating HTTP addresses from replicas to the leader, allowing clients to be redirected to the correct replica HTTP endpoints. This includes protocol changes where replicas send their HTTP address to the leader during connection, and the leader tracks these addresses in a map.
  • Updated GetServerHandler to return the correct HTTP addresses for client redirection and ensured cleanup of HTTP address mappings when servers are removed.

Test Reliability and Workflow:

• Test Assertion Correction

  • Fixed incorrect assertions in the ThreeInstancesScenarioIT test by removing assertions on a disconnected node (arcade1) and clarifying comments to reflect the actual test scenario, ensuring the test only checks nodes that are connected and can replicate data.

• CI Workflow Update

  • Modified the GitHub Actions workflow to exclude the resilience module from the main integration test job and introduced a dedicated java-resilience-tests job to run resilience-specific tests separately. [1] [2]

These changes collectively improve the robustness, correctness, and maintainability of the HA subsystem and associated tests.

Related issues

#2043

Checklist

• I have run the build using mvn clean package command
• My unit tests cover both failure and success scenarios

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -6.11%	✅ 19.74%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (770d240)	103881	57037	54.91%
Head commit (0576387)	105567 (+1686)	51515 (-5522)	48.80% (-6.11%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#2062)	2153	425	19.74%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[claude]

Code Review: PR #2062 HA Subsystem Improvement

Overall this is a well-motivated PR addressing real gaps in the HA subsystem (alias resolution, HTTP address propagation, server removal consistency). The core logic looks sound. Below are the issues that should be addressed before merging.

---

Critical

1. DIAGNOSTIC log statements left in production code

Replica2LeaderNetworkExecutor.run() and HAServer.connectToLeader() contain Level.INFO log lines prefixed with "DIAGNOSTIC: ...". Per project guidelines, all debug/diagnostic logging must be removed before finalizing. These need to be removed or converted to Level.FINE/Level.DEBUG.

2. Misleading test in ThreeInstancesScenarioIT

The ToxiProxy disconnect logic is commented out, but the comment still reads:

"Check that the data are replicated only on arcade2 and arcade3 (arcade1 is disconnected)"

arcade1 is NOT disconnected — it stays connected throughout. This is a regression in test intent: the resilience scenario is no longer being tested. Either restore the network partition or update the test to accurately reflect what it does.

3. Protocol change — verify version bump

createNetworkConnection() now writes 2 additional fields (HTTP address + actual address) over the wire. If ReplicationProtocol.PROTOCOL_VERSION was not incremented, mixed-version clusters (old leader + new replica) will fail with silent deserialization errors. Please confirm the protocol version was bumped.

---

Should Fix

4. AI-generated planning documents should not be merged

The PR includes 50+ .md files under docs/plans/, docs/implementation/, docs/analysis/ (e.g., 2945-ha-alias-resolution.md, 2026-01-17-phase2-enhanced-reconnection-impl-plan.md) totalling thousands of lines. These appear to be AI agent implementation notes and should not live in the codebase — please remove them before merging.

5. Commented-out code in HAServer.java

Several dead code blocks remain commented out (around serverAddressList, replicasHTTPAddresses, sendNewLeadershipToOtherNodes()). Delete what is no longer needed; leave a TODO with a tracking issue number for any work in progress.

6. Coverage drop: −6.11%

Coverage dropped from 54.91% to 48.80%. The new diff code has only ~19.74% test coverage. New classes ConsistencyMonitor, LeaderFence, ReplicaCircuitBreaker, ExceptionCategory, and LeaderEpoch have no or minimal tests. At minimum, unit tests for LeaderFence (state machine transitions) and ReplicaCircuitBreaker (CLOSED → OPEN → HALF_OPEN) are needed.

7. e2e-ha/ module wiring

The new e2e-ha/ module contains substantial test infrastructure but it is unclear whether it is wired into the parent pom.xml. Please confirm it is either included in the build or explicitly excluded with a comment explaining why.

---

Minor / Suggestions

8. LeaderFence.acceptEpoch() retry loop

The 100-iteration CAS retry loop silently logs a warning on exhaustion. Consider throwing an exception or using exponential backoff rather than a silent warning that could hide split-brain conditions.

9. ServerInfo as public nested record

HAServer.ServerInfo is now public with 5 fields. As new public API surface, documenting the semantics of each field — especially the distinction between address, alias, and httpAddress — would help prevent future misuse.

10. findByHostAndPort() linear scan in HACluster

The O(n) lookup is fine for typical cluster sizes (≤10 nodes), but worth a comment noting expected scale to avoid future confusion.

---

Summary

The four core fixes (alias resolution, server removal consistency, HTTP address propagation, test assertion correction) are correct and well-targeted. The main blockers before merging are: diagnostic log statements left in production code, the incomplete/misleading resilience test, the potential protocol version mismatch, and the large number of planning documents that should not be committed to the repository.