ctubbsii commented on a change in pull request #2213:
URL: https://github.com/apache/accumulo/pull/2213#discussion_r676796020
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactableImpl.java
##########
@@ -214,6 +202,9 @@ public CompactableImpl(Tablet tablet, CompactionManager
manager,
}
return Set.copyOf(servicesIds);
}, 2, TimeUnit.SECONDS);
+
+ this.fileMgr = new
TabletCompactionFileManager(Collections.unmodifiableSet(runningJobs),
+ tablet.getExtent(), extCompactingFiles, extSelInfo);
Review comment:
This creates an unmodifiable view over `runningJobs`. However,
`runningJobs` can still be modified in the below synchronized methods and this
view doesn't protect against concurrent reads over the set, which could lead to
`ConcurrentModificationException`s while streaming/iterating the set in the new
`TabletCompactionFileManager` class. I think instead of using a `HashSet` and
synchronize methods, you could use `ConcurrentHashMap.newKeySet()` for the
`runningJobs` field. You can still wrap it with unmodifiable to ensure
`TabletCompactionFileManager` can't alter the set.
--
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]
