imbajin commented on PR #3130:
URL: https://github.com/apache/hugegraph/pull/3130#issuecomment-5282824936
## 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:
```text
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:
```text
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.
```text
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
```mermaid
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
```
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:
```text
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
