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


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -1353,7 +1353,7 @@ public HugeGraph createGraph(String graphSpace, String 
name, String creator,
         }
 
         // Let gremlin server and rest server context add graph
-        this.eventHub.notify(Events.GRAPH_CREATE, graph);
+        this.notifyAndWaitEvent(Events.GRAPH_CREATE, graph);

Review Comment:
   Right, fixed in 9a4ac02e. `EventHub.notify` resolves with the count of 
listeners that returned normally, so `notifyAndWaitEvent` now compares that 
against the registered listener count and fails the create when one is missing.
   
   Rolling back `addGraphConfig` / `notifyGraphAdd` would let this replica's 
binding failure delete a graph the others bound fine, so the graph is instead 
bound locally before it is published to meta; the failure path unregisters and 
closes it, like a failed backend init.
   
   The count is an inference, not the real exception. Happy to switch to 
verifying the bindings directly, or to having `EventHub` surface listener 
failures, if you prefer either.
   



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -1353,7 +1353,7 @@ public HugeGraph createGraph(String graphSpace, String 
name, String creator,
         }
 
         // Let gremlin server and rest server context add graph
-        this.eventHub.notify(Events.GRAPH_CREATE, graph);
+        this.notifyAndWaitEvent(Events.GRAPH_CREATE, graph);

Review Comment:
   Agreed, fixed in 9a4ac02e: bounded `future.get(30s)`, and 
`InterruptedException` is caught on its own and restores the interrupt status 
before failing. The drop path gets the same bounded wait but stays lenient, 
since the data is already gone when the event fires and TinkerPop's 
`removeGraph` throws for a graph the Gremlin server never bound.
   
   30s is a constant rather than an option, as `ServerOptions` has nothing 
comparable; happy to promote it if you want it tunable.
   
   One call I would rather leave to you: a timeout currently fails the create, 
so a merely slow listener breaks a create that actually worked. Treating a 
timeout as unknown (log loudly, do not fail) is the alternative.
   



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