dlmarion commented on code in PR #2632:
URL: https://github.com/apache/accumulo/pull/2632#discussion_r851364729


##########
core/src/main/java/org/apache/accumulo/core/conf/AccumuloConfiguration.java:
##########
@@ -330,6 +330,23 @@ public int getCount(Property property) {
     return Integer.parseInt(countString);
   }
 
+  /**
+   * Gets the default value of a property of type {@link PropertyType#COUNT}, 
interpreting the value
+   * properly (as an integer).
+   *
+   * @param property
+   *          property to get default value
+   * @return property default value
+   * @throws IllegalArgumentException
+   *           if the property is of the wrong type
+   */
+  public int getDefaultCount(Property property) {

Review Comment:
   This doesn't appear to be used anywhere



##########
server/tserver/src/main/java/org/apache/accumulo/tserver/ThriftClientHandler.java:
##########
@@ -686,16 +685,27 @@ private void setUpdateTablet(UpdateSession us, KeyExtent 
keyExtent) {
 
   @Override
   public void applyUpdates(TInfo tinfo, long updateID, TKeyExtent tkeyExtent,
-      List<TMutation> tmutations) {
+      List<TMutation> tmutations) throws TException {
     UpdateSession us = (UpdateSession) 
server.sessionManager.reserveSession(updateID);
     if (us == null) {
       return;
     }
 
+    ThreadPoolExecutor writeThreadExecutor = server.getWriteThreadExecutor();
     boolean reserved = true;
+
     try {
       KeyExtent keyExtent = KeyExtent.fromThrift(tkeyExtent);
-      setUpdateTablet(us, keyExtent);
+
+      // get write thread pool handler but only for user tablets
+      if (TabletType.type(keyExtent) == TabletType.USER) {
+        if (writeThreadExecutor.isTerminating()) {
+          throw new TException("Write Thread pool is terminating");
+        }
+        writeThreadExecutor.execute(() -> setUpdateTablet(us, keyExtent));

Review Comment:
   I'm not sure that there is any benefit to performing `setUpdateTablet` in 
another thread. The method either queues up the mutations for the Tablet, or it 
add them all to the failure queue. Also, I believe this has to be done before 
you get to the code on line 712 below. Otherwise there may be no 
queuedMutations.



##########
server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java:
##########
@@ -394,6 +404,12 @@ private static long jitter() {
         * TabletServer.TIME_BETWEEN_LOCATOR_CACHE_CLEARS);
   }
 
+  /**
+   * If the user has set {@link Property#TSERV_WRITE_THREADS_MAX} then return 
the semaphore that
+   * controls the number of write threads. If the user has not set the value 
then return
+   * Optional.empty(), which is essentially a no-op.
+   */

Review Comment:
   Is this comment in the right place?



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

Reply via email to