imbajin commented on code in PR #3221:
URL: https://github.com/apache/hugegraph/pull/3221#discussion_r4164893234
##########
hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java:
##########
@@ -87,6 +92,56 @@ 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 answers this server: one admin round trip
+ * (does the graph's first table exist) 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) {
+ E.checkArgument(timeoutMs > 0, "The probe timeout must be > 0, but got
%s", timeoutMs);
+ Map<String, Object> body = new LinkedHashMap<>();
+ body.put("ready", false);
+ List<String> tables = this.tableNames();
+ String table = tables.isEmpty() ? null : tables.get(0);
+ long start = System.currentTimeMillis();
+ Future<Boolean> exists = READINESS_EXECUTOR.submit(() -> {
+ return table != null && this.sessions.existsTable(table);
Review Comment:
‼️ Table existence does not prove that an HBase graph table can serve
traffic. This call uses HbaseSessions.existsTable(), which invokes
Admin.tableExists; an existing disabled table is still reported as ready, while
graph reads and writes fail. Please check that the selected table is
enabled/available (or perform a bounded read through the serving path) before
returning ready, and add a disabled-table case.
##########
hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java:
##########
@@ -0,0 +1,376 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.hugegraph.backend.store.hstore;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicReference;
+
+import org.apache.commons.lang3.concurrent.BasicThreadFactory;
+import org.apache.hugegraph.pd.client.PDClient;
+import org.apache.hugegraph.pd.common.PDException;
+import org.apache.hugegraph.pd.grpc.Metapb;
+import org.apache.hugegraph.store.grpc.state.HgStoreStateGrpc;
+import org.apache.hugegraph.store.grpc.state.SubStateReq;
+import org.apache.hugegraph.util.E;
+import org.apache.hugegraph.util.Log;
+import org.slf4j.Logger;
+
+import io.grpc.ManagedChannel;
+import io.grpc.ManagedChannelBuilder;
+import io.grpc.StatusRuntimeException;
+
+/**
+ * Storage-aware readiness of this server, from this server's point of view:
+ * at least one Store answers a direct, local, read-only gRPC call
+ * (HgStoreState.getScanState, which reads the node's own scan-pool stats and
+ * never touches raft). The Store list comes from PD, refreshed in the
+ * background (single-flight) while the last known list is used right away, so
+ * PD only matters until the first list is known: a PD that is slow, restarting
+ * or down afterwards does not change the readiness of a server whose Stores
+ * still answer. Every known Store is pinged in parallel and the first answer
+ * wins; every wait is bounded by one shared time budget. The result carries
+ * no addresses and no raw exception text, since it is served without
+ * authentication; the full messages go to the log.
+ * <p>
+ * Scope: ready means this server knows a Store list and reaches at least one
+ * Store over gRPC. A Store whose status RPC answers while its raft or
+ * partition path is broken is not detected. A read through the graph path
+ * was measured as the gate and rejected: a failed read invalidates the
+ * partition cache the queries share (with PD down the data plane then fails
+ * too), and one moving partition stalls every read through that client past
+ * the budget, so a rolling Store restart took every server out of the
+ * Service while the traffic was fine.
+ */
+public final class HstoreStorageProbe {
+
+ public static final String META_STORAGE_READINESS = "storage_readiness";
+
+ private static final Logger LOG = Log.logger(HstoreStorageProbe.class);
+
+ private static final ExecutorService EXECUTOR =
Executors.newCachedThreadPool(
+ new
BasicThreadFactory.Builder().namingPattern("storage-readiness-%d")
+ .daemon(true).build());
+
+ private static final KnownStores KNOWN = new KnownStores();
+ private static final Map<String, ManagedChannel> CHANNELS = new
ConcurrentHashMap<>();
+
+ private HstoreStorageProbe() {
+ }
+
+ /** The active stores as PD sees them. */
+ public interface StoreLister {
+
+ List<Metapb.Store> activeStores() throws Exception;
+ }
+
+ /** One cheap call to one store; returning (any value) means it answered.
*/
+ public interface StorePinger {
+
+ void ping(Metapb.Store store, long timeoutMs) throws Exception;
+ }
+
+ /** The last store list PD answered with, shared by consecutive probes. */
+ public static final class KnownStores {
+
+ private volatile List<Metapb.Store> stores = Collections.emptyList();
+ private volatile long at;
+ private volatile Boolean pdOk;
+ private volatile long pdAt;
+ private final AtomicReference<CompletableFuture<List<Metapb.Store>>>
inFlight =
+ new AtomicReference<>();
+
+ public List<Metapb.Store> stores() {
+ return this.stores;
+ }
+
+ public long ageMs() {
+ return this.at == 0L ? -1L : System.currentTimeMillis() - this.at;
+ }
+
+ /** Outcome of the last finished PD refresh, null before the first
one. */
+ public Boolean pdOk() {
+ return this.pdOk;
+ }
+
+ public long pdAgeMs() {
+ return this.pdAt == 0L ? -1L : System.currentTimeMillis() -
this.pdAt;
+ }
+
+ public void update(List<Metapb.Store> stores) {
+ this.pdOk = true;
+ this.pdAt = System.currentTimeMillis();
+ if (stores != null && !stores.isEmpty()) {
+ this.stores = Collections.unmodifiableList(new
ArrayList<>(stores));
+ this.at = this.pdAt;
+ }
+ }
+
+ public void pdFailed() {
+ this.pdOk = false;
+ this.pdAt = System.currentTimeMillis();
+ }
+
+ /**
+ * The refresh in flight, or a new one started on `executor`: only one
+ * PD call runs at a time no matter how many probes miss the cache,
+ * so a hung PD parks one thread, not one per probe.
+ */
+ CompletableFuture<List<Metapb.Store>> refresh(StoreLister lister,
+ ExecutorService
executor) {
+ CompletableFuture<List<Metapb.Store>> running =
this.inFlight.get();
+ if (running != null && !running.isDone()) {
+ return running;
+ }
+ CompletableFuture<List<Metapb.Store>> mine = new
CompletableFuture<>();
+ if (!this.inFlight.compareAndSet(running, mine)) {
+ return this.inFlight.get();
+ }
+ executor.execute(() -> {
+ try {
+ List<Metapb.Store> stores = lister.activeStores();
+ this.update(stores);
+ mine.complete(stores == null ? Collections.emptyList() :
stores);
+ } catch (Throwable e) {
+ this.pdFailed();
+ mine.completeExceptionally(e);
+ }
+ });
+ return mine;
+ }
+ }
+
+ /** The unauthenticated body: no addresses, no raw exception text. */
+ private static Map<String, Object> result(boolean ready, String reason,
int activeStores,
+ Long answeredStore, Boolean
pdReachable,
+ long pdAgeMs, long storesAgeMs,
long storeMillis) {
+ Map<String, Object> map = new LinkedHashMap<>();
+ map.put("ready", ready);
+ map.put("reason", reason);
+ map.put("active_stores", activeStores);
+ map.put("answered_store", answeredStore);
+ map.put("pd_reachable", pdReachable);
+ map.put("pd_checked_age_ms", pdAgeMs);
+ map.put("stores_age_ms", storesAgeMs);
+ map.put("store_millis", storeMillis);
+ return map;
+ }
+
+ /**
+ * Probe through the process-wide PD client and this probe's own plaintext
+ * channels to the stores (the store gRPC server takes no credentials).
+ *
+ * @param timeoutMs the whole budget for PD plus stores
+ */
+ public static Map<String, Object> probe(long timeoutMs) {
+ PDClient pd = HstoreSessionsImpl.getDefaultPdClient();
+ if (pd == null) {
+ return result(false, "pd client not initialised", 0, null, false,
-1L, -1L, 0L);
+ }
+ return probe(KNOWN, () -> {
+ List<Metapb.Store> stores = pd.getActiveStores();
+ pruneChannels(CHANNELS, stores);
Review Comment:
⚠️ The PD refresh prunes old-address channels before KnownStores.update()
publishes the new list, while a concurrent probe can already have snapshotted
known.stores() at line 250. During a full Store replacement, that probe still
pings the old list and can return not-ready although PD has returned the
replacement list; StorageReadiness then caches the false result for the
configured TTL. Please make pruning and pings use one store-list generation, or
defer channel shutdown until probes using the previous snapshot finish.
--
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]
