[GitHub] [accumulo] keith-turner commented on a change in pull request #1632: Fix #1609 dynamically reinitialize compaction services when config ch…

[GitBox <gi...@apache.org>]

keith-turner commented on a change in pull request #1632:
URL: https://github.com/apache/accumulo/pull/1632#discussion_r454059617



##########
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:
       I am using Map.copyOf() for the maps, so its a volatile pointer to an immutable map.   Whenever its updated the entire map is replaced with another immutable map.  This pattern is good for maps that are concurrently read a lot and updated infrequently.  The internal pointers in concurrent hash map are volatile, making all operations on the map slower.  The internal pointers in an immutable map are probably not volatile, just our pointer to the entire map is.




----------------------------------------------------------------
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:
users@infra.apache.org