fix(store): refresh gRPC channels after address changes

[bitflicker64]

Purpose of the PR

• close [Bug][HStore][Kubernetes] Data writes remain unavailable after Store pod replacement due to stale DNS resolution #3124

HStore caches gRPC channels and blocking/asynchronous stubs by Store target.
When a stable target resolves to a different address, those pools can retain the
failed transport indefinitely even though the replacement Store is reachable.

This is the channel-lifecycle half of #3124. Complete recovery behind a stable
DNS name still requires the finite positive DNS TTL from #3126; this patch does
not bypass an indefinitely stale JVM DNS cache.

Main Changes

• Fingerprint each target's sorted, deduplicated resolved address set before
  selecting a channel, with explicit first-run state and overflow-safe refresh
  deadlines.
• Keep DNS resolution and cold channel initialization off restricted Gremlin
  callers using bounded prestarted executors (4 maintenance, 4 initialization,
  and 1 retirement thread), with independent targets initialized concurrently.
• Publish complete replacement pools before retiring old channels; preserve the
  healthy pool across resolution, submission, creation, cleanup, and fatal-error
  paths, and force-terminate every partial pool before propagating failure.
• Rebuild blocking and asynchronous stub pools only from the currently published
  channel pool; route QueryV2 through that guarded path and clean up injected test
  channels when they are replaced.
• Add client/target lifecycle cleanup for channels, stubs, refresh metadata, and
  locks. Terminal Store notices evict exact node identities, stale sessions cannot
  resurrect state, and late notices cannot evict a same-address replacement.
• Parse raw host:port, dns:///host:port, and bracketed IPv6 targets for
  fingerprinting; unsupported resolver schemes remain delegated to gRPC.
• Add focused concurrency and failure regressions covering SecurityManager
  boundaries, target isolation, lifecycle races, partial creation, interrupts,
  executor shutdown, Store-node eviction, QueryV2, and DNS deadline edge cases.

Verifying these changes

• Need tests and can be verified as follows:
  • Java 11: mvn test -pl hugegraph-store/hg-store-test -am -Dtest=AbstractGrpcClientTest -Dsurefire.failIfNoSpecifiedTests=false: 37 tests, 0 failures, 0 errors, 0 skips.
  • git diff --check fad20a3c515018278869cc62e8162894373bff95..bdd8df11106d8b015228655ed140e70543469738: passed with no output.
  • Three independent exact-head reviews and one Design Audit completed with no blocking findings.

Does this PR potentially affect the following parts?

• Dependencies
• Modify configurations
• The public API (adds explicit channel/client lifecycle operations)
• Other affects
• Nope

Documentation Status

• Doc - TODO
• Doc - Done
• Doc - No Need

Blocking review follow-up (2026-08-13)

• Exact fix commit: bdd8df11106d8b015228655ed140e70543469738, directly on prior PR head fad20a3c515018278869cc62e8162894373bff95.
• Closed the three latest blocking findings: eager thread footprint, permanent target-state retention, and partial/null publication after Error.
• Also hardened close/register/refresh races, stale-node replacement identity, executor-shutdown convergence, and interrupt restoration.

[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 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.73%. Comparing base (b026a90) to head (fad20a3).
⚠️ Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
...he/hugegraph/store/client/query/QueryV2Client.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3130      +/-   ##
============================================
- Coverage     39.13%   34.73%   -4.40%     
- Complexity      264      498     +234     
============================================
  Files           770      782      +12     
  Lines         65779    67086    +1307     
  Branches       8726     8956     +230     
============================================
- Hits          25742    23304    -2438     
- Misses        37288    41178    +3890     
+ Partials       2749     2604     -145     

☔ 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) proves that explicit address fingerprinting and
pool replacement can be made increasingly defensive, but the accumulated
concurrency and lifecycle machinery suggests that we should first re-check the
ownership boundary.

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

flowchart LR
    HG["HugeGraph Store client<br/>stable DNS target only"]
    CH["gRPC ManagedChannel"]
    NR["gRPC NameResolver<br/>re-resolution / reconnect"]
    DNS["Kubernetes DNS / Endpoint"]
    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

The intended responsibility split would be:

Kubernetes       assigns the new Pod IP and updates DNS/Endpoints
Java DNS policy  permits a fresh answer (#3126)
gRPC             owns name resolution, reconnect/backoff and transport state
HugeGraph        owns logical Store identity, bounded business retry and close

Under that model, HugeGraph would not normally maintain resolved IP
fingerprints, refresh deadlines, DNS worker pools, or a second connection
replacement state machine. It would retain the stable target and use the
smallest gRPC lifecycle trigger needed after a retryable transport failure.

This is a hypothesis, not yet a replacement proposal. Before changing this PR,
we should validate it against the repository's effective gRPC version and the
original Store-replacement failure:

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 explicit DNS
   fingerprint/pool-replacement machinery.
2. Create and keep the same ManagedChannel for a stable Store DNS target.
3. Replace the Store Pod so the DNS name and Store node ID remain unchanged but
   the IP changes.
4. Keep the Server JVM and channel alive; capture whether gRPC requests fresh
   name resolution and reconnects after UNAVAILABLE.
5. If it recovers, reduce this PR to the missing retry/lifecycle/cleanup
   boundary and focused regressions.
6. If it does not recover, locate the exact failed boundary (JVM DNS cache,
   gRPC resolver refresh, reconnect/backoff, HugeGraph retry, or client cache)
   and add only the minimum explicit invalidation/replacement mechanism proven
   necessary.

The key acceptance test is:

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

I am running three independent validation tracks next: gRPC/JDK source and
version behavior, a minimal runtime experiment, and a blind root-cause/design
analysis that is not given this proposal. I will report whether they converge
before recommending a rewrite of the current implementation.

[imbajin]

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

Three independent tracks now converge: gRPC/JDK source analysis, a Java 11
runtime experiment, and a blind root-cause analysis that was not given the
proposed design.

What the runtime experiment proved

The final Server distribution resolves the behavior-driving gRPC artifacts
(grpc-netty-shaded, grpc-core, grpc-api, grpc-context, grpc-stub) to
1.47.0. Protobuf-related artifacts remain 1.39.0, so this is a mixed classpath,
not a blanket "all gRPC is 1.47" statement.

Using Java 11.0.18, the #3126-equivalent Java security property
networkaddress.cache.ttl=30, gRPC 1.47, one stable
dns:///store-lab:50051 target, and explicit distinct backend IPs:

A  192.168.157.2
       |
       v
B  192.168.157.4
       |
       v
C  192.168.157.6

• All 32 original ManagedChannel identities recovered from A to B without
  creating a new channel. Full-pool recovery completed about 33 seconds after
  initial resolution.
• One original channel, with the same JVM and channel identity, recovered from
  B to C about 31 seconds after the switch.

This directly refutes the assumption that a Store IP change inherently requires
HugeGraph to destroy and rebuild the 32-channel pool.

The source path explains the result:

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

resetConnectBackoff() can only accelerate an existing transient-failure
backoff; it does not bypass the resolver or JVM DNS TTL. enterIdle() rebuilds
resolver/load-balancer state for the whole channel and should be a measured,
throttled fallback, not the default response to every RPC exception.

Separate blocking HugeGraph issue found by the blind analysis

The current PR evicts the exact HgStoreNode on NOT_WORK and adds
node::isCurrent guards to session/stream stub acquisition. However,
NodeTxExecutor.openNodeSession() still caches HgStoreSession only by node ID.

transport failure
  -> evict current HgStoreNode
  -> retry obtains/requires a current node
  -> session cache returns the old session for the same node ID
  -> old session's node is not current
  -> node::isCurrent rejects every retry

This can prevent the newly added eviction path from recovering even when gRPC
and DNS are healthy. The retry/session cache must become node-identity or
generation aware, or discard the stale session after eviction. The regression
must use the same node ID and stable DNS target and prove that the next attempt
uses a current node/session.

The same audit must cover stream errors and the separate channel caches in
GrpcStoreStateClient and GrpcNodeHealthyClient; otherwise they remain outside
the lifecycle contract.

Proposed refactor boundary

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

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

Loading

The default refactor should remove the duplicated external IP fingerprint,
five-second refresh state, refresh/initialization/retirement executors, and
whole-pool replacement. Keep stable logical DNS targets and long-lived channels;
fix the HugeGraph node/session/retry lifecycle and explicit shutdown instead.

Only if a real Kubernetes run falsifies native recovery should we add the
smallest measured fallback (resetConnectBackoff(), then a throttled
enterIdle() if still necessary). The current fingerprint/full-pool rebuild
should be the comparison case, not the starting design.

Remaining integration gate

The minimal Docker experiment did not exercise full HugeGraph, CoreDNS,
HugeSecurityManager, PD metadata, transactions, streams, or Store node/session
caches. I am now running a resource-bounded Kubernetes comparison on the actual
merge baseline (current master, including #3126, plus this PR's changes),
recording:

old/new Store Pod IP, stable DNS and node ID, Server PID,
loaded gRPC implementation version, channel/session identity,
first failure/recovery times, and data-integrity checks

I will post the exact result and cleanup state before recommending the final
rewrite.

[imbajin]

K8s A/B result: recovery exists, but the current PR breaks the in-flight retry

I ran a resource-bounded 1 PD + 1 Store + 1 Server test on Kubernetes. The Store was registered and consumed as the stable FQDN pr3130-hugegraph-store-0.pr3130-hugegraph-store.pr-3130-hstore.svc:8500, not as a Pod IP. For each phase I deleted only that Store Pod, then verified a new Pod UID/IP with the same PVC, Store ID, and FQDN. The Server Pod/JVM stayed unchanged within each phase.

Phase	Store replacement	In-flight write	First observable later success	Server restart
current base + #3126, without #3130	UID changed; IP 192.168.194.8 -> .10; Store ID unchanged	HTTP 500 after 128.19s, UNAVAILABLE: io exception	HTTP 201 at about t+199s from deletion	no
exact current-base + PR merge (4066d2ff, image sha256:2c4b914d...)	UID changed; IP 192.168.194.10 -> .12; Store ID unchanged	HTTP 500 after 38.39s, The gRPC client is closed	the next transaction succeeded immediately; then 10/10 writes succeeded	no

The merge image includes #3126 and has networkaddress.cache.ttl=30. Runtime name-resolution behavior is from gRPC core/API 1.47.0 (the distribution still has protobuf-related 1.39.0 artifacts).

What this proves

Store transport failure
  -> NOT_WORK evicts HgStoreNode and closes its channel
  -> NodeTxExecutor retry keeps its HgStoreSession cached by nodeId
  -> cached session still points to the evicted HgStoreNode
  -> node::isCurrent == false
  -> every remaining attempt in the same transaction sees
     "The gRPC client is closed"

next HTTP transaction
  -> transaction/session map is recreated
  -> current Store node/session is acquired
  -> succeeds immediately

This matches the independent source-level analysis. The failure is no longer hypothetical: the current PR shortens the failure to the configured retry window, but it cannot recover the request that encountered the Store replacement.

Required refactor before merge

1. Fix logical session generation first. NodeTxExecutor.openNodeSession() must reject/replace a cached session whose HgStoreNode is no longer the current node (or stop caching it across retry attempts). A retry after NOT_WORK must acquire a fresh node + session.
2. Keep the stable Kubernetes FQDN as the target. Do not turn Pod IPs into application-owned identity.
3. Keep one bounded channel lifecycle per logical target and let gRPC own DNS/address updates. Add only a deliberate reconnect/backoff trigger if the measured ~199s native recovery is unacceptable; avoid the current 5-second InetAddress fingerprint plus full 32-channel/stub-pool replacement.
4. Add a regression test for: same Store ID + same FQDN + new IP, first unary request fails transport, the same retrying transaction reacquires a current session and succeeds. Then cover stream/state/healthy clients, which currently have separate lifecycle paths.

One test nuance: short client-side probe timeouts can still leave Server-side requests running; several timed-out probes later committed. Acceptance therefore needs unique operation IDs and must distinguish “HTTP response observed” from “write eventually committed”.

Based on this K8s run, I would not merge the current channel-refresh implementation as-is. The small, coherent fix is stable FQDN + native gRPC resolution + generation-aware HugeGraph node/session retry.

[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).