You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "keith-turner (via GitHub)" <gi...@apache.org> on 2024/11/16 02:12:33 UTC

[PR] avoids uneeded work when splitting tablets [accumulo]

keith-turner opened a new pull request, #5071:
URL: https://github.com/apache/accumulo/pull/5071

   When tablets needed to split they were queued in a thread pool to seed a fate operation.  When the tablet group watcher ran again if the tablet was still in queue it would add it again. This would not cause any correctness issues, but it would cause a lot of wasted work. Also if one tablet was queued multiple times it could prevent another tablet that was not queued at all from being added.  This made splitting very when a large number of tablets needed to split very inefficient.


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


Re: [PR] avoids uneeded work when splitting tablets [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #5071:
URL: https://github.com/apache/accumulo/pull/5071


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


Re: [PR] avoids uneeded work when splitting tablets [accumulo]

Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on code in PR #5071:
URL: https://github.com/apache/accumulo/pull/5071#discussion_r1845234382


##########
server/manager/src/main/java/org/apache/accumulo/manager/split/Splitter.java:
##########
@@ -191,6 +186,29 @@ public FileInfo getCachedFileInfo(TableId tableId, TabletFile tabletFile) {
   }
 
   public void initiateSplit(SeedSplitTask seedSplitTask) {
-    splitExecutor.execute(seedSplitTask);
+    // Want to avoid queuing the same tablet multiple times, it would not cause bugs but would waste
+    // work. Use the metadata row to identify a tablet because the KeyExtent also includes the prev
+    // end row which may change when splits happen. The metaRow is conceptually tableId+endRow and
+    // that does not change for a split.
+    Text metaRow = seedSplitTask.getExtent().toMetaRow();
+    int qsize = queuedTablets.size();
+    if (qsize < 10_000 && queuedTablets.add(metaRow)) {

Review Comment:
   This is not something that needs to be changed now as it was already hard coded, but I am wondering if there would ever be a use case requiring tuning the size of max number of queued tablets that would make creating a property for this worthwhile



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


Re: [PR] avoids uneeded work when splitting tablets [accumulo]

Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5071:
URL: https://github.com/apache/accumulo/pull/5071#discussion_r1845256398


##########
server/manager/src/main/java/org/apache/accumulo/manager/split/Splitter.java:
##########
@@ -191,6 +186,29 @@ public FileInfo getCachedFileInfo(TableId tableId, TabletFile tabletFile) {
   }
 
   public void initiateSplit(SeedSplitTask seedSplitTask) {
-    splitExecutor.execute(seedSplitTask);
+    // Want to avoid queuing the same tablet multiple times, it would not cause bugs but would waste
+    // work. Use the metadata row to identify a tablet because the KeyExtent also includes the prev
+    // end row which may change when splits happen. The metaRow is conceptually tableId+endRow and
+    // that does not change for a split.
+    Text metaRow = seedSplitTask.getExtent().toMetaRow();
+    int qsize = queuedTablets.size();
+    if (qsize < 10_000 && queuedTablets.add(metaRow)) {

Review Comment:
   Not sure if will need to be configurable, would like to wait for a need before making it configurable.  Seems like if it was configurable it would need metrics or log messages that help when changing the config.



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