fix(server): wait for GRAPH_CREATE event when creating graph on PD path

[bitflicker64]

Purpose

This is Phase 1 of #3137. It closes the local race on the Server that handles graph creation; cluster-wide readiness remains a follow-up.

In distributed mode (PD + HStore), graph creation previously returned HTTP 200 after scheduling GRAPH_CREATE, but before the embedded Gremlin Server had necessarily registered the graph and its TraversalSource. An immediate Gremlin or Cypher request to the same Server could therefore fail with:

Could not rebind [g] to [__g_<name>]

Before and after

Before: create graph -> schedule GRAPH_CREATE -> publish/return 200 -> bind later
After:  create graph -> bind locally and verify listeners -> publish -> return 200

After this PR, HTTP 200 guarantees that the creating Server has completed its local REST and Gremlin registrations, including the __g_<space>-<graph> traversal binding.

Changes

• Add synchronous EventHub.notifySync() dispatch while preserving the existing asynchronous notify() behavior for other callers.
• Report attempted and successful listeners from the same notification snapshot, avoiding false create results if listeners change concurrently.
• Run graph-create listeners and the standalone local drop listener synchronously because they define the request's local success boundary.
• Fail graph creation when any registered GRAPH_CREATE listener fails.
• On local binding failure, clear the local graph and remove partial REST/Gremlin registrations; make Gremlin removal safe when only part of the binding was installed.
• Publish the graph to PD metadata only after local binding succeeds.
• Carry graph identity through REST and Gremlin cleanup, and serialize local Gremlin registration/removal so a stale rollback cannot remove a replacement binding.

Scope: intentionally not handled here

• Other Server replicas may still be loading the graph after 200. Requests routed to another replica can still need retry or sticky routing. Cluster-wide LOADING / READY / FAILED reporting is Phase 2 of [Feature] Orchestrate graph creation through PD: new graphs are not consistently available across Server replicas ("Could not rebind [g]") #3137.
• Graph creation ownership is not moved into PD; that architectural change remains Phase 3 of [Feature] Orchestrate graph creation through PD: new graphs are not consistently available across Server replicas ("Could not rebind [g]") #3137.
• Failures after local binding during PD metadata publication or prepareSchema are not made transactional here; they belong to the Phase 3 PD creation state machine.
• The distributed graph-drop path is unchanged.
• Full serialization of concurrent create/drop operations for the same graph name is not introduced; that belongs to the broader graph lifecycle state-machine work.
• Requests racing with Server shutdown are outside this local create-readiness guarantee.
• Listener deadlines and cancellation are unchanged; lifecycle listeners are expected to remain local and non-blocking.

Verification

• EventHubTest: 21 tests passed on JDK 11, including synchronous dispatch, listener-failure counting, and snapshot mutation.
• hugegraph-api and all required reactor modules compiled successfully on JDK 11.
• The original distributed reproduction remains: create a graph and immediately query it on the creating Server. That Server must no longer return 200 before its Gremlin binding exists.

Does this PR potentially affect the following parts?

• Dependencies
• Modify configurations
• Public API contract
• Internal graph lifecycle handling

Documentation Status

• Doc - No Need

[codecov]

Codecov Report

❌ Patch coverage is 64.00000% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 34.66%. Comparing base (1716c77) to head (92997d8).
⚠️ Report is 3 commits behind head on master.

Files with missing lines	Patch %	Lines
...n/java/org/apache/hugegraph/core/GraphManager.java	66.66%	13 Missing and 1 partial ⚠️
...rg/apache/hugegraph/auth/ContextGremlinServer.java	50.00%	1 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3138      +/-   ##
============================================
- Coverage     39.18%   34.66%   -4.52%     
- Complexity      264      498     +234     
============================================
  Files           770      782      +12     
  Lines         65779    67088    +1309     
  Branches       8726     8956     +230     
============================================
- Hits          25774    23256    -2518     
- Misses        37244    41230    +3986     
+ Partials       2761     2602     -159     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[imbajin]

Blocking: yes. Summary: The PD create path now waits for GRAPH_CREATE listeners, but listener failures remain non-fatal and the current head has a failing codecov/project check. Evidence: EventHub.notify() catches listener Throwable at hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/event/EventHub.java:200-205; final gate reports codecov/project FAILURE.

[imbajin]

⚠️ Making this path synchronous also introduces an unbounded request wait: notifyAndWaitEvent() calls future.get() without a timeout at GraphManager.java:1777, and its catch(Throwable) swallows InterruptedException without restoring the interrupt status. A hung listener or shutdown can now hold the create request indefinitely, or return before registration after interruption. Please use a bounded wait and handle InterruptedException explicitly.

[bitflicker64]

Agreed, fixed in 9a4ac02: 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.

[bitflicker64]

Both points addressed in 9a4ac02, details in the inline replies.

Two things left out on purpose:

• The PD drop path (GraphManager.java:2153) still fires GRAPH_DROP without waiting, and passes the graph after close(). Same class of bug, pre-existing, on the drop side. Glad to fold it in if you want create and drop symmetric here.
• Creation is still publish-then-finish overall, since prepareSchema runs after the config is in meta. That is the rollback risk in [Feature] Orchestrate graph creation through PD: new graphs are not consistently available across Server replicas ("Could not rebind [g]") #3137 that phase 3 is meant to close.

