milleruntime commented on a change in pull request #1632:
URL: https://github.com/apache/accumulo/pull/1632#discussion_r454347182
##########
File path:
server/tserver/src/main/java/org/apache/accumulo/tserver/compactions/CompactionManager.java
##########
@@ -20,34 +20,89 @@
import java.util.HashMap;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.TimeUnit;
+import org.apache.accumulo.core.conf.AccumuloConfiguration;
import org.apache.accumulo.core.conf.Property;
import org.apache.accumulo.core.dataImpl.KeyExtent;
import org.apache.accumulo.core.spi.compaction.CompactionKind;
import org.apache.accumulo.core.spi.compaction.CompactionServiceId;
import org.apache.accumulo.core.spi.compaction.CompactionServices;
-import org.apache.accumulo.core.spi.compaction.DefaultCompactionPlanner;
import org.apache.accumulo.core.util.NamingThreadFactory;
import org.apache.accumulo.fate.util.Retry;
import org.apache.accumulo.server.ServerContext;
-import org.apache.accumulo.tserver.TabletServerResourceManager;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Sets;
+
public class CompactionManager {
private static final Logger log =
LoggerFactory.getLogger(CompactionManager.class);
private Iterable<Compactable> compactables;
- private Map<CompactionServiceId,CompactionService> services;
+ private volatile Map<CompactionServiceId,CompactionService> services;
Review comment:
Thanks for the explanation. I think what you have is fine. I was just
wondering what is the best way to guard against future changes which could
introduce concurrency issues. For example, you used a coding pattern which may
not be obvious to a new developer, versus using a concrete type that is less
likely to change. Your code is definitely more efficient but it was not
initially clear to me why. Perhaps just some comments with your explanation or
creating a method to specifically update the map would help.
----------------------------------------------------------------
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:
us...@infra.apache.org
