Bill commented on a change in pull request #6037:
URL: https://github.com/apache/geode/pull/6037#discussion_r579391765
##########
File path:
geode-membership/src/main/java/org/apache/geode/distributed/internal/membership/gms/GMSMembership.java
##########
@@ -273,6 +273,9 @@ boolean isDistributionMessage() {
/**
* Members that have sent a shutdown message. This is used to suppress
suspect processing that
* otherwise becomes pretty aggressive when a member is shutting down.
+ *
+ * Accesses to this map should be synchronized on the map to avoid concurrent
+ * modification exceptions
Review comment:
Since we only ever `shutdownMembers.put(id, id)` this can be a
`Set<ID>`. It needn't be a map.
Furthermore, if we made it a concurrent map-based set, we could eliminate
all the synchronization below.
This declaration could become:
```
private final Set<ID> shutdownMembers = ConcurrentHashMap.newKeySet();
```
Then code below like this:
```
synchronized (shutdownMembers) {
if (!shutdownMembers.containsKey(dm)) {
// if we've received a shutdown message then DistributionManager
will already have
// notified listeners
listener.memberDeparted(dm, crashed, reason);
}
}
```
changes to this:
```
if (!shutdownMembers.contains(dm)) {
// if we've received a shutdown message then DistributionManager
will already have
// notified listeners
listener.memberDeparted(dm, crashed, reason);
}
```
and code below like this:
```
synchronized (shutdownMembers) {
return latestView.getMembers().stream().filter(id ->
!shutdownMembers.containsKey(id))
.collect(
Collectors.toSet());
}
```
changes to code like this:
```
final Set<ID> shutdownMembersStable = this.shutdownMembers;
return latestView.getMembers().stream().filter(id ->
!shutdownMembersStable.contains(id))
.collect(
Collectors.toSet());
```
or maybe:
```
final Set<ID> membersNotShuttingDown = new
HashSet<ID>(latestView.getMembers());
final Set<ID> shutdownMembersStable = this.shutdownMembers;
membersNotShuttingDown.removeIf(shutdownMembersStable::contains);
return membersNotShuttingDown;
```
----------------------------------------------------------------
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]
