bitflicker64 opened a new issue, #3124:
URL: https://github.com/apache/hugegraph/issues/3124
## HugeGraph version
Current `master`, verified at commit
`b9710a7b0319ace41dd3c4ff5fb1165f53ee538e`.
Backend and topology:
- HStore backend
- 3 PD nodes
- 3 Store nodes
- 3 Server nodes
- Kubernetes deployment using stable per-Store DNS names
## Problem
Replacing a Store pod during a rolling update can leave running HugeGraph
Server pods permanently unable to write data to that Store.
The replacement Store pod has the same stable DNS name and Store node ID,
but a different pod IP. PD reports the Store as `Up` with the correct node ID
and address. The replacement Store is reachable from the Server pod, but HStore
data writes continue to use the failed transport until the Server process is
restarted.
This makes routine Store upgrades, node drains, and pod rescheduling capable
of taking the graph data plane out of service while the Server pods remain
Ready.
## Observed behavior
After replacing a Store pod, every affected data write returned HTTP 500
after the HStore retry loop:
```text
HgStoreClientException {sessionInfo: {storeNodeSession: {storeNode:
{address: "hugegraph-store-1.hugegraph-store.<namespace>.svc:8500",
nodeId: 2554185034686971778},
graphName: "DEFAULT/hugegraph/g"}},
reason: "UNAVAILABLE: io exception"}
```
Relevant stack frames:
```text
NodeTxExecutor.retryingInvoke(NodeTxExecutor.java:404)
NodeTxExecutor.doCommit(NodeTxExecutor.java:120)
NodeTxExecutor.commitTx(NodeTxExecutor.java:100)
NodeTxSessionProxy.commit(NodeTxSessionProxy.java:110)
HstoreSessionsImpl$HstoreSession.commit(HstoreSessionsImpl.java:464)
HstoreStore.commitTx(HstoreStore.java:691)
```
At the time of failure:
- PD reported all Stores as `Up`.
- PD returned the same node ID shown in the exception.
- `hugegraph-store-1...:8500` accepted TCP connections from inside the
Server pod.
- The failure did not recover after more than six minutes.
- Restarting the Server pods restored writes immediately; the same request
returned HTTP 201.
- Schema operations continued to work because they did not exercise this
HStore commit path.
- `/versions` and `/graphs` continued responding, so the Server remained
Ready and continued receiving traffic.
## Expected behavior
After a Store pod is replaced behind the same DNS name, existing Server
processes should discard or recover the failed Store transport, resolve the
current address, and resume writes without requiring a Server restart.
## Source analysis
### 1. The normal container path enables a SecurityManager
`bin/start-hugegraph.sh` defaults to:
```bash
OPEN_SECURITY_CHECK="true"
```
The normal Server Docker entrypoint invokes:
```bash
./bin/start-hugegraph.sh -j "${JAVA_OPTS:-}" -t 120
```
It does not pass `-s`, so the default is retained. `bin/hugegraph-server.sh`
then adds:
```bash
-Djava.security.manager=org.apache.hugegraph.security.HugeSecurityManager
```
The HStore Server image currently uses `eclipse-temurin:11-jre-jammy`.
There is no `networkaddress.cache.ttl` or `sun.net.inetaddr.ttl` setting in
the packaged `bin/` or `conf/` files.
Under Java 11, when a SecurityManager is installed and no positive DNS TTL
is explicitly configured, successful `InetAddress` resolutions are cached
forever (`-1`). This behavior is specified by Java's `InetAddress`
documentation:
https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/net/InetAddress.html
The corresponding OpenJDK implementation is here:
https://github.com/openjdk/jdk11u/blob/master/src/java.base/share/classes/sun/net/InetAddressCachePolicy.java
The JDK uses this infinite default as protection against DNS spoofing or
rebinding after an initial trusted resolution. HugeGraph does not explicitly
select or document this DNS policy; it is an incidental consequence of enabling
a SecurityManager to sandbox Gremlin execution.
### 2. HStore channels and stubs are retained by target address
`AbstractGrpcClient` stores channels in a static map keyed by the target
string:
```java
protected static Map<String, ManagedChannel[]> channels = new
ConcurrentHashMap<>();
```
Channels are constructed with:
```java
ManagedChannelBuilder.forTarget(target).usePlaintext().build();
```
No Store-client path removes or shuts down these cached channels when a
transport call fails.
The separate channel-pool indexing defect in this class should be reported
independently so it does not get buried in this DNS lifecycle issue.
Source:
https://github.com/apache/hugegraph/blob/b9710a7b0319ace41dd3c4ff5fb1165f53ee538e/hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/grpc/AbstractGrpcClient.java
### 3. `retryingInvoke` retries the same cached state
`NodeTxExecutor.retryingInvoke()` invokes the same `Supplier` repeatedly
after sleeping. It does not invalidate a Store node, session, stub, channel, or
DNS state.
The transaction's Store sessions are cached by node ID for the duration of
the complete retry loop and are replaced only in `doCommit()`'s final cleanup.
Source:
https://github.com/apache/hugegraph/blob/b9710a7b0319ace41dd3c4ff5fb1165f53ee538e/hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/NodeTxExecutor.java
### 4. `UNAVAILABLE` is not handled distinctly
`NotifyingExecutor` catches all thrown transport failures, reports
`HgNodeStatus.NOT_WORK`, and rethrows. It does not inspect
`StatusRuntimeException.getStatus().getCode()` and has no `UNAVAILABLE`
recovery path.
The resulting notification only calls `pdClient.invalidPartitionCache()`. It
does not invalidate:
- `HgStoreNodeManager.nodeIdMap`
- the cached Store address
- blocking or asynchronous stubs
- cached ManagedChannels
Sources:
-
https://github.com/apache/hugegraph/blob/b9710a7b0319ace41dd3c4ff5fb1165f53ee538e/hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/grpc/NotifyingExecutor.java
-
https://github.com/apache/hugegraph/blob/b9710a7b0319ace41dd3c4ff5fb1165f53ee538e/hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/HgStoreNodePartitionerImpl.java
### 5. Re-querying PD does not refresh the existing Store node
Invalidating the partition cache allows the next retry to re-query PD for
partition leadership. However, when PD returns the same Store node ID,
`HgStoreNodeManager.applyNode()` immediately returns the existing `nodeIdMap`
entry. It does not call the provider again to retrieve Store metadata or
rebuild the Store node.
Source:
https://github.com/apache/hugegraph/blob/b9710a7b0319ace41dd3c4ff5fb1165f53ee538e/hugegraph-store/hg-store-client/src/main/java/org/apache/hugegraph/store/client/HgStoreNodeManager.java
### 6. gRPC attempts DNS refresh, but the JVM cache remains stale
The effective Server dependency resolves gRPC Java to 1.47.0. Its
`pick_first` load balancer requests name resolution when a subchannel reaches
`TRANSIENT_FAILURE` or `IDLE`:
https://github.com/grpc/grpc-java/blob/v1.47.0/core/src/main/java/io/grpc/internal/PickFirstLoadBalancer.java
However, `DnsNameResolver` ultimately resolves through
`InetAddress.getAllByName()`:
https://github.com/grpc/grpc-java/blob/v1.47.0/core/src/main/java/io/grpc/internal/DnsNameResolver.java
With the JVM positive DNS cache set effectively to forever by the active
SecurityManager, a gRPC refresh receives the previously cached pod IP again.
This also explains why restarting only the Server process fixes the failure: it
clears the JVM DNS cache and all Store-client caches.
## Reproduction outline
1. Deploy PD, Store, and Server on Kubernetes with HStore enabled.
2. Ensure Server resolves a Store through a stable per-pod/headless-Service
DNS name.
3. Successfully perform a graph data write.
4. Replace that Store pod so the DNS name is unchanged but the pod IP
changes.
5. Wait for the Store to register as `Up` in PD.
6. Confirm the Store DNS name and port are reachable from the Server pod.
7. Perform another graph data write.
Actual result: writes repeatedly fail with `UNAVAILABLE: io exception` until
Server is restarted.
Expected result: the existing Server reconnects to the replacement Store
automatically.
## Suggested fixes
### Required runtime fix
Configure a finite positive DNS cache TTL for the official Server
distribution/container when the SecurityManager is enabled. Prefer a Java
security properties override containing, for example:
```properties
networkaddress.cache.ttl=30
```
An immediate OpenJDK 11 workaround is to pass:
```text
-Dsun.net.inetaddr.ttl=30
```
This was validated on Temurin 11 with the following effective positive TTLs:
| Runtime condition | Effective positive DNS TTL |
| --- | ---: |
| No SecurityManager, no TTL setting | `30` |
| SecurityManager, no TTL setting | `-1` (forever) |
| SecurityManager with `-Dsun.net.inetaddr.ttl=30` | `30` |
| SecurityManager with `-Dnetworkaddress.cache.ttl=30` | `-1` (forever) |
`networkaddress.cache.ttl` is a Java security property. Passing it only as
an ordinary `-D` system property does not change Java 11's `InetAddress` cache
policy. The supported distribution-level fix should set it through Java
security properties; the chart-level JVM workaround should use
`-Dsun.net.inetaddr.ttl=30`.
Java 11 checks the `networkaddress.cache.ttl` security property before the
`sun.net.inetaddr.ttl` system-property fallback. Therefore, if the distribution
later configures `networkaddress.cache.ttl` through Java security properties,
that distribution value takes precedence and the chart's
`-Dsun.net.inetaddr.ttl=30` becomes a harmless no-op. It will not override or
conflict with the upstream policy.
### Compatibility and security tradeoff
For a healthy cluster whose Store addresses do not change, making the TTL
finite does not tear down established channels or force working connections to
re-resolve. DNS cache expiration is lazy: it permits a later resolution to
consult DNS again, but does not itself schedule a DNS query every 30 seconds.
In this client, gRPC requests name resolution when the channel encounters
failure or enters idle state.
The intended observable change is that a new Store IP behind an unchanged
DNS name can eventually be discovered instead of the old IP remaining cached
for the lifetime of the Server JVM.
This is not entirely security-neutral. The SecurityManager-dependent
infinite TTL is a DNS spoofing and rebinding defense, so changing it to a
finite value reopens a bounded re-resolution window. The proposed tradeoff is
appropriate here because:
- `HugeSecurityManager` exists to sandbox Gremlin script execution; the
repository contains no indication that its incidental infinite DNS cache is a
deliberate Store-transport security control.
- HStore names are obtained from PD metadata rather than arbitrary
user-controlled request hostnames.
- `30` seconds is OpenJDK 11's own default positive TTL when no
SecurityManager is installed.
- The negative DNS cache policy is separate
(`networkaddress.cache.negative.ttl`, normally 10 seconds) and is not changed
by this proposal.
If the project considers PD-provided Store addresses untrusted, that trust
boundary should be addressed directly through PD authentication, transport
security, and address validation rather than by permanently pinning the first
DNS answer received by each Server process.
### Optional Store-client hardening
- Detect `Status.Code.UNAVAILABLE` explicitly.
- Atomically evict the target's cached blocking stubs, asynchronous stubs,
and ManagedChannels.
- Shut down evicted ManagedChannels.
- Invalidate or refresh the corresponding cached `HgStoreNode` so address
changes for the same node ID can be observed.
- Reset connection backoff or rebuild the target before the next application
retry.
Channel invalidation alone will not solve the stable-DNS case while the JVM
DNS cache remains infinite, so the finite TTL is required.
## Kubernetes readiness impact
The official HStore image health check currently calls `/versions`. That
endpoint remains healthy during this failure because the Server process and
control-plane APIs are available even though HStore data writes are unavailable.
It would be useful to expose a non-mutating data-plane readiness endpoint
that verifies the Server can reach the current HStore partition leaders. Helm
charts could then use that endpoint instead of a process-only readiness check.
## Operational severity
This failure is triggered by ordinary Kubernetes operations such as Store
upgrades, pod rescheduling, and node drains. It does not self-heal, is
invisible to the current readiness probe, and requires restarting otherwise
healthy Server pods to restore writes.
--
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]
