Skip to content

feat: multi-server seed list — startup + runtime failover (issue #12) - #16

Open
rigazilla wants to merge 3 commits into
feat/hotrod-urifrom
feat/multi-seed
Open

rigazilla wants to merge 3 commits into
feat/hotrod-urifrom
feat/multi-seed

Conversation

@rigazilla

@rigazilla rigazilla commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Closes #12. Stacked on #11 (feat/hotrod-uri) — review/merge that first.

What

fromUri already parsed all comma-separated hosts but used only the first as the seed. This makes all hosts seed servers with two failover tiers, mirroring the Java client's immutable ClusterInfo.getInitialServers() fallback.

  • Startup failover — connect() tries each seed in order (reusing the pre-built bootstrap connection for seeds_[0]); if all are unreachable it throws one HotRodClientException whose message lists every seed tried.
  • Runtime failover — new tryConnectSeeds() helper wired into the fall-through points of selectAnyServer and selectServerForKey (including the old return connection_ no-owners/empty path). It prefers the live bootstrap connection_ (fast path → happy path and BASIC mode do no new I/O), else walks the seeds via the existing pure orderKeyCandidates, adopting the first reachable seed as the new bootstrap connection.

Design decisions (see docs/DECISIONS.md 2026-10-19 for full rationale + Java comparison)

  • Keyed seed fallback gated on proxyToNonOwner_ — a seed serving a non-owned key is a proxy, exactly what proxyToNonOwner=false forbids (preserves the D5 contract). Default true matches Java, which always proxies.
  • Caller-driven exhaustion ≠ unreachability — excluding every known server still throws (no seed rescue of an excluded node, which can alias a seed under a different address, e.g. localhost vs. a topology container IP). This surfaced via a PingRetry regression and is fixed in both selection paths.
  • connection_ reassigned on runtime failover (+ fast path) so isConnected() stays truthful and dead seeds aren't re-probed per op; its reads in isConnected()/disconnect() now take stateMutex_ to remove the shared_ptr assignment race.

Intentional divergences from the Java 16.3 client

  • connect() throws when all seeds are down (Java tolerates and defers to the first op) — keeps our existing "establish ≥1 connection or throw" contract; the message names every seed.
  • No failed-server timeout memo (Java's connectionFailedServers/serverFailureTimeout) — dead topology members are re-probed per op during an outage (dead bootstrap seed is avoided by the fast path). Latency-under-failure only. Left as future work.
  • The privileged bootstrap connection_ is kept rather than folded into the pool (the Java-faithful flat model) — a larger refactor noted as future work.

Tests

  • +2 startup (FromUriIntegrationTest): recover via 2nd seed; all-seeds-unreachable throws naming each.
  • +2 runtime (MultiSeedIntegrationTest): BASIC client seeded with both nodes recovers after the connected node is killed; HASH client from a multi-seed URI with a dead first seed fails over at connect() then learns topology and hash-routes.
  • All affected regressions re-run green (RetryView incl. D5 proxy-disabled throw, PingRetry incl. all-excluded throw, HashAwareRouting, ConcurrentMultiServer). Unit 250/250; -Werror/Release build clean.

🤖 Generated with Claude Code


Also in this PR: docs hygiene + PR #14's test-count convention

A follow-up docs: commit (separate from the feature commit) does current-state cleanup and adopts the convention from #14 (closed unmerged as stale; its decision + DECISIONS entry were re-applied here):

  • STATUS.md trimmed to current-state-only: 1–3 Next steps, to-do-only backlog, dead Step-10 plan removed, stale dates/phrasings fixed.
  • Test counts live once in STATUS.md (250 unit / 96 integration / 20 suites, via --gtest_list_tests); README drops hard counts and points to STATUS.
  • DECISIONS.md: non-date heading fixed; schema-bug gotcha + docs: de-duplicate test counts; drop CI test-count badge #14's count-dedup entry preserved.

fromUri now carries all comma-separated hosts as seed servers, retained in
URI order (RemoteCache::seeds_), mirroring the Java client's immutable
ClusterInfo.getInitialServers() fallback.

Startup failover: connect() tries each seed in order (reusing the pre-built
bootstrap connection for seeds_[0]); all unreachable -> one
HotRodClientException listing every seed tried.

Runtime failover: a new tryConnectSeeds() helper is wired into the
fall-through points of selectAnyServer and selectServerForKey (incl. the
previous `return connection_` no-owners/empty path). It prefers the live
bootstrap connection (fast path, so happy path + BASIC mode do no new I/O),
else walks the seeds via orderKeyCandidates and adopts the first reachable
seed as the new bootstrap connection.

Semantics:
- Keyed seed fallback is gated on proxyToNonOwner_ (a seed serving a
  non-owned key is a proxy; preserves D5). Default true matches Java.
- Caller-driven exhaustion != unreachability: excluding every known server
  still throws (no seed rescue of an excluded node, which can alias a seed
  under a different address — localhost vs. topology container IP).
- connection_ is reassigned on runtime failover and its reads in
  isConnected()/disconnect() now take stateMutex_ (removes the shared_ptr
  assignment race); disconnect() closes outside the lock to avoid joining
  the read loop while it waits on stateMutex_.

Tests: +2 startup-failover (FromUriIntegrationTest), +1 runtime-failover
(MultiSeedIntegrationTest, 2-node BASIC client, kill connected node). All
affected regressions re-run green (RetryView, PingRetry, HashAwareRouting,
ConcurrentMultiServer); unit 250/250; -Werror/Release build clean.

Rationale, Java comparison, and divergences in docs/DECISIONS.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… convention

STATUS.md trimmed to current-state-only (per AGENTS.md/WORKFLOW.md):
- ⏭ Next steps rebuilt as a clean 1–3 (Step 12 / benchmark / cleanup) + a
  one-line "recently shipped" pointer; dropped the completed-work narrative pile
  and fixed the broken numbering.
- Backlog reduced to open [ ] items only; completed [x] blocks removed (they live
  in "Working and shipped" + dated DECISIONS entries).
- Removed the ~88-line Step-10 implementation plan (shipped work); its schema-bug
  gotcha was preserved in DECISIONS.md first.
- Refreshed stale date stamps (2026-09-16 / 2026-10-09 → 2026-10-19) and stale
  phrasings ("all four operations"; ops table).

Adopt PR #14's convention (imported here; PR #14 closed unmerged as stale):
- Test counts live once, in STATUS.md "Test status" (250 unit / 96 integration /
  20 suites, read via --gtest_list_tests). README drops all hard counts and points
  to STATUS for the breakdown.
- Imported PR #14's "stop repeating test counts" DECISIONS entry (dated 2026-10-09,
  placed chronologically) with a provenance note.

DECISIONS.md: fixed the one non-date heading (multiplexing landing → 2026-07-02,
per commit 5b35248); preserved the replace_if_unmodified nibble-flip schema gotcha.

README.md: Current Progress / milestone footer now include multi-server seed list
(#12); test-count strings removed per the convention above.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lestone

Align the multi-node cluster fixture with the single-server/auth fixtures:
test-configs/docker-compose-cluster.yml now uses quay.io/infinispan/server:latest
(was pinned 16.2) for all four nodes, so every integration suite exercises the same
(newest) server. The quickstart example stays pinned to 16.2 (it's a demo, not a
test). Verified green: the full multi-node suite set (Topology, HashAware,
ConcurrentClients, ConcurrentMultiServer, RetryView, PingRetry, MultiSeed) passes
7/7 against :latest = 16.3.

Docs: STATUS.md "integration tests pull :latest" bullet now notes the cluster
fixture too; DECISIONS.md records the alignment (rationale + risk/mitigation);
README requirements line corrected (only the quickstart pins 16.2) and the
milestone log gains a 2026-10 entry for Hot Rod URI (#9) + multi-server seed
list (#12), which shipped between the Step 11 and Step 12 milestones.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant