fix(store): recover retries after Store replacement

[bitflicker64]

Purpose

Closes #3124.

In Kubernetes, a Store keeps the same DNS name and Store ID when its Pod is replaced, but its IP can change. HugeGraph already connects through that stable DNS name. The missing piece was the retry path: after a transport failure, the same transaction could keep using the session bound to the old Store process.

flowchart LR
    subgraph Before["Before"]
        direction TB
        B1["Store Pod gets a new IP"] --> B2["transport fails"]
        B2 --> B3["retry reuses old Node + Session"]
        B3 --> B4["request fails"]
    end

    subgraph After["After"]
        direction TB
        A1["Store Pod gets a new IP"] --> A2["transport fails with UNAVAILABLE"]
        A2 --> A3["discard exact failed Node + target"]
        A3 --> A4["retry opens current Node + Session"]
        A4 --> A5["gRPC reconnects to new IP"]
        A5 --> A6["same request succeeds"]
    end

Loading

What changed

• Remove the custom IP fingerprint, five-second DNS polling, pool replacement, and maintenance executors. Kubernetes owns Pod IPs, while gRPC owns DNS resolution and transport reconnect.
• When gRPC reports UNAVAILABLE, discard only the exact failed Store node, close that target, and replace its cached session during the same transaction retry. CANCELLED and unrelated failures do not evict the node.
• Rebuild blocking and async stubs only when their channel generation changes, with focused tests for stale sessions, Store replacement, and channel rebinding.

This relies on the finite JVM DNS TTL from #3126 and follows the ownership boundaries documented by Kubernetes StatefulSets, Java 11 DNS cache properties, and gRPC's NameResolver.

Verification

• Java 11 store-client-test: 9 tests, 0 failures, 0 errors, 0 skipped.
• Java 11 compile, EditorConfig, Checkstyle, Apache RAT, and git diff --check: passed.
• Exact master + PR Docker build: 38/38 modules passed.
• Exact-head GitHub CI: 17/17 checks passed.

Kubernetes scenario	Result
1 Server + 1 PD + 1 Store	Pod UID/IP changed while PVC, Store ID, and FQDN stayed stable; the in-flight request returned 201 after about 31 seconds
1 Server + 3 PD + 3 Store, Leader failure	Store-0 was force deleted while leading 7/12 partitions; logs showed UNAVAILABLE → NOT_WORK → retry, and the 120-vertex request returned 201 after about 35 seconds
1 Server + 3 PD + 3 Store, Follower failure	Store-0 was force deleted again with zero Leaders; the 60-vertex request returned 201 immediately
Final replicated state	All 3 Stores were Up and all 12 partitions had 3 normal replicas; the final scan observed all 420 expected uniquely identified vertices
Process and recovery	Server Pod UID, JVM PID, and start time stayed unchanged with 0 restarts; a later write/delete and 10/10 scans passed; no gRPC client is closed or OOM event was observed

The runtime uses gRPC api/core/netty/stub 1.47.0 for this behavior. Protobuf-related artifacts remain at 1.39.0.

Scope

This PR fixes Store replacement recovery. It does not add cross-process exactly-once commit or deduplication semantics, and it does not add a second DNS or reconnect state machine.

Documentation

No user-facing configuration or dependency is added.

[Copilot]

Pull request overview

This PR updates the HStore gRPC client channel/stub lifecycle so that when a stable Store target (e.g., DNS name) resolves to a different address set, the client discards the prior channel pool and rebuilds related stub pools to avoid getting stuck on failed transports (issue #3124).

Changes:

• Add per-target “resolved address set” fingerprinting and retire/replace cached channel pools when the fingerprint changes (or when a previously-unresolved target first resolves).
• Rebuild blocking and async stub pools when they no longer correspond to the current channel pool, with concurrency ordering to prevent stale work from reintroducing retired channels.
• Add regression tests covering address changes and concurrent refresh/stub-build interleavings.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/grpc/AbstractGrpcClient.java	Adds resolved-address fingerprinting, refresh/retire logic for channel pools, and stub-pool rebuild safeguards under concurrent refresh.
hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/client/grpc/AbstractGrpcClientTest.java	Adds refresh-focused tests validating channel replacement, stub-pool rebuild, and concurrency ordering behavior.
hugegraph-store/hg-store-test/src/main/java/org/apache/hugegraph/store/client/ClientSuiteTest.java	Introduces a small suite to run the refresh regression tests together.

Comments suppressed due to low confidence (1)

hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/grpc/AbstractGrpcClient.java:172

• When (re)building the async stub pool, the loop always selects targetChannels[index] for every slot, so all cached async stubs share a single channel. This defeats the channel pool and undermines concurrency/failover across channels. Bind each stub to its corresponding channel (or at least distribute across the pool) by using the loop index.

                        IntStream.range(0, concurrency).parallel().forEach(i -> {
                            ManagedChannel channel = targetChannels[index];
                            AbstractAsyncStub stub = getAsyncStub(channel);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[imbajin]

Blocking: yes. Summary: Address refresh can abort active RPCs, adds synchronous DNS resolution to every request path, and does not preserve the existing gRPC target contract; the async refresh race also lacks equivalent coverage. Evidence: static review across six independent lanes; mvn test -pl hugegraph-store/hg-store-test -am -Dtest=AbstractGrpcClientTest -DfailIfNoTests=false passed 4 tests.

[codecov]

Codecov Report

❌ Patch coverage is 79.16667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.34%. Comparing base (c9a646d) to head (8335865).

Files with missing lines	Patch %	Lines
...che/hugegraph/store/client/HgStoreNodeManager.java	75.00%	1 Missing and 4 partials ⚠️

❗ There is a different number of reports uploaded between BASE (c9a646d) and HEAD (8335865). Click for more details.

HEAD has 2 uploads less than BASE

Flag	BASE (c9a646d)	HEAD (8335865)
	3	1

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #3130       +/-   ##
============================================
- Coverage     39.23%   0.34%   -38.90%     
+ Complexity      264      74      -190     
============================================
  Files           771     749       -22     
  Lines         65938   63447     -2491     
  Branches       8759    8318      -441     
============================================
- Hits          25872     220    -25652     
- Misses        37310   63204    +25894     
+ Partials       2756      23     -2733     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[imbajin]

Blocking: yes. Summary: DNS refresh can fail under the default security policy, and QueryV2 can race channel retirement. Evidence: six independent exact-head lanes; 14/14 focused Java tests and git diff --check passed; all visible exact-head checks passed.

[imbajin]

Blocking: yes. The initial refresh deadline can suppress DNS monitoring for the lifetime of a target when the JVM's nanoTime origin is negative; three independent current-head review lanes converged on this issue.

[bitflicker64]

Codecov follow-up for the earlier 0% patch report at 198de19e: codecov/patch is green on final head 5afea3b2. The original finding was valid; the Store aggregate report had been produced before the focused client execution. The final Store POM keeps the existing centralized profile design but declares Surefire before jacoco:report-aggregate, and testQueryV2StubFollowsPublishedPoolAcrossRefresh() now executes the real QueryV2Client#getQueryServiceStub(String) path. The Java 11 profile passes 24/24, and the generated JaCoCo XML records changed line 47 as mi=0, ci=5.

[bitflicker64]

Final-head CI note for 6beea6f: all 17 GitHub checks passed, and the Store job generated and discovered the expected JaCoCo reports. Its legacy codecov-action v3.0.0 upload was rejected by Codecov with HTTP 429 and an expected retry window of 1,703 seconds, after which the action intentionally exited 0; consequently Codecov never registered the final commit. The cooldown has elapsed, but GitHub does not allow the fork contributor to rerun the completed Apache job. I am briefly closing and reopening this PR to retrigger trusted CI on the unchanged SHA, without modifying history or the four-file diff. The regenerated Java 11 Store profile still passes 27/27 and records QueryV2 line 47 as mi=0, ci=5 and line 58 as mb=0, cb=2.

[imbajin]

Blocking: yes. Summary: The refresh implementation still has a non-atomic stub-to-channel handoff and a shared resolver pool that can pause refreshes for every target when DNS blocks. Evidence: exact-head static review across six independent lanes; current-head checks are green, while these races remain distinct from the previously resolved SecurityManager, target parsing, stub distribution, and nanoTime findings.

[imbajin]

Blocking: yes. Summary: Cold-target initialization can still run channel creation from a restricted Gremlin request, and the fingerprint parser accepts a DNS target form that gRPC 1.39 cannot build. Evidence: AbstractGrpcClient.java lines 131, 490, and 546; ExecutorPool.createExecutor() uses a lazy SynchronousQueue executor with CallerRunsPolicy; gRPC 1.39 DnsNameResolverProvider requires a slash-prefixed URI path.

[imbajin]

Blocking: yes. Summary: The change eagerly creates 129 executor threads, retains target-keyed state indefinitely, and can publish a channel pool containing null entries after an Error. Focused validation passed 29/29 tests, while the latest dependency-check and Codecov checks are failing. Evidence: static review of AbstractGrpcClient.java:54-93 and 444-485; mvn -q test -pl hugegraph-store/hg-store-test -am -Dtest=AbstractGrpcClientTest -Dsurefire.failIfNoSpecifiedTests=false -Djacoco.skip=true; gh pr checks 3130 -R apache/hugegraph.

[bitflicker64]

Current-head CI triage for fad20a3: the dependency-check job did not execute dependency analysis; actions/setup-java failed while downloading JDK 11 after repeated socket hang up errors, so that failure is external setup/network noise. The Codecov failures are current and substantive as reported: patch coverage is 0% for two changed QueryV2Client lines and project coverage is 34.73%. The Store job itself passed, but its uploaded aggregate coverage did not demonstrate those changed lines to Codecov. The three new exact-head review findings were validated and answered inline; all three require code/lifecycle follow-up before the review can be considered clear.

[imbajin]

Design direction to validate: keep Store identity at the DNS target layer

The current head (bdd8df11) shows that explicit address fingerprinting and pool replacement can be made defensive, but the added concurrency and lifecycle machinery suggests that the ownership boundary needs another look.

In Kubernetes, HugeGraph should use a stable logical target such as store-1.<headless-service>:8500, not the Pod IP behind it:

flowchart LR
    HG["HugeGraph Store client<br/>stable DNS target"]
    CH["gRPC ManagedChannel"]
    NR["gRPC NameResolver<br/>resolve + reconnect"]
    DNS["Kubernetes DNS"]
    OLD["old Store Pod<br/>10.0.0.8"]
    NEW["replacement Store Pod<br/>10.0.0.19"]

    HG --> CH --> NR --> DNS
    DNS -. "before replacement" .-> OLD
    DNS -- "after replacement" --> NEW

Loading

Kubernetes       assigns the new Pod IP and updates DNS
Java DNS policy  permits a fresh answer (#3126)
gRPC             resolves the target and reconnects the transport
HugeGraph        owns Store identity, session state, retry, and shutdown

Under this model, HugeGraph does not need IP fingerprints, refresh deadlines, DNS worker pools, or a second connection-replacement state machine. It keeps the stable target and invalidates only the failed logical or transport state required for retry.

The validation plan is:

1. Start from merged fix(server): configure finite DNS cache TTL #3126 and fix(store): bind each gRPC stub to its own channel #3128 without this PR's DNS fingerprint and pool-replacement machinery.
2. Keep the same ManagedChannel for a stable Store DNS target.
3. Replace the Store Pod while keeping the DNS name and Store ID unchanged.
4. Keep the Server JVM alive and observe whether gRPC resolves the new IP after UNAVAILABLE.
5. If native recovery works, reduce this PR to the missing HugeGraph node, session, and retry lifecycle.
6. If it does not work, identify the exact failing boundary and add only the smallest proven invalidation mechanism.

same Store DNS + same node ID + new Pod IP
    -> no Server restart
    -> bounded read/write recovery
    -> no data-integrity regression

Three independent tracks will validate this direction: gRPC and JDK source behavior, a minimal Java 11 runtime experiment, and a blind root-cause analysis.

[imbajin]

Validation update: native gRPC recovery works; refocus this PR on the HugeGraph lifecycle

Three independent tracks reached the same conclusion: gRPC and JDK source analysis, a Java 11 runtime experiment, and a blind root-cause analysis.

Native gRPC recovery

The final Server distribution uses gRPC 1.47.0 for the behavior-driving api/core/netty/stub artifacts. Protobuf-related artifacts remain at 1.39.0, so the runtime is a mixed classpath.

The runtime experiment used Java 11.0.18, networkaddress.cache.ttl=30, one stable dns:///store-lab:50051 target, and three distinct backend IPs:

A  192.168.157.2
       |
       v
B  192.168.157.4
       |
       v
C  192.168.157.6

• All 32 original ManagedChannel instances recovered from A to B without channel replacement. The pool recovered about 33 seconds after the initial resolution.
• One original channel recovered from B to C in the same JVM about 31 seconds after the switch.

The relevant gRPC path is:

old transport fails
  -> InternalSubchannel enters IDLE or TRANSIENT_FAILURE
  -> pick_first requests name-resolution refresh
  -> DnsNameResolver resolves after its TTL gate
  -> Subchannel receives the new address and reconnects

This shows that a Store IP change does not require HugeGraph to destroy and rebuild its channel pool. resetConnectBackoff() only shortens an active transport backoff; it does not bypass DNS TTL. enterIdle() rebuilds resolver and load-balancer state for the whole channel, so it should not be the default response to an RPC failure.

HugeGraph lifecycle gap

The PR evicts the failed HgStoreNode, but NodeTxExecutor.openNodeSession() still caches HgStoreSession by node ID:

transport failure
  -> evict current HgStoreNode
  -> retry obtains the current node with the same Store ID
  -> session cache returns the old session
  -> old session's node is no longer current
  -> retry cannot obtain a usable stub

The retry must replace a cached session when its node is no longer current. The regression needs the same Store ID and stable DNS target and must prove that the same transaction retry opens a current session.

Refactor boundary

flowchart LR
    K8S["Kubernetes<br/>Pod IP + DNS"]
    JDK["Java DNS policy<br/>finite TTL from #3126"]
    GRPC["gRPC ManagedChannel<br/>resolve + reconnect"]
    HG["HugeGraph<br/>node + session + retry"]

    K8S --> JDK --> GRPC --> HG

Loading

The refactor should remove the external IP fingerprint, five-second polling, refresh executors, and whole-pool replacement. HugeGraph should keep the stable DNS target and fix only its node, session, retry, and explicit shutdown lifecycle.

The remaining integration test is a resource-bounded Kubernetes run on the exact master + PR result, recording the Store identity and IP change, Server process identity, first failure and recovery, and final data.

[imbajin]

K8s A/B result: the current PR cannot recover the in-flight retry

I ran a 1 PD + 1 Store + 1 Server Kubernetes test. HugeGraph used the stable Store FQDN pr3130-hugegraph-store-0.pr3130-hugegraph-store.pr-3130-hstore.svc:8500, not a Pod IP. In each phase, the Store Pod received a new UID and IP while its PVC, Store ID, and FQDN stayed unchanged. The Server Pod and JVM did not restart.

Phase	Store replacement	In-flight write	Later recovery	Server restart
Base + #3126, without #3130	IP 192.168.194.8 -> .10; Store ID unchanged	HTTP 500 after 128.19s with UNAVAILABLE	HTTP 201 about 199s after deletion	No
Exact base + current PR (4066d2ff)	IP 192.168.194.10 -> .12; Store ID unchanged	HTTP 500 after 38.39s with The gRPC client is closed	The next transaction succeeded immediately; then 10/10 writes passed	No

The exact merge includes #3126 and uses networkaddress.cache.ttl=30. The behavior-driving gRPC core/API artifacts are 1.47.0; protobuf-related artifacts remain at 1.39.0.

Store transport fails
  -> NOT_WORK evicts the current HgStoreNode and closes its target
  -> NodeTxExecutor retry reuses the session cached by node ID
  -> cached session still points to the evicted node
  -> node::isCurrent is false
  -> every remaining attempt sees "The gRPC client is closed"

next HTTP transaction
  -> creates a new transaction and session map
  -> obtains the current Store node and session
  -> succeeds immediately

This reproduces the source-level finding: the current PR invalidates the failed node and target, but the same transaction cannot replace its stale session.

Before merge, the PR needs to:

1. Replace a cached session when its HgStoreNode is no longer current.
2. Keep the stable Kubernetes FQDN as the Store target.
3. Let gRPC own DNS resolution and reconnect instead of maintaining IP fingerprints and rebuilding all channels.
4. Add a regression where the same Store ID and FQDN move to a new IP and the same transaction retry opens a current session.

Short client-side timeouts can leave Server-side requests running, so the final acceptance test must use a unique operation ID and distinguish an observed HTTP response from a later commit.

[imbajin]

Blocking: no. Summary: No new actionable findings were identified on the exact head after local review and focused Java 11 validation. Evidence: mvn -q test -pl hugegraph-store/hg-store-test -am -Dtest=AbstractGrpcClientTest -Dsurefire.failIfNoSpecifiedTests=false -Djacoco.skip=true (37/37 passed); gh -R apache/hugegraph pr checks 3130 (17/17 passed).

[imbajin]

Final result: what changed and what the Kubernetes test proves

The final refactor is on head 18ca057856684ff09ab8ac46951d30de84c0aca7.

Before and after

The previous version assumed that HugeGraph had to detect every Store IP change itself. It added five-second DNS polling, address fingerprints, several maintenance executors, and full channel-pool replacement. Runtime testing showed that this duplicated work already owned by Kubernetes DNS, the JVM DNS policy from #3126, and gRPC name resolution.

The actual failure was in HugeGraph's retry state. After UNAVAILABLE, HugeGraph discarded the failed Store node, but the same transaction reused the session cached under the same Store ID. That session still belonged to the old node, so every retry failed even after gRPC could reach the replacement Store.

The final implementation keeps the stable Store FQDN and makes three focused changes:

• UNAVAILABLE discards only the exact failed node and target. CANCELLED and unrelated failures do not evict the node.
• The same transaction retry replaces a cached session when its Store node is no longer current.
• Blocking and async stubs are rebuilt only when their channel generation changes.

This reduced the PR from 2,295 to 486 added lines. Production additions fell from 732 to 140, and AbstractGrpcClient fell from 671 to 95 added lines. The deleted code implemented the duplicate DNS polling and connection-replacement state machine, not required recovery behavior.

Exact Helm/Kubernetes acceptance

The exact master + PR image passed both the minimal 1 Server + 1 PD + 1 Store test and a replicated 1 Server + 3 PD + 3 Store test through the HugeGraph Helm chart.

Scenario	Observed result
Minimal topology	Store Pod UID/IP changed while PVC, Store ID, and FQDN stayed stable; the in-flight request returned 201 after about 31 seconds without restarting Server
Rolling replacement	Store-1 rejoined with a new Pod UID/IP and the same PVC, Store ID, and FQDN; the tested 120-vertex transaction returned 201
Leader Store failure	Store-0 was force deleted while leading 7/12 partitions; the replacement was still Pending when a 120-vertex transaction began; logs showed UNAVAILABLE -> NOT_WORK -> retry, and the request returned 201 after about 35 seconds
Follower Store failure	The same Store was force deleted again after it had zero Leaders; the tested 60-vertex transaction returned 201 immediately
Final cluster state	All 3 Stores were Up; all 12 partitions had 3 normal replicas; the final scan observed all 420 expected uniquely identified vertices
Recovery checks	A later write returned 201, its delete returned 204, and 10/10 scans returned 200; logs contained no gRPC client is closed
Process and resources	The Server Pod UID, JVM PID, and process start time stayed unchanged with 0 restarts; every test container reported 0 OOM events
Cleanup	The dedicated namespace, PVCs, PVs, and image were removed; the pre-existing 3 PD + 3 Store cluster kept the same Pod/PVC identities and restart counts

These results cover both leader movement and follower replacement while Kubernetes changes Pod IPs behind stable StatefulSet DNS names. They support the PR's intended Store reconnect and session-rebind path; they do not claim cross-process exactly-once commit behavior.

Other verification

• Java 11 Store client suite: 9/9 passed.
• Compile, EditorConfig, Checkstyle, Apache RAT, and git diff --check: passed.
• Exact-merge Docker build: 38/38 modules passed.
• Exact-head GitHub CI: 17/17 checks passed.
• All independent review lanes passed, with 0 unresolved review threads.

Future TODOs

These items are not blockers for this PR:

• P1, data integrity: define the outcome when a Store commits a write but the response is lost. Start with a failure-injection test using a non-idempotent operation, then choose either stable batch-ID deduplication or an explicit unknown-commit result.
• P2, regression coverage: automate the now-passing 1 + 3 + 3 leader/follower Store replacement scenario so future channel/session changes cannot regress it.