Skip to content

fix(server):fix graph server cache notifier mechanism#2729

Merged
simon824 merged 3 commits intoapache:masterfrom
haohao0103:alan-graph-server-notifier-fix
Apr 2, 2025
Merged

fix(server):fix graph server cache notifier mechanism#2729
simon824 merged 3 commits intoapache:masterfrom
haohao0103:alan-graph-server-notifier-fix

Conversation

@haohao0103
Copy link
Contributor

@haohao0103 haohao0103 commented Feb 13, 2025

fix #2728

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. bug Something isn't working labels Feb 13, 2025
@imbajin imbajin requested a review from javeme February 13, 2025 11:25
@imbajin imbajin requested a review from Copilot March 30, 2025 10:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes the graph server cache notifier mechanism by introducing a controlled registration of cache listeners via static maps and enhancing logging during cache event processing.

  • Introduces static ConcurrentHashMaps (graphCacheListenStatus and storeEventListenStatus) for managing listener registration.
  • Updates listener registration and unregistration in CachedGraphTransaction using the new maps.
  • Enhances logging in the AbstractCacheNotifier in StandardHugeGraph for better traceability.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
hugegraph-core/src/main/java/org/apache/hugegraph/backend/tx/GraphTransaction.java Added static maps for tracking cache listener registration.
hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java Updated cache and store event listener registration/unregistration using static maps.
hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java Enhanced logging within the cache notifier for improved debugging.
Comments suppressed due to low confidence (1)

hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java:194

  • Using static ConcurrentHashMaps to track listener registration may cause unintended unregistrations if multiple CachedGraphTransaction instances share the same graph name. Consider implementing a reference counting mechanism or a more granular registry to ensure that an active listener is not removed prematurely.
String graphName = this.params().name();

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Apr 1, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 1, 2025
@simon824 simon824 merged commit c99cfcd into apache:master Apr 2, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] graph server cache notifier mechanism is not working properly

4 participants