pivotal-jbarrett commented on a change in pull request #6037:
URL: https://github.com/apache/geode/pull/6037#discussion_r582342689



##########
File path: 
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
##########
@@ -1231,23 +1197,16 @@ public void shutdownMessageReceived(ID id, String 
reason) {
     if (logger.isDebugEnabled()) {
       logger.debug("Membership: recording shutdown status of {}", id);
     }
-    synchronized (this.shutdownMembers) {
-      this.shutdownMembers.put(id, id);
-      services.getHealthMonitor()
-          .memberShutdown(id, reason);
-      services.getJoinLeave().memberShutdown(id, reason);
-    }
+    this.shutdownMembers.add(id);
+    services.getJoinLeave().memberShutdown(id, reason);
   }
 
   @Override
   public Set<ID> getMembersNotShuttingDown() {
-    latestViewReadLock.lock();
-    try {
-      return latestView.getMembers().stream().filter(id -> 
!shutdownMembers.containsKey(id))
+    synchronized (shutdownMembers) {

Review comment:
       If the list isn't updated frequently even a long list with copy on write 
would be better less intrusive than frequently grabbing a read lock. If this 
method is called for every p2p message we certainly can't have it 
synchronizing, it should do the least amount of blocking as possible.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to