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 2025/05/28 21:52:29 UTC
[PR] Looks ahead more than once in compaction planning. [accumulo]
keith-turner opened a new pull request, #5588:
URL: https://github.com/apache/accumulo/pull/5588
When planning compactions for a tablet that currently has compactions running, then planning process will estimate what the size of the file the running compactions would produce and see if its more optimal to wait for these. When doing this it only looks ahead once. This change makes the planning process look ahead more for files to compact.
--
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] Looks ahead more than once in compaction planning. [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5588:
URL: https://github.com/apache/accumulo/pull/5588#discussion_r2114253258
##########
core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java:
##########
@@ -307,6 +309,19 @@ public CompactionPlan makePlan(PlanningParameters params) {
group = findDataFilesToCompact(filesCopy, params.getRatio(), maxFilesToCompact,
maxSizeToCompact);
+ while (!group.isEmpty() && !Collections.disjoint(group, expectedFiles)) {
+ // remove these files as compaction candidates because they include a file that a running
+ // compaction would produce
+ filesCopy.removeAll(group);
+ // Create a fake file+size entry that predicts what this projected compaction would
+ // produce
+ var futureFile = getExpectedFile(group, nextExpected);
+ Preconditions.checkState(expectedFiles.add(futureFile));
Review Comment:
A message might be useful here for the log
##########
core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java:
##########
@@ -307,6 +309,19 @@ public CompactionPlan makePlan(PlanningParameters params) {
group = findDataFilesToCompact(filesCopy, params.getRatio(), maxFilesToCompact,
maxSizeToCompact);
+ while (!group.isEmpty() && !Collections.disjoint(group, expectedFiles)) {
+ // remove these files as compaction candidates because they include a file that a running
+ // compaction would produce
+ filesCopy.removeAll(group);
+ // Create a fake file+size entry that predicts what this projected compaction would
+ // produce
+ var futureFile = getExpectedFile(group, nextExpected);
+ Preconditions.checkState(expectedFiles.add(futureFile));
+ // look for any compaction work in the remaining set of files
+ group = findDataFilesToCompact(filesCopy, params.getRatio(), maxFilesToCompact,
+ maxSizeToCompact);
+ }
+
Review Comment:
You have a higher probability of `group` being empty at the end of this loop with these changes. Is that the intended effect?
--
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] Looks ahead more than once in compaction planning. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5588:
URL: https://github.com/apache/accumulo/pull/5588#discussion_r2116705387
##########
core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java:
##########
@@ -307,6 +309,19 @@ public CompactionPlan makePlan(PlanningParameters params) {
group = findDataFilesToCompact(filesCopy, params.getRatio(), maxFilesToCompact,
maxSizeToCompact);
+ while (!group.isEmpty() && !Collections.disjoint(group, expectedFiles)) {
+ // remove these files as compaction candidates because they include a file that a running
+ // compaction would produce
+ filesCopy.removeAll(group);
+ // Create a fake file+size entry that predicts what this projected compaction would
+ // produce
+ var futureFile = getExpectedFile(group, nextExpected);
+ Preconditions.checkState(expectedFiles.add(futureFile));
+ // look for any compaction work in the remaining set of files
+ group = findDataFilesToCompact(filesCopy, params.getRatio(), maxFilesToCompact,
+ maxSizeToCompact);
+ }
+
Review Comment:
Yeah in most cases it probably will end up being empty. When its empty it means no work was found that meets the compaction ratio. When its not empty and exits the loop it means work was found that meets the compaction ratio and would not use the result of a running compaction. This second case is more likely to occur when a tablet has built up a lot of files and has running compactions, so this will probably not be the norm.
--
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] Looks ahead more than once in compaction planning. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5588:
URL: https://github.com/apache/accumulo/pull/5588#discussion_r2116705387
##########
core/src/main/java/org/apache/accumulo/core/spi/compaction/DefaultCompactionPlanner.java:
##########
@@ -307,6 +309,19 @@ public CompactionPlan makePlan(PlanningParameters params) {
group = findDataFilesToCompact(filesCopy, params.getRatio(), maxFilesToCompact,
maxSizeToCompact);
+ while (!group.isEmpty() && !Collections.disjoint(group, expectedFiles)) {
+ // remove these files as compaction candidates because they include a file that a running
+ // compaction would produce
+ filesCopy.removeAll(group);
+ // Create a fake file+size entry that predicts what this projected compaction would
+ // produce
+ var futureFile = getExpectedFile(group, nextExpected);
+ Preconditions.checkState(expectedFiles.add(futureFile));
+ // look for any compaction work in the remaining set of files
+ group = findDataFilesToCompact(filesCopy, params.getRatio(), maxFilesToCompact,
+ maxSizeToCompact);
+ }
+
Review Comment:
Yeah in most cases it probably will end up being empty. When its not empty and exits the loop it means work was found that meets the compaction ratio and would not use the result of a running compaction. This second case is more likely to occur when a tablet has built up a lot of files and has running compactions, so this will probably not be the norm.
--
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] Looks ahead more than once in compaction planning. [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner merged PR #5588:
URL: https://github.com/apache/accumulo/pull/5588
--
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