Copilot commented on code in PR #3017:
URL: https://github.com/apache/hugegraph/pull/3017#discussion_r3208728386
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java:
##########
@@ -184,17 +185,29 @@ private void listenChanges() {
}
return false;
};
- if (graphCacheListenStatus.putIfAbsent(this.params().spaceGraphName(),
true) == null) {
- EventHub graphEventHub = this.params().graphEventHub();
+ EventHub graphEventHub = this.params().graphEventHub();
+ EventListener previous =
+ graphCacheEventListeners.putIfAbsent(
+ this.params().spaceGraphName(), listener);
+ if (previous == null) {
+ this.cacheEventListener = listener;
+ this.registeredCacheEventListener = true;
graphEventHub.listen(Events.CACHE, this.cacheEventListener);
+ } else {
+ this.cacheEventListener = previous;
+ this.registeredCacheEventListener = false;
}
}
private void unlistenChanges() {
String graphName = this.params().spaceGraphName();
- if (graphCacheListenStatus.remove(graphName) != null) {
+ if (this.registeredCacheEventListener) {
EventHub graphEventHub = this.params().graphEventHub();
- graphEventHub.unlisten(Events.CACHE, this.cacheEventListener);
+ if (graphCacheEventListeners.remove(graphName,
+ this.cacheEventListener)) {
+ graphEventHub.unlisten(Events.CACHE, this.cacheEventListener);
+ }
+ this.registeredCacheEventListener = false;
Review Comment:
The cache listener registry is global/static (graphCacheEventListeners), but
this teardown removes the shared listener as soon as the *registering*
transaction instance closes. If multiple CachedGraphTransaction instances exist
concurrently for the same graph (e.g., different threads), closing the original
“owner” will unregister the only hub listener and other live transactions will
stop receiving cache invalidation/clear events. Consider tracking a per-graph
ref-count (or a holder object) and only unlistening/removing when the last
transaction for that graph is closed, and keep map removal + hub unlisten
atomic to avoid gaps/duplicates under concurrent open/close.
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedSchemaTransaction.java:
##########
@@ -143,17 +156,24 @@ private void unlistenChanges() {
// Unlisten cache event
EventHub schemaEventHub = this.params().schemaEventHub();
- schemaEventHub.unlisten(Events.CACHE, this.cacheEventListener);
+ if (this.registeredCacheEventListener) {
+ schemaEventHub.unlisten(Events.CACHE, this.cacheEventListener);
+ SCHEMA_CACHE_EVENT_LISTENERS.remove(this.params().spaceGraphName(),
+ this.cacheEventListener);
+ this.registeredCacheEventListener = false;
+ }
Review Comment:
SCHEMA_CACHE_EVENT_LISTENERS is a static per-graph registry, but this close
path unregisters/removes the shared listener whenever the original registering
CachedSchemaTransaction instance closes. With multiple schema transactions
alive concurrently for the same graph (e.g., across threads), closing the
“owner” can drop the hub listener and other live transactions will stop
receiving cache events. Consider using a per-graph ref-count (or holder) and
only unlisten/remove when the last transaction closes, and ensure the map
update + hub unlisten happen atomically to avoid a window where the map points
at an unregistered listener or where two listeners are temporarily registered.
--
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]
