[GitHub] [accumulo] ctubbsii commented on a change in pull request #2129: Add experimental annotation to external compaction properties, output warning on start

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

ctubbsii commented on a change in pull request #2129:
URL: https://github.com/apache/accumulo/pull/2129#discussion_r643461660



##########
File path: server/compaction-coordinator/src/main/java/org/apache/accumulo/coordinator/CoordinatorExecutable.java
##########
@@ -42,6 +42,7 @@ public String description() {
 
   @Override
   public void execute(final String[] args) throws Exception {
+    System.out.println("WARNING: External compaction processes are experimental");

Review comment:
       System.err or using the logger might be better for these.

##########
File path: core/src/main/java/org/apache/accumulo/core/conf/Property.java
##########
@@ -1022,45 +1022,63 @@
       "Amount of time for a single replication RPC call to last before failing"
           + " the attempt. See replication.work.attempts."),
   // Compactor properties
+  @Experimental
   COMPACTOR_PREFIX("compactor.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo compactor server."),
+  @Experimental
   COMPACTOR_PORTSEARCH("compactor.port.search", "true", PropertyType.BOOLEAN,
       "If the compactor.port.client is in use, search higher ports until one is available"),
+  @Experimental
   COMPACTOR_CLIENTPORT("compactor.port.client", "9101", PropertyType.PORT,
       "The port used for handling client connections on the compactor servers"),
+  @Experimental
   COMPACTOR_MINTHREADS("compactor.server.threads.minimum", "1", PropertyType.COUNT,
       "The minimum number of threads to use to handle incoming requests."),
+  @Experimental
   COMPACTOR_MINTHREADS_TIMEOUT("compactor.server.threads.timeout", "0s", PropertyType.TIMEDURATION,
       "The time after which incoming request threads terminate with no work available.  Zero (0) will keep the threads alive indefinitely."),
+  @Experimental
   COMPACTOR_THREADCHECK("compactor.server.threadcheck.time", "1s", PropertyType.TIMEDURATION,
       "The time between adjustments of the server thread pool."),
+  @Experimental
   COMPACTOR_MAX_MESSAGE_SIZE("compactor.server.message.size.max", "10M", PropertyType.BYTES,
       "The maximum size of a message that can be sent to a tablet server."),
   // CompactionCoordinator properties
+  @Experimental
   COORDINATOR_PREFIX("coordinator.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo compaction coordinator server."),
+  @Experimental
   COORDINATOR_THRIFTCLIENT_PORTSEARCH("coordinator.thrift.port.search", "false",
       PropertyType.BOOLEAN,
       "If the ports above are in use, search higher ports until one is available"),
+  @Experimental
   COORDINATOR_CLIENTPORT("coordinator.port.client", "9100", PropertyType.PORT,

Review comment:
       Ports 9100 and 9101 are registered IANA-assigned ports, and should not be used as the default port numbers for these services. Port 9100, in particular, is used by HP JetDirect printing protocol, and probably commonly referenced in enterprise firewall rules (or anywhere where somebody uses networked printers).

##########
File path: core/src/main/java/org/apache/accumulo/core/conf/Property.java
##########
@@ -1022,45 +1022,63 @@
       "Amount of time for a single replication RPC call to last before failing"
           + " the attempt. See replication.work.attempts."),
   // Compactor properties
+  @Experimental
   COMPACTOR_PREFIX("compactor.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo compactor server."),
+  @Experimental
   COMPACTOR_PORTSEARCH("compactor.port.search", "true", PropertyType.BOOLEAN,
       "If the compactor.port.client is in use, search higher ports until one is available"),
+  @Experimental
   COMPACTOR_CLIENTPORT("compactor.port.client", "9101", PropertyType.PORT,
       "The port used for handling client connections on the compactor servers"),
+  @Experimental
   COMPACTOR_MINTHREADS("compactor.server.threads.minimum", "1", PropertyType.COUNT,
       "The minimum number of threads to use to handle incoming requests."),
+  @Experimental
   COMPACTOR_MINTHREADS_TIMEOUT("compactor.server.threads.timeout", "0s", PropertyType.TIMEDURATION,
       "The time after which incoming request threads terminate with no work available.  Zero (0) will keep the threads alive indefinitely."),
+  @Experimental
   COMPACTOR_THREADCHECK("compactor.server.threadcheck.time", "1s", PropertyType.TIMEDURATION,
       "The time between adjustments of the server thread pool."),
+  @Experimental
   COMPACTOR_MAX_MESSAGE_SIZE("compactor.server.message.size.max", "10M", PropertyType.BYTES,
       "The maximum size of a message that can be sent to a tablet server."),
   // CompactionCoordinator properties
+  @Experimental
   COORDINATOR_PREFIX("coordinator.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo compaction coordinator server."),
+  @Experimental
   COORDINATOR_THRIFTCLIENT_PORTSEARCH("coordinator.thrift.port.search", "false",
       PropertyType.BOOLEAN,
       "If the ports above are in use, search higher ports until one is available"),
+  @Experimental
   COORDINATOR_CLIENTPORT("coordinator.port.client", "9100", PropertyType.PORT,
       "The port used for handling Thrift client connections on the compaction coordinator server"),
+  @Experimental
   COORDINATOR_MINTHREADS("coordinator.server.threads.minimum", "1", PropertyType.COUNT,
       "The minimum number of threads to use to handle incoming requests."),
+  @Experimental
   COORDINATOR_MINTHREADS_TIMEOUT("coordinator.server.threads.timeout", "0s",
       PropertyType.TIMEDURATION,
       "The time after which incoming request threads terminate with no work available.  Zero (0) will keep the threads alive indefinitely."),
+  @Experimental
   COORDINATOR_THREADCHECK("coordinator.server.threadcheck.time", "1s", PropertyType.TIMEDURATION,
       "The time between adjustments of the server thread pool."),
+  @Experimental
   COORDINATOR_MAX_MESSAGE_SIZE("coordinator.server.message.size.max", "10M", PropertyType.BYTES,
       "The maximum size of a message that can be sent to a tablet server."),
+  @Experimental
   COORDINATOR_DEAD_COMPACTOR_CHECK_INTERVAL("coordinator.server.compactor.dead.check.interval",
       "5m", PropertyType.TIMEDURATION, "The interval at which to check for dead compactors."),
+  @Experimental
   COORDINATOR_FINALIZER_TSERVER_NOTIFIER_MAXTHREADS("coordinator.server.finalizer.threads.maximum",
       "5", PropertyType.COUNT,
       "The maximum number of threads to use for notifying tablet servers that an external compaction has completed."),
+  @Experimental
   COORDINATOR_FINALIZER_COMPLETION_CHECK_INTERVAL("coordinator.server.finalizer.check.interval",
       "60s", PropertyType.TIMEDURATION,
       "The interval at which to check for external compaction final state markers in the metadata table."),
+  @Experimental
   COORDINATOR_TSERVER_COMPACTION_CHECK_INTERVAL(
       "coordinator.server.tserver.compaction.check.interval", "1m", PropertyType.TIMEDURATION,

Review comment:
       Is `coordinator.server` redundant, since a coordinator *is* a server?

##########
File path: core/src/main/java/org/apache/accumulo/core/conf/Property.java
##########
@@ -1022,45 +1022,63 @@
       "Amount of time for a single replication RPC call to last before failing"
           + " the attempt. See replication.work.attempts."),
   // Compactor properties
+  @Experimental
   COMPACTOR_PREFIX("compactor.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo compactor server."),
+  @Experimental
   COMPACTOR_PORTSEARCH("compactor.port.search", "true", PropertyType.BOOLEAN,

Review comment:
       The default for port search should be false to be consistent with other services that have this feature.

##########
File path: core/src/main/java/org/apache/accumulo/core/conf/Property.java
##########
@@ -1022,45 +1022,63 @@
       "Amount of time for a single replication RPC call to last before failing"
           + " the attempt. See replication.work.attempts."),
   // Compactor properties
+  @Experimental
   COMPACTOR_PREFIX("compactor.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo compactor server."),
+  @Experimental
   COMPACTOR_PORTSEARCH("compactor.port.search", "true", PropertyType.BOOLEAN,
       "If the compactor.port.client is in use, search higher ports until one is available"),
+  @Experimental
   COMPACTOR_CLIENTPORT("compactor.port.client", "9101", PropertyType.PORT,
       "The port used for handling client connections on the compactor servers"),
+  @Experimental
   COMPACTOR_MINTHREADS("compactor.server.threads.minimum", "1", PropertyType.COUNT,
       "The minimum number of threads to use to handle incoming requests."),
+  @Experimental
   COMPACTOR_MINTHREADS_TIMEOUT("compactor.server.threads.timeout", "0s", PropertyType.TIMEDURATION,
       "The time after which incoming request threads terminate with no work available.  Zero (0) will keep the threads alive indefinitely."),
+  @Experimental
   COMPACTOR_THREADCHECK("compactor.server.threadcheck.time", "1s", PropertyType.TIMEDURATION,
       "The time between adjustments of the server thread pool."),
+  @Experimental
   COMPACTOR_MAX_MESSAGE_SIZE("compactor.server.message.size.max", "10M", PropertyType.BYTES,
       "The maximum size of a message that can be sent to a tablet server."),
   // CompactionCoordinator properties
+  @Experimental
   COORDINATOR_PREFIX("coordinator.", null, PropertyType.PREFIX,
       "Properties in this category affect the behavior of the accumulo compaction coordinator server."),
+  @Experimental
   COORDINATOR_THRIFTCLIENT_PORTSEARCH("coordinator.thrift.port.search", "false",

Review comment:
       This directly exposes "thrift" as the RPC implementation. It should probably not reference thrift.




-- 
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