bitflicker64 commented on code in PR #3221:
URL: https://github.com/apache/hugegraph/pull/3221#discussion_r4187494543


##########
hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java:
##########
@@ -87,6 +93,83 @@ private void registerMetaHandlers() {
             HbaseMetrics metrics = new HbaseMetrics(this.sessions);
             return metrics.compact(this.tableNames());
         });
+
+        this.registerMetaHandler(META_STORAGE_READINESS, (session, meta, args) 
-> {
+            E.checkArgument(args.length == 1 && args[0] instanceof Number,
+                            "Expect the timeout in ms as the only argument");
+            return this.storageReadiness(((Number) args[0]).longValue());
+        });
+    }
+
+    public static final String META_STORAGE_READINESS = "storage_readiness";
+
+    private static final ExecutorService READINESS_EXECUTOR = 
Executors.newCachedThreadPool(r -> {
+        Thread thread = new Thread(r, "hbase-readiness");
+        thread.setDaemon(true);
+        return thread;
+    });
+
+    /**
+     * Whether the HBase cluster can serve this graph: admin round trips
+     * (is every table of the graph enabled and available) within the budget.
+     * The body carries no addresses and no raw exception text, since the
+     * readiness endpoint is unauthenticated.
+     */
+    private Map<String, Object> storageReadiness(long timeoutMs) {
+        List<String> tables = this.tableNames();

Review Comment:
   Important: This probe runs on the graph store only. `graph.metadata(null, 
"storage_readiness", t)` goes `StandardHugeGraph.metadata` -> 
`graphTransaction().metadata` -> `AbstractBackendStore.metadata`, so `this` is 
the `HbaseGraphStore`, and `tableNames()` lists vertices, edges and the index 
tables. The schema tables (vertex/edge label, property key, index label) and 
the `counters` table belong to `HbaseSchemaStore`; `counters` is added only in 
`HbaseSchemaStore.tableNames()`. The `firstUnavailable` Javadoc below lists 
counters, but this handler never checks that table. If a schema-table region is 
unassigned (a RegionServer crash, a region in transition) or the table is 
disabled, schema cache misses and schema id generation (`SchemaTransaction` -> 
`store().nextId`) fail while `/readiness` answers 200. That is the same 
false-ready case as the single-table finding in round 6, one store further. 
`HbaseReadinessTest` passes `firstUnavailable` a synthetic list, so it cannot 
catch this.
   
   Requested change: check the schema store's tables as well, for example a 
second `metadata` call with a schema `HugeType` so it dispatches to the schema 
store, or pass both stores' table lists. Correct the Javadoc, and add a case 
where only a schema or counters table is unavailable.



-- 
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]

Reply via email to