jchen21 commented on code in PR #7628:
URL: https://github.com/apache/geode/pull/7628#discussion_r860271939


##########
geode-core/src/main/java/org/apache/geode/internal/cache/FilterProfile.java:
##########
@@ -1846,18 +1846,27 @@ protected void process(ClusterDistributionManager dm) {
         }
 
         CacheDistributionAdvisor cda = (CacheDistributionAdvisor) 
r.getDistributionAdvisor();
-        CacheDistributionAdvisor.CacheProfile cp =
-            (CacheDistributionAdvisor.CacheProfile) 
cda.getProfile(getSender());
-        if (cp == null) { // PR accessors do not keep filter profiles around
-          if (logger.isDebugEnabled()) {
-            logger.debug(
-                "No cache profile to update, adding filter profile message to 
queue. Message :{}",
-                this);
+        CacheDistributionAdvisor.CacheProfile cp;
+
+        // prevent adding the message to queue after we have processed the 
queue
+        // in CreateRegionReplyProcessor.process
+        synchronized (cda) {

Review Comment:
   It is not clear to me how `synchronize (cda)` can achieve your solution:
   >When we are at step 5, synchronize on the advisor so that 5 finishes before 
6 or completely after 6.
   
   I don't see `CacheDistributionAdvisor` is synchronized on 
`CreateRegionReplyProcessor`. Also my understanding is that on the receiver 
side, there is only one thread that processes this single instance of 
`OperationMessage`. Maybe I am missing something. Can you tell me what are the 
threads that compete for the lock of `cda` here?
   And for the synchronized block, why it does not include the case when cp != 
null?



-- 
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: notifications-unsubscr...@geode.apache.org

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

Reply via email to