imbajin commented on PR #3011:
URL: https://github.com/apache/hugegraph/pull/3011#issuecomment-4377640539
Thanks for the update. The current direction looks much better scoped than
the earlier `Userdata` normalization idea.
From my reading, the current PR mechanism is:
- keep the local `EventHub` schema-cache listener for same-JVM cache events
- add one JVM-wide `MetaManager` listener for cross-server
schema-cache-clear events
- publish graph-scoped schema-cache-clear events on schema add/remove
- include a per-JVM `source` id so the publisher can skip its own watch echo
- clear all V2 schema cache layers on remote JVMs: `schema-id`,
`schema-name`, and the array attachment cache
- continue accepting legacy plain-string event values from older publishers
Could you please update the PR description/comment with a compact before ->
after diagram? I think that will make the design much easier to review,
especially since most of the added code is test coverage.
Suggested structure:
```text
Before
+----------+ schema add/remove +---------------+
| Server A | ----------------------------> | HStore / Meta |
+----------+ +---------------+
|
| local V2 cache updated
v
+-------------------+
| Server A cache |
| fresh |
+-------------------+
+-------------------+
| Server B cache |
| stale |
+-------------------+
Problem:
Server B had no effective cross-JVM V2 schema-cache-clear listener, so it
could
keep serving stale schema from local cache.
```
```text
After
+----------+ schema add/remove +---------------+
| Server A | ----------------------------> | HStore / Meta |
| source A | +---------------+
+----------+ |
| |
| publish schema-cache-clear |
| {"graph":"DEFAULT-g","source":"A"} |
v v
+-------------------+ +-------------------+
| Server A listener | | Server B listener |
| source == A | | source != B |
| skip self echo | | clear V2 caches |
+-------------------+ +-------------------+
|
v
+---------------------+
| schema-id cache |
| schema-name cache |
| array attachment |
+---------------------+
```
A few points I would like to confirm before merge:
1. `resetMetaListenerForReconnect()` does not seem to be wired to any actual
`MetaManager` / `MetaDriver` reconnect callback.
If there is an existing call path, could you point it out? If not, please
adjust the comment so it is clear this is only a future/manual recovery hook,
not a complete reconnect recovery mechanism in this PR.
2. The `~create_time` String/Date round-trip should stay out of this PR, but
please mention it as a follow-up.
The self-echo skip avoids forcing the publisher to immediately reload its
freshly created schema from backend, but remote nodes can still reload schema
after cache invalidation. So the backend userdata serialization issue still
exists; it is just outside the current PR scope.
3. Please add a short compatibility note.
Something like:
> This PR does not change persisted schema data. It only changes the
runtime schema-cache-clear meta-event value so new events can carry a per-JVM
`source` id. New consumers still accept legacy plain-string graph names.
Overall, the design now looks reasonable to me. The main remaining thing is
to make the lifecycle and compatibility boundaries explicit so future readers
do not assume the reconnect and `~create_time` issues are fully solved here.
--
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]