There is no automated coverage for the new failure semantics: nothing exercises notifyAndWaitEvent today, and the race itself needs a multi-server deployment. A unit test registering a deliberately throwing listener on a scratch EventHub would cover the count check and the cleanup cheaply, if you want it here.

codecov/project looks like a base-report artifact: it reports +11 files and +1150 lines against base, which this diff cannot produce, while codecov/patch passes.

[imbajin]

Blocking: yes. The Phase 1 boundary is appropriate: this PR only needs to guarantee that the creating Server completes its local graph and TraversalSource bindings before returning 200; cross-replica readiness remains in #3137. The happy path now meets that scope, but the two local rollback races below can still leave stale bindings after a failed create. Codecov is intentionally ignored here.

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Ensures graph creation in distributed mode doesn’t return success before the local server has completed GRAPH_CREATE listeners (notably Gremlin bindings injection), preventing immediate follow-up Gremlin/Cypher requests from failing.

Changes:

• Introduces a synchronous event notification path (EventHub.notifySync) and uses it during graph create to run listeners before publishing graph metadata.
• Adds stricter listener-failure detection in GraphManager during GRAPH_CREATE, with rollback/cleanup on failure.
• Makes Gremlin bindings removal more defensive by avoiding removals when the binding doesn’t exist.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java	Synchronous GRAPH_CREATE notification + failure detection and rollback/cleanup adjustments
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/ContextGremlinServer.java	Defensive removal of graph and traversal source bindings
hugegraph-commons/hugegraph-common/src/test/java/org/apache/hugegraph/unit/event/EventHubTest.java	Adds unit test for new notifySync() behavior
hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/event/EventHub.java	Adds notifySync() and refactors listener iteration/notification logic

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[imbajin]

Re-reviewed current head 5569400b2 against the Phase 1 scope. No blocking findings.

• Graph creation now completes and validates local REST/Gremlin registration before publishing or returning success.
• Failed creation rolls back partial bindings and local caches; Gremlin registration/removal is synchronized and identity-guarded.
• Existing asynchronous EventHub.notify() behavior remains unchanged for other callers.
• The PR description clearly separates the creating-server guarantee from cross-replica readiness, PD lifecycle state, distributed drop, and full same-name lifecycle serialization.

Verification: EventHubTest passes 20/20 on JDK 11, and the hugegraph-api reactor compiles successfully on JDK 11. The remaining lack of a direct GraphManager/Gremlin integration test is a non-blocking coverage gap.

[imbajin]

Re-reviewed current head 92997d893 with six independent adversarial lanes. No in-scope blocking findings.

The listener outcome check now derives attempted and successful counts from the same COW snapshot, while the existing asynchronous EventHub API retains its behavior. Local binding failure rollback, identity guards, and synchronized Gremlin registration/removal remain sound.

Verification: EventHubTest passes 21/21 on JDK 11 and the hugegraph-api reactor compiles successfully. Residual risks are explicitly scoped: no direct GraphManager/Gremlin integration test, no listener deadline, and post-bind PD metadata transactions remain follow-up work.

[bitflicker64]

@copilot-pull-request-reviewer[bot] Validated the final head 92997d893. The synchronous path counts attempted and successful listeners from the same dispatch iterator; failed local bindings are rolled back through synchronous, identity-guarded Gremlin cleanup before the graph is removed and closed. The existing asynchronous notify() API remains unchanged. The review-comments API exposes no Copilot inline findings to address individually despite the summary reporting five. EventHubTest passes on Java 11 with -DskipCommonsTests=false; the intentional throwing-listener warning is covered by that test.

[imbajin]

Blocking: yes. The local GRAPH_CREATE synchronization is sound, but PD graph creation still publishes local state before later metadata and schema steps that can fail without a unified rollback; the current head also has a failing codecov/project check. Evidence: GraphManager.java:1353-1393; gh pr checks -R apache/hugegraph 3138.

[imbajin]

‼️ graphs.put() and the synchronous GRAPH_CREATE notification run before addGraphConfig, notifyGraphAdd, updateGraphSpaceConfig, and schema initialization. If any later step throws, this method exits without removing the local graph/bindings, closing and unregistering the graph, or compensating metadata already written, so the failed request can leave a graph that blocks retry and can be published to other servers. Please extend one rollback boundary across the publish/schema phase, with best-effort local binding and HugeFactory cleanup plus compensation for partially written PD metadata.

[imbajin]

@bitflicker64 Confirmed. This is a real follow-up gap: after local GRAPH_CREATE succeeds, a failure in PD metadata publication or schema initialization can still leave local graph/bindings and partially written metadata behind.

For the current PR, we are keeping this outside the minimal Phase 1 boundary (the creating Server's local bindings must be ready before 200). Please track it under #3137 as the Phase 3 graph-creation state-machine work, including compensating metadata cleanup and one rollback boundary across publication/schema initialization.

Marking this as a follow-up for now; I am leaving the thread unresolved until that work is addressed.