keith-turner commented on code in PR #3479:
URL: https://github.com/apache/accumulo/pull/3479#discussion_r1224783610
##########
server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java:
##########
@@ -149,16 +149,13 @@ private static class TabletLists {
private final List<TabletLocationState> suspendedToGoneServers = new
ArrayList<>();
private final Map<KeyExtent,UnassignedTablet> unassigned = new HashMap<>();
private final Map<TServerInstance,List<Path>> logsForDeadServers = new
TreeMap<>();
- // read only lists of tablet servers
- private final SortedMap<TServerInstance,TabletServerStatus>
currentTServers;
- private final SortedMap<TServerInstance,TabletServerStatus> destinations;
+ // read only list of tablet servers that are not shutting down
+ private final SortedMap<TServerInstance,TabletServerStatus>
validAssignmentDestinations;
public TabletLists(Manager m,
SortedMap<TServerInstance,TabletServerStatus> curTServers) {
var destinationsMod = new TreeMap<>(curTServers);
- // Don't move tablets to servers that are shutting down
destinationsMod.keySet().removeAll(m.serversToShutdown);
Review Comment:
This is a pre-existing problem, not something introduced in this PR. It
could be fixed in this PR, but does not have to be. I noticed that
serversToShutdown is a syncronized set and the removeAll method may iterator
over it. The following is how the Collections javadoc recommend iterating over
synchronized sets.
```suggestion
synchronized(m.serversToShutdown) {
destinationsMod.keySet().removeAll(m.serversToShutdown);
}
```
EDIT from @ctubbsii to fix formatting, so this won't break formatting if
merged.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
