SebastianGruza commented on code in PR #3210:
URL: https://github.com/apache/hugegraph/pull/3210#discussion_r4034011376
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -482,6 +495,82 @@ private HugeGraph createSysGraphIfNeed() {
return graph;
}
+ private void waitForActiveStores() {
+ int timeout = this.conf.get(ServerOptions.PD_STORES_WAIT_TIMEOUT);
+ if (timeout <= 0) {
+ return;
+ }
+ PDConfig pdConfig = PDConfig.of(this.pdPeers);
+ pdConfig.setAuthority(PdMetaDriver.PDAuthConfig.service(),
+ PdMetaDriver.PDAuthConfig.token());
+ // same short-lived client as limitStorage(); PDClient has no close()
+ PDClient pdClient = PDClient.create(pdConfig);
Review Comment:
Done in 3d64daa. Instead of a `PDClient` there is now a `PdReadinessProbe`:
one plaintext gRPC channel per PD peer, a raw `PDGrpc.newBlockingStub` with the
same `Authentication` interceptor that `AbstractClient.setBlockingParams()`
adds (credentials from `PDConfig.setAuthority()`, so without PD authentication
no interceptor is attached), no watchers, closed with `shutdownNow()` in a
try-with-resources once the wait is over. PD forwards requests to its leader on
the server side, so any peer answers; the probe rotates through the peers so
one dead PD does not eat every poll.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -482,6 +495,82 @@ private HugeGraph createSysGraphIfNeed() {
return graph;
}
+ private void waitForActiveStores() {
+ int timeout = this.conf.get(ServerOptions.PD_STORES_WAIT_TIMEOUT);
+ if (timeout <= 0) {
+ return;
+ }
+ PDConfig pdConfig = PDConfig.of(this.pdPeers);
+ pdConfig.setAuthority(PdMetaDriver.PDAuthConfig.service(),
+ PdMetaDriver.PDAuthConfig.token());
+ // same short-lived client as limitStorage(); PDClient has no close()
+ PDClient pdClient = PDClient.create(pdConfig);
+ try {
+ Metapb.PDConfig pd = pdClient.getPDConfig();
Review Comment:
Done in 3d64daa. Every probe call gets `withDeadlineAfter(min(remaining
budget, 5 s poll))`, and the budget is checked before the call, not after.
`testBlackholedPdStaysWithinTheBudget`: a probe that hangs for its whole
deadline on every call receives shrinking deadlines (all ≤ 1 s at a 1 s poll)
and the whole wait ends below timeout + one poll. On the lab with
`pd.peers=192.168.80.250:8686` (a black hole) and `pd.stores_wait_timeout=20`:
`Waiting for the PD cluster: 192.168.80.250:8686: UNAVAILABLE (16s left … 10s …
4s)`, then `Timed out after 20s waiting for the PD cluster to be ready
(192.168.80.250:8686: UNAVAILABLE); start the stores first or raise
pd.stores_wait_timeout`, exit 1 after 31 s (10 s of JVM boot plus the 20 s
budget). Log: `results/issue-3203/fix/after-v2-unreachable-pd-timeout20.log` in
https://github.com/SebastianGruza/hugegraph-validation.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -482,6 +495,82 @@ private HugeGraph createSysGraphIfNeed() {
return graph;
}
+ private void waitForActiveStores() {
+ int timeout = this.conf.get(ServerOptions.PD_STORES_WAIT_TIMEOUT);
+ if (timeout <= 0) {
+ return;
+ }
+ PDConfig pdConfig = PDConfig.of(this.pdPeers);
+ pdConfig.setAuthority(PdMetaDriver.PDAuthConfig.service(),
+ PdMetaDriver.PDAuthConfig.token());
+ // same short-lived client as limitStorage(); PDClient has no close()
+ PDClient pdClient = PDClient.create(pdConfig);
+ try {
+ Metapb.PDConfig pd = pdClient.getPDConfig();
+ int required = pd.getMinStoreCount() > 0 ? pd.getMinStoreCount() :
Review Comment:
Done in 3d64daa, thanks, that case was a real regression. The probe first
asks `queryPartitions` with an empty query: if the cluster has any partition it
is ready and nothing is waited for, whatever PD thinks of the stores at that
moment. Only a cluster without partitions (first boot) waits, and it waits for
`getClusterStats() == Cluster_OK`, which is exactly what `allocShards()` needs,
with no count derived on the server side. One correction to your description:
`checkStoreStatus()` also sets `Cluster_Not_Ready` when fewer than
`pd.initial-store-count` stores are active (`StoreNodeService.java:831-835`),
so with three stores and `initial-store-count: 3` `Cluster_OK` on its own would
block a restart with one store down as well; hence "partitions exist" as the
first criterion and `Cluster_OK` only for an empty cluster.
Measured on the lab (1 PD + 3 Store + 1 Server from a tarball,
`usePD=true`): a cold start with stores at +5 s / +71 s → 14 progress lines
carrying PD's message (`The number of active stores is 1, less than
pd.initial-store-count:3`), `PD cluster ready after 70s: PD reports
Cluster_OK`, 0 × 105, exit 0 after 81 s, REST 200; a restart with the node2
store killed with `kill -9` (port 8500 closed) → `PD cluster ready after 0s:
cluster already has 24 partition(s)`, exit 0 after 8 s, REST 200; PD still
listed that store as Up 200 s later, which only confirms that the skip must not
depend on PD's view of the stores. `testInitialisedClusterIsNotWaitedFor`
covers the case (one probe, zero wait). Logs:
`results/issue-3203/fix/after-v2-*.log`.
--
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]
