You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hugegraph.apache.org by "haohao0103 (via GitHub)" <gi...@apache.org> on 2024/08/06 05:42:39 UTC

[PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

haohao0103 opened a new pull request, #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620

   close #2618 
   
   Define initialization identifiers for storeEventListeners in AbstractBackendStoreProvider to ensure that storeEventListeners are not registered repeatedly


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "zyxxoo (via GitHub)" <gi...@apache.org>.
zyxxoo commented on PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#issuecomment-2291283548

   thanks for you push code for solve the problem, this is a great code, but i have some different advise for that. ohh, the transaction monitor some event happen, meawhile, the code change the state local, every instance maybe need care the event, so  we should allow repeated event register; for the problem, i think the main problem maybe is the threadlocal not correct release, that make the listen not be remove 


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1714787317


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java:
##########
@@ -44,6 +45,8 @@ public abstract class AbstractBackendStoreProvider
     private final EventHub storeEventHub = new EventHub("store");
 
     protected Map<String, BackendStore> stores = null;
+    private static volatile Boolean schemaCacheClearListened = Boolean.FALSE;

Review Comment:
   Is `boolean` enough? 



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "haohao0103 (via GitHub)" <gi...@apache.org>.
haohao0103 commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1713504301


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java:
##########
@@ -67,7 +70,10 @@ protected final void checkOpened() {
 
     @Override
     public void listen(EventListener listener) {
-        this.storeEventHub.listen(EventHub.ANY_EVENT, listener);
+        if (schemaCacheClearListened.compareAndSet(false, true) ||
+            vertexEdgeCacheClearListened.compareAndSet(false, true)) {

Review Comment:
   1、define 2 automic_param boolean A & B;this operation is to ensure that storeEventListener does not deliver to storeEventHub of storeProvider once in each thread. I understand that there is no need to re register during the entire Hugegraph lifecycle, so there is no reset operation; Will this miss other necessary scenarios?
   2、schemaCacheClearListened for schemaCache clear , vertexEdgeCacheClearListened for vertex&edge cache;
   3、 I have modified it to use the volatile+synchronized,thanks



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server): avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r2020135487


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java:
##########
@@ -70,6 +72,30 @@ public void listen(EventListener listener) {
         this.storeEventHub.listen(EventHub.ANY_EVENT, listener);
     }
 
+    @Override
+    public void listenSchemaCacheClear(EventListener listener) {
+        if (!schemaCacheClearListened) {
+            synchronized (this) {

Review Comment:
   Using an instance-level lock with static volatile flags may lead to race conditions if multiple instances call listenSchemaCacheClear concurrently; consider synchronizing on the class (e.g., synchronized(AbstractBackendStoreProvider.class)) to ensure global exclusivity.
   ```suggestion
               synchronized (AbstractBackendStoreProvider.class) {
   ```



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java:
##########
@@ -70,6 +72,30 @@ public void listen(EventListener listener) {
         this.storeEventHub.listen(EventHub.ANY_EVENT, listener);
     }
 
+    @Override
+    public void listenSchemaCacheClear(EventListener listener) {
+        if (!schemaCacheClearListened) {
+            synchronized (this) {
+                if (!schemaCacheClearListened) {
+                    listen(listener);
+                    schemaCacheClearListened = true;
+                }
+            }
+        }
+    }
+
+    @Override
+    public void listenDataCacheClear(EventListener listener) {
+        if (!vertexEdgeCacheClearListened) {
+            synchronized (this) {

Review Comment:
   Using an instance-level lock with static volatile flags may lead to race conditions in listenDataCacheClear; consider synchronizing on the class (e.g., synchronized(AbstractBackendStoreProvider.class)) to ensure that the flag update is thread-safe across all instances.
   ```suggestion
               synchronized (AbstractBackendStoreProvider.class) {
   ```



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "VGalaxies (via GitHub)" <gi...@apache.org>.
VGalaxies commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1720878997


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedSchemaTransactionV2.java:
##########
@@ -105,7 +105,8 @@ private void listenChanges() {
             }
             return false;
         };
-        this.graphParams().loadGraphStore().provider().listen(this.storeEventListener);
+        this.graphParams().loadGraphStore().provider()
+            .listenSchemaCacheClear(this.storeEventListener);
 
         // Listen cache event: "cache"(invalid cache item)
         this.cacheEventListener = event -> {

Review Comment:
   CachedSchemaTransactionV2 is the CachedSchemaTransaction corresponding to the SchemaTransaction of the pd-store version. We added a V2 to distinguish it from the previous version. Due to the inconsistency of the parameters required for construction, it is currently not easy to reuse, will be merged later...



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "haohao0103 (via GitHub)" <gi...@apache.org>.
haohao0103 commented on PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#issuecomment-2295577048

   > thanks for you push code for solve the problem, this is a great code, but i have some different advise for that. ohh, the transaction monitor some event happen, meawhile, the code change the state local, every instance maybe need care the event, so we should allow repeated event register; for the problem, i think the main problem maybe is the threadlocal not correct release, that make the listen not be remove
   
   thanks , i try to understand  what you mean , Indeed, there some issues with threadlocal processing,and my code did not address this problem. In addition ,I thind it is indeed valuable to pay attention to events for each transaction instance; But in the current sceario,based on my understanding,this operation seems redundant because schema and vertex/edge cache are both global sinleton, and it seems unnecessay to execute their operations on every transaction instance. of course ,if there are personalized scenarios that allow for reregistration of listener,it is indeed necessary and valuable.this is my idea, welcome to correct it.


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "haohao0103 (via GitHub)" <gi...@apache.org>.
haohao0103 commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1721156289


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/BackendStoreProvider.java:
##########
@@ -67,6 +67,12 @@ public interface BackendStoreProvider {
 
     void listen(EventListener listener);
 
+    default void listenSchemaCacheClear(EventListener listener) {

Review Comment:
   thanks ,  you mean to update the method name to  listenSchemaCacheAtMostOnce?



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1712520066


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java:
##########
@@ -67,7 +70,10 @@ protected final void checkOpened() {
 
     @Override
     public void listen(EventListener listener) {
-        this.storeEventHub.listen(EventHub.ANY_EVENT, listener);
+        if (schemaCacheClearListened.compareAndSet(false, true) ||
+            vertexEdgeCacheClearListened.compareAndSet(false, true)) {

Review Comment:
   Some question:
   - we define 2 automic_param boolean A & B and try to set them to `true`, when to reset it?
   - why we need 2 flag to mark it? (in this context)
   - since **assignment operations** (`=`) for **primitive types** are atomic by default(64bit), it seems that we only need to use a `volatile` boolean flag to achieve the desired effect?
   
   @Pengzna @VGalaxies



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java:
##########
@@ -67,7 +70,10 @@ protected final void checkOpened() {
 
     @Override
     public void listen(EventListener listener) {
-        this.storeEventHub.listen(EventHub.ANY_EVENT, listener);
+        if (schemaCacheClearListened.compareAndSet(false, true) ||
+            vertexEdgeCacheClearListened.compareAndSet(false, true)) {

Review Comment:
   Some questions:
   - we define 2 automic_param boolean A & B and try to set them to `true`, when to reset it?
   - why we need 2 flag to mark it? (in this context)
   - since **assignment operations** (`=`) for **primitive types** are atomic by default(64bit), it seems that we only need to use a `volatile` boolean flag to achieve the desired effect?
   
   @Pengzna @VGalaxies



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#issuecomment-2285736488

   ## [Codecov](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2620?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   Attention: Patch coverage is `80.95238%` with `4 lines` in your changes missing coverage. Please review.
   > Project coverage is 34.64%. Comparing base [(`a038d23`)](https://app.codecov.io/gh/apache/incubator-hugegraph/commit/a038d2341cc58cb66af34e270d6134b56e54389f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`ceba46c`)](https://app.codecov.io/gh/apache/incubator-hugegraph/commit/ceba46ce8e83f7a8d65e0486b83e90fe70cb9ee3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   
   | [Files](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2620?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [...ph/backend/store/AbstractBackendStoreProvider.java](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2620?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Fbackend%2Fstore%2FAbstractBackendStoreProvider.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL2JhY2tlbmQvc3RvcmUvQWJzdHJhY3RCYWNrZW5kU3RvcmVQcm92aWRlci5qYXZh) | 87.50% | [0 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2620?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   | [.../hugegraph/backend/store/BackendStoreProvider.java](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2620?src=pr&el=tree&filepath=hugegraph-server%2Fhugegraph-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fhugegraph%2Fbackend%2Fstore%2FBackendStoreProvider.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aHVnZWdyYXBoLXNlcnZlci9odWdlZ3JhcGgtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVnZWdyYXBoL2JhY2tlbmQvc3RvcmUvQmFja2VuZFN0b3JlUHJvdmlkZXIuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2620?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   
   > :exclamation:  There is a different number of reports uploaded between BASE (a038d23) and HEAD (ceba46c). Click for more details.
   > 
   > <details><summary>HEAD has 4 uploads less than BASE</summary>
   >
   >| Flag | BASE (a038d23) | HEAD (ceba46c) |
   >|------|------|------|
   >||5|1|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #2620       +/-   ##
   =============================================
   - Coverage     45.17%   34.64%   -10.53%     
   + Complexity      583      510       -73     
   =============================================
     Files           718      729       +11     
     Lines         58434    59520     +1086     
     Branches       7491     7660      +169     
   =============================================
   - Hits          26396    20622     -5774     
   - Misses        29318    36628     +7310     
   + Partials       2720     2270      -450     
   ```
   
   </details>
   
   [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/incubator-hugegraph/pull/2620?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server): avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2620: fix(server): avoid duplicate registration of StoreEventListener
URL: https://github.com/apache/incubator-hugegraph/pull/2620


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "haohao0103 (via GitHub)" <gi...@apache.org>.
haohao0103 commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1721162754


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java:
##########
@@ -133,7 +133,7 @@ private void listenChanges() {
             }
             return false;
         };
-        this.store().provider().listen(this.storeEventListener);
+        this.store().provider().listenDataCacheClear(this.storeEventListener);

Review Comment:
   simply put,i found that registering a listener for each transaction instance is redundant because each transaction receives events and then performs a clear operation on the schema/vertex/edge cache , i think that this is redundant.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #2620: fix(server) Avoid duplicate registration of StoreEventListener
URL: https://github.com/apache/incubator-hugegraph/pull/2620


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "imbajin (via GitHub)" <gi...@apache.org>.
imbajin commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1717293165


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java:
##########
@@ -20,6 +20,7 @@
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicBoolean;

Review Comment:
   ```suggestion
   ```



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "javeme (via GitHub)" <gi...@apache.org>.
javeme commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1720600241


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/BackendStoreProvider.java:
##########
@@ -67,6 +67,12 @@ public interface BackendStoreProvider {
 
     void listen(EventListener listener);
 
+    default void listenSchemaCacheClear(EventListener listener) {
+    }
+
+    default void listenDataCacheClear(EventListener listener) {

Review Comment:
   prefer listenGraphCacheAtMostOnce



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedSchemaTransactionV2.java:
##########
@@ -105,7 +105,8 @@ private void listenChanges() {
             }
             return false;
         };
-        this.graphParams().loadGraphStore().provider().listen(this.storeEventListener);
+        this.graphParams().loadGraphStore().provider()
+            .listenSchemaCacheClear(this.storeEventListener);
 
         // Listen cache event: "cache"(invalid cache item)
         this.cacheEventListener = event -> {

Review Comment:
   @imbajin can you share some context why we added CachedSchemaTransactionV2.java? not sure it's used in what scenarios, and can we merge V1+V2 into one class?



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/BackendStoreProvider.java:
##########
@@ -67,6 +67,12 @@ public interface BackendStoreProvider {
 
     void listen(EventListener listener);
 
+    default void listenSchemaCacheClear(EventListener listener) {

Review Comment:
   prefer listenSchemaCacheAtMostOnce



##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java:
##########
@@ -133,7 +133,7 @@ private void listenChanges() {
             }
             return false;
         };
-        this.store().provider().listen(this.storeEventListener);
+        this.store().provider().listenDataCacheClear(this.storeEventListener);

Review Comment:
   Sorry I have read the issue, but I still don't quite understand in what scenarios.the problem is encountered.



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "javeme (via GitHub)" <gi...@apache.org>.
javeme commented on code in PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#discussion_r1729810920


##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/cache/CachedGraphTransaction.java:
##########
@@ -133,7 +133,7 @@ private void listenChanges() {
             }
             return false;
         };
-        this.store().provider().listen(this.storeEventListener);
+        this.store().provider().listenDataCacheClear(this.storeEventListener);

Review Comment:
   Note each provider has multiple listeners, which is our design intention, because each listener wants to get notifications.
   This modification looks a bit special at the moment, and we will propose a solution after we figure out the problem. Could you post the error stack if there is an error?
   



-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org


Re: [PR] fix(server) Avoid duplicate registration of StoreEventListener [incubator-hugegraph]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #2620:
URL: https://github.com/apache/incubator-hugegraph/pull/2620#issuecomment-2369380677

   Due to the lack of activity, the current pr is marked as stale and will be closed after 180 days, any update will remove the stale label


-- 
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: issues-unsubscribe@hugegraph.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@hugegraph.apache.org
For additional commands, e-mail: issues-help@hugegraph.apache.org