[GitHub] [accumulo] FineAndDandy commented on a diff in pull request #3221: Do not calculate split point in Tablet.needsSplit()

["FineAndDandy (via GitHub)" <gi...@apache.org>]

FineAndDandy commented on code in PR #3221:
URL: https://github.com/apache/accumulo/pull/3221#discussion_r1124951615


##########
server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java:
##########
@@ -1814,7 +1824,7 @@ List<FileRef> findChopFiles(KeyExtent extent, Map<FileRef,Pair<Key,Key>> firstAn
   public synchronized boolean needsSplit() {
     if (isClosing() || isClosed())
       return false;
-    return findSplitRow(getDatafileManager().getFiles()) != null;
+    return isSplitPossible();

Review Comment:
   This call will block for as long as it takes to calculate the new split point. Although no longer in a synchronized block, and will prevent that split point from being calculated more than once. The initial calculation is still going to block the call that triggered the operation. Especially for bulk loads, this can cause bulk load failures that would have otherwise been successes due to timeouts. Since the split happens in another thread, is it that negative to do this there asynchronously from the original triggering call?



-- 
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: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org