imbajin commented on code in PR #3008:
URL: https://github.com/apache/hugegraph/pull/3008#discussion_r3523792138
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/ServerInfoManager.java:
##########
@@ -198,16 +206,12 @@ public synchronized void heartbeat() {
LOG.info("ServerInfo is missing: {}, may not be initialized yet",
this.selfNodeId());
return;
}
- if (this.selfIsMaster()) {
- // On the master node, just wait for ServerInfo re-init
- LOG.warn("ServerInfo is missing: {}, may be cleared before",
this.selfNodeId());
- return;
- }
/*
- * Missing server info on non-master node, may be caused by graph
- * truncated on master node then synced by raft.
- * TODO: we just patch it here currently, to be improved.
+ * Missing server info on any node (master or worker), may be caused by
+ * graph truncated or store data cleared.
+ * Re-save ServerInfo to recover automatically.
*/
+ LOG.warn("ServerInfo is missing: {}, re-saving it now",
this.selfNodeId());
serverInfo = this.saveServerInfo(this.selfNodeId(),
this.selfNodeRole());
Review Comment:
`heartbeat()` now re-saves missing ServerInfo for master nodes too, and the
latest hbase job is failing on that path. The log shows `ServerInfo is missing:
DEFAULT-hugegraph/server-1, re-saving it now`, then
`GremlinApiTest.testClearAndInit` hits `Already existed master
'DEFAULT-hugegraph/server-1' in current cluster`, followed by `decreaseLoad()`
NPEs because self ServerInfo is still missing for `DEFAULT-hugegraph/server1`.
Please make current-node master re-registration idempotent, or skip only the
normalized current node in the uniqueness scan, and guard `decreaseLoad()` when
self ServerInfo is unavailable.
##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/auth/ManagerAPI.java:
##########
@@ -259,6 +262,45 @@ public String getRolesInGs(@Context GraphManager manager,
result));
}
+ @GET
+ @Timed
+ @Path("default")
+ public String checkDefaultRole(@Context GraphManager manager,
+ @PathParam("graphspace") String graphSpace,
+ @QueryParam("role") String role,
+ @QueryParam("graph") String graph) {
+ LOG.debug("check if current user is default role: {} {} {}",
+ role, graphSpace, graph);
+ ensurePdModeEnabled(manager);
+ AuthManager authManager = manager.authManager();
+ String user = HugeGraphAuthProxy.username();
+
+ E.checkArgument(StringUtils.isNotEmpty(role) &&
+ StringUtils.isNotEmpty(graphSpace),
+ "Must pass graphspace and role params");
+
+ HugeDefaultRole defaultRole;
+ try {
+ defaultRole = HugeDefaultRole.valueOf(role.toUpperCase());
+ } catch (IllegalArgumentException e) {
+ E.checkArgument(false, "Invalid role value '%s'", role);
+ defaultRole = null; // unreachable, satisfies compiler
+ }
+ boolean hasGraph = defaultRole.equals(HugeDefaultRole.OBSERVER);
+ E.checkArgument(!hasGraph || StringUtils.isNotEmpty(graph),
+ "Must set a graph for observer");
+
+ boolean result;
+ if (hasGraph) {
+ result = authManager.isDefaultRole(graphSpace, graph, user,
Review Comment:
This endpoint checks raw default-role metadata without first validating that
the path graphspace exists, and for `OBSERVER` it only checks that `graph` is
non-empty. A typo or stale graph name can therefore return a metadata result
instead of a clear 4xx. Please mirror `GraphSpaceAPI`: validate the graphspace
before the auth lookup, and for observer roles verify that the graph exists in
that graphspace before calling `isDefaultRole()`.
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ApiTestSuite.java:
##########
@@ -44,6 +44,9 @@
GraphSpaceApiTest.class,
GraphSpaceApiStandaloneTest.class,
ManagerApiStandaloneTest.class,
+ GraphsApiStandaloneTest.class,
Review Comment:
Adding the backend-specific suites here does not make them run in CI as
written. The new standalone and hstore tests gate on
`System.getProperty("backend")`, but `run-api-test.sh` invokes Maven as `mvn
... -P api-test,$BACKEND`, and the surefire config does not pass the Maven
`backend` profile property into the test JVM. That leaves `backend` null, so
`assumeStandaloneMode()` skips the standalone suite and `GraphSpaceApiTest`
skips as non-hstore. Please pass `backend` into surefire, for example with
`systemPropertyVariables`, or call Maven with `-Dbackend=$BACKEND`.
--
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]
