diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/StoreNodeService.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/StoreNodeService.java index 3503d1ffc8..f8d8b81ed6 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/StoreNodeService.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/StoreNodeService.java @@ -158,6 +158,15 @@ public Metapb.Store register(Metapb.Store store) throws PDException { } // offline or up, or in the initial activation list, go live automatically + // TODO: do not mark a re-registering Store Up before it has restored its partition engines + // (HgStoreEngine.restoreLocalPartitionEngine); report a restoring state, or expose + // restore-complete per shard group, so a rolling restart can wait on it. The Helm chart + // (helm/hugegraph) production preset (values-cluster.yaml) keeps Store rollouts on + // OnDelete with a manual /v1/shardGroups barrier between Pod deletions; the default + // values use RollingUpdate. Restore-complete alone does not retire the barrier: a stopped + // Store stays Up until its keep-alive entry expires, so keep the rollout checks until PD + // also exposes liveness that tells stale membership apart. + // https://github.com/apache/hugegraph/issues/3229 Metapb.StoreState storeState = lastStore.getState(); if (storeState == Metapb.StoreState.Offline || storeState == Metapb.StoreState.Up || inInitialStoreList(store)) { diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java index d07ab75f8c..65451297ca 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java @@ -167,6 +167,12 @@ public void onLeaderStart(final long term) { @Override public void onLeaderStop(final Status status) { this.leaderTerm.set(-1); + // TODO: keep STATE_ERROR set by onError instead of overwriting it with STATE_FOLLOWER, so + // the probe view, and with it /v1/ready, reports a PD that stepped down for good after a + // snapshot failure. /v1/health does not read this view; making it report the state is the + // separate TODO at StoreAPI.checkHealthy. The Helm chart (helm/hugegraph) works around both + // by deriving a single PD's startup and liveness probes to /v1/ready. + // https://github.com/apache/hugegraph/issues/3222 this.probeView = new ProbeView(State.STATE_FOLLOWER, false); super.onLeaderStop(status); log.info("Raft lost leader "); diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java index bd2e7a9e22..fd5b9c6031 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/store/HgKVStoreImpl.java @@ -78,6 +78,11 @@ public void init(PDConfig config) { } openRocksDB(dbPath); } catch (PDException e) { + // TODO: retry the open and then fail PD startup instead of logging: a held RocksDB LOCK + // leaves this PD running uninitialized (/v1/ready answers 503 STATE_UNINITIALIZED + // while /v1/health answers 200), and with several PDs nothing restarts it. The Helm + // chart (helm/hugegraph) documents this as a limitation; drop that entry once fixed. + // https://github.com/apache/hugegraph/issues/3226 log.error("Failed to open data file,{}", e); } finally { writeLock.unlock(); diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java index 4fcf3660f5..d9104544bb 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java @@ -395,6 +395,12 @@ class StoreStatistics { * @return Returns a string indicating the service's health status. Typically, an empty * string indicates the service is healthy. */ + // TODO: answer non-200 when the raft state machine is in STATE_ERROR (a failed snapshot) or + // the KV store never opened (a held RocksDB LOCK): both leave this PD unable to recover while + // /health stays 200 forever. The Helm chart (helm/hugegraph) derives a single PD's startup + // and liveness probes to /v1/ready for this reason; drop that derivation once this reports it. + // https://github.com/apache/hugegraph/issues/3222 + // https://github.com/apache/hugegraph/issues/3226 @GetMapping(value = "/health", produces = MediaType.TEXT_PLAIN_VALUE) public Serializable checkHealthy() { return ""; diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/TaskAPI.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/TaskAPI.java index 60aad2d554..eaa53908d1 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/TaskAPI.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/TaskAPI.java @@ -89,6 +89,12 @@ public String splitPartitions() { } } + // TODO: make the task routes tell a real run from a no-op and a follower answer (a follower + // returns an empty success without doing anything), and return a body that names what was + // scheduled. Part 1 (#3233) replaced the bare 500 with a refusal body; this is part 2. The + // Helm chart (helm/hugegraph) documents the leader-first sequence and the 180 s spacing in its + // README until then. + // https://github.com/apache/hugegraph/issues/3231 @GetMapping(value = "/balanceLeaders", produces = MediaType.APPLICATION_JSON_VALUE) @ResponseBody public String balanceLeaders() { diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java index 96eb273be7..13bb05d8d9 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java @@ -75,6 +75,10 @@ public class AuthenticationFilter implements ContainerRequestFilter, ContainerRe private static final Logger LOG = Log.logger(AuthenticationFilter.class); private static final AntPathMatcher MATCHER = new AntPathMatcher(); + // TODO: add the unauthenticated /readiness probe (#3221) here once it lands. The Helm chart + // (helm/hugegraph) defaults server.readinessPath to /versions, which stays 200 with zero + // Stores, and offers /readiness as an opt-in; flip that default when this set grows. + // https://github.com/apache/hugegraph/issues/3212 private static final Set FIXED_WHITE_API_SET = ImmutableSet.of( "versions", "openapi.json" @@ -180,6 +184,11 @@ protected User authenticate(ContainerRequestContext context) { if (auth.startsWith(BASIC_AUTH_PREFIX)) { auth = auth.substring(BASIC_AUTH_PREFIX.length()); + // TODO: decode the Basic credential as UTF-8 and split it on the first colon only. + // Decoding as ASCII and splitting on every colon makes a non-ASCII password answer + // 401 and a password containing ':' answer 400, although both were accepted at + // account creation. The Helm chart (helm/hugegraph) refuses such admin passwords in + // its schema and Server wrapper; drop that guard once this is fixed. auth = new String(DatatypeConverter.parseBase64Binary(auth), Charsets.ASCII_CHARSET); String[] values = auth.split(":"); if (values.length != 2) { diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index e6ed6954a1..285c4b23a5 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -480,6 +480,11 @@ public class ServerOptions extends OptionHolder { "" ); + // TODO: accept the initial admin password from the environment (or document the properties + // contract): read through PropertiesConfiguration, the value is trimmed, backslash-unescaped + // and decoded as ISO-8859-1, so the stored password can differ from what the operator set. + // The Helm chart (helm/hugegraph) refuses padded, backslash or non-ASCII admin passwords in + // its schema and Server wrapper; relax that guard once the credential bypasses the file. public static final ConfigOption ADMIN_PA = new ConfigOption<>( "auth.admin_pa", diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index 87ff5ca6f4..62b588c911 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -1820,6 +1820,13 @@ private String defaultSpaceGraphName(String graphName) { } private void loadGraph(String name, String graphConfPath) { + // TODO: offer a loaded graph to Gremlin Server (notify GRAPH_CREATE, as the create paths + // do) or fail startup when its static Gremlin instantiation failed. Today a Server whose + // first open failed against a PD member mid-restart serves REST and passes readiness + // while every Gremlin call on it fails for the life of the process. The Helm chart + // (helm/hugegraph) detects this with a per-Pod Gremlin query in `helm test` and documents + // the manual Pod deletion; retire both once this is fixed. + // https://github.com/apache/hugegraph/issues/3228 HugeConfig config = new HugeConfig(graphConfPath); // Transfer `raft.group_peers` from server config to graph config diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java index d2d12b0f16..60e17f2b33 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManagerV2.java @@ -252,6 +252,12 @@ private void invalidateUserCache() { } private void invalidatePasswordCache(Id id) { + // TODO: invalidate the password and token caches on every Server replica, not only on the + // one that handled updateUser. AuthMetaManager.updateUser publishes no AuthEvent and + // nothing calls MetaManager.listenAuthEvent, so the other replicas on the shared PD catalog + // accept the old password until auth.cache_expire elapses. Publish a USER update event and + // listen for it here. The Helm chart (helm/hugegraph) documents the per-replica expiry as a + // limitation of admin password rotation; drop that note once fixed. this.pwdCache.invalidate(id); // Clear all tokenCache because can't get userId in it this.tokenCache.clear(); diff --git a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh index b5ba2de34f..2be225a04e 100755 --- a/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh +++ b/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh @@ -26,6 +26,13 @@ mkdir -p "${DOCKER_FOLDER}" log() { echo "[hugegraph-server-entrypoint] $*"; } +# TODO: write values so the Server reads them back unchanged. A space is escaped as "\ " here, +# which java.util.Properties would unescape, but the Server reads these files through Commons +# Configuration's PropertiesConfiguration, which keeps the backslash before a space: PASSWORD +# "a b" becomes auth.admin_pa "a\ b" and the admin login with "a b" answers 401. Emit interior +# spaces as-is (they round-trip) and add a write-then-read test through HugeConfig for the keys +# this script sets. The Helm chart (helm/hugegraph) refuses admin passwords containing spaces in +# its schema and Server wrapper; drop that guard once this is fixed. encode_prop_value() { local value="$1" encoded="" char local i @@ -146,6 +153,11 @@ if [[ -n "${PASSWORD:-}" && -z "${HG_SERVER_AUTH_TOKEN_SECRET:-}" ]]; then fi # ── Map env → properties file ───────────────────────────────────────── +# TODO: map server.urls_to_pd and server.deploy_in_k8s from the environment here, as PASSWORD is +# mapped to auth.admin_pa below. Without them the Helm chart (helm/hugegraph) rewrites +# rest-server.properties in a wrapper before exec'ing this script; drop that wrapper once both keys +# have a mapping. A read-only root filesystem also needs the conf files on a writable path, since +# set_prop in this script edits them in place; the chart refuses readOnlyRootFilesystem until then. [[ -n "${HG_SERVER_BACKEND:-}" ]] && set_prop "backend" "${HG_SERVER_BACKEND}" "${GRAPH_CONF}" [[ -n "${HG_SERVER_PD_PEERS:-}" ]] && set_prop "pd.peers" "${HG_SERVER_PD_PEERS}" "${GRAPH_CONF}" [[ -n "${HG_SERVER_USE_PD:-}" ]] && \ diff --git a/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/HgStoreEngine.java b/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/HgStoreEngine.java index eae08dfad7..03eb5fe77b 100644 --- a/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/HgStoreEngine.java +++ b/hugegraph-store/hg-store-core/src/main/java/org/apache/hugegraph/store/HgStoreEngine.java @@ -260,6 +260,12 @@ public void stateChanged(Store store, Metapb.StoreState oldState, Metapb.StoreSt * 1. Need to check the partition saved this time, delete the invalid partitions. */ public void restoreLocalPartitionEngine() { + // TODO: surface the outcome of this restore (a per-group ready signal, or a failed state + // reported to PD) instead of logging only; a Store is marked Up before this runs and a + // failed restore leaves it Up with missing shard groups. Paired with the TODO in + // StoreNodeService, which also says why this signal alone does not retire the manual + // Store rollout barrier in the Helm chart (helm/hugegraph) cluster preset. + // https://github.com/apache/hugegraph/issues/3229 try { if (!options.isFakePD()) { // FakePD mode does not require synchronization partitionManager.syncPartitionsFromPD(partition -> { diff --git a/hugegraph-store/hg-store-dist/src/assembly/static/conf/application-pd.yml b/hugegraph-store/hg-store-dist/src/assembly/static/conf/application-pd.yml index 0315c4b4fe..a4a06386d5 100644 --- a/hugegraph-store/hg-store-dist/src/assembly/static/conf/application-pd.yml +++ b/hugegraph-store/hg-store-dist/src/assembly/static/conf/application-pd.yml @@ -27,6 +27,15 @@ management: rocksdb: # rocksdb total memory usage, force flush to disk when reaching this value + # TODO: default this lower, or size it from the container memory limit, or map it from an + # HG_STORE_* variable in the Docker entrypoint. A default of 0 only makes AppConfig fall back + # to the JVM max heap, which is another heap-sized native budget, not a safe container total. + # It is the capacity of the RocksDB block and write LRU caches (RaftRocksdbOptions), not an + # upfront allocation, but pinned at 32 GB those native caches can grow past a smaller + # container memory limit under load and get the Store OOM-killed. Until then the only + # override is the JVM system property -Drocksdb.total_memory_size= (JAVA_OPTS, the + # Helm chart's store.javaOpts; helm/hugegraph). + # https://github.com/apache/hugegraph/issues/3254 total_memory_size: 32000000000 # memtable size used by rocksdb write_buffer_size: 32000000