bitflicker64 opened a new pull request, #3138:
URL: https://github.com/apache/hugegraph/pull/3138
## Purpose of the PR
- Part of #3137 (phase 1 / "PR 1" of the roadmap discussed there); the issue
stays open for phases 2 and 3.
In distributed mode (PD + HStore), `POST /graphspaces/{space}/graphs/{name}`
returns 200 before the creating server has actually bound the new graph into
its embedded Gremlin server. `GraphManager.createGraph` (PD path) fires
`GRAPH_CREATE` via `this.eventHub.notify(...)`, which is asynchronous, so the
REST response can be written before `ContextGremlinServer` injects the graph
and its `TraversalSource` into the Gremlin global bindings. An immediate
follow-up Gremlin/Cypher request to the same server then fails with HTTP 400:
`Could not rebind [g] to [__g_<name>] as [__g_<name>] could not be found in the
Graph or TraversalSource global bindings`.
The local path already handles this correctly: `createGraphLocal` calls
`this.notifyAndWaitEvent(Events.GRAPH_CREATE, graph)`, which blocks on the
event future before returning. The PD path simply lost that parity.
## Main Changes
- One-line change in `GraphManager.createGraph` (PD path): replace
`this.eventHub.notify(Events.GRAPH_CREATE, graph)` with
`this.notifyAndWaitEvent(Events.GRAPH_CREATE, graph)`, matching what
`createGraphLocal` already does.
- Effect: when the create call returns 200, the CREATING server is
guaranteed to have completed its `GRAPH_CREATE` listeners (Gremlin bindings
injection included). Cross-replica convergence (other servers picking the graph
up via the PD/meta watch) is intentionally out of scope here; that is phase 2/3
of #3137.
One question for reviewers, as raised in #3137: `notifyAndWaitEvent`
(GraphManager.java, around line 1774) waits on the event future but swallows
listener failures with only a `LOG.warn`, so a failed bindings injection would
still return 200. Should a listener failure fail the create instead? I left
that behavior unchanged here to keep this PR a pure parity fix with
`createGraphLocal`.
## Verifying these changes
- [ ] Trivial rework / code cleanup without any test coverage. (No Need)
- [ ] Already covered by existing tests, such as *(please modify tests
here)*.
- [x] Need tests and can be verified as follows:
- This is a timing race that needs a multi-server distributed deployment
(PD + HStore), so no new unit test is added. Manual reproduction from #3137:
run two hugegraph-server instances behind a load balancer in distributed mode,
create a graph via the REST API, then immediately send a Gremlin query for that
graph pinned to the creating server. Before this change the query can fail with
400 "Could not rebind [g] to [__g_...]"; after it, requests hitting the
creating server succeed as soon as the create returns 200.
## Does this PR potentially affect the following parts?
- [ ] Dependencies ([add/update
license](https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses)
info &
[regenerate_known_dependencies.sh](../install-dist/scripts/dependency/regenerate_known_dependencies.sh))
- [ ] Modify configurations
- [ ] The public API
- [ ] Other affects (typed here)
- [x] Nope
## Documentation Status
- [ ] `Doc - TODO`
- [ ] `Doc - Done`
- [x] `Doc - No Need`
--
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]
