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..e4e76dd78f 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,12 @@ 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) keeps Store rollouts on OnDelete with a manual /v1/shardGroups barrier + // between Pod deletions; retire that procedure once PD reports restoration. + // 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..2e3b61bcc7 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,11 @@ 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 /v1/health, see StoreAPI.checkHealthy) can report a PD that stepped + // down for good after a snapshot failure. The Helm chart (helm/hugegraph) works around the + // missing signal with pd.livenessPath on /v1/ready for a single PD. + // 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/StandardAuthManager.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManager.java index 02f5e9b385..07c4744aaa 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManager.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/auth/StandardAuthManager.java @@ -159,6 +159,10 @@ 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; with a shared PD catalog the other replicas accept the old + // password until auth.cache_expire elapses. The Helm chart (helm/hugegraph) documents the + // per-replica expiry as a limitation of admin password rotation; drop it 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..4277f0a592 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,10 @@ if [[ -n "${PASSWORD:-}" && -z "${HG_SERVER_AUTH_TOKEN_SECRET:-}" ]]; then fi # ── Map env → properties file ───────────────────────────────────────── +# TODO: map server.urls_to_pd, server.deploy_in_k8s and auth.admin_pa from the environment here. +# Without them the Helm chart (helm/hugegraph) rewrites rest-server.properties in a wrapper before +# exec'ing this script, which is why it refuses readOnlyRootFilesystem for Server; drop that +# wrapper once these keys have an env mapping. [[ -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..97849aab0c 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,11 @@ 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; the Helm chart's manual Store roll barrier depends on it. + // 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..4bbec11a24 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,10 @@ management: rocksdb: # rocksdb total memory usage, force flush to disk when reaching this value + # TODO: make this overridable from the environment, or default it to 0 so AppConfig falls back + # to the JVM max heap: pinned at 32 GB, a Store in a container with a smaller memory limit is + # OOM-killed by native caches outside the heap. The Helm chart (helm/hugegraph) can only + # document the ceiling and size its cluster preset around it until then. total_memory_size: 32000000000 # memtable size used by rocksdb write_buffer_size: 32000000