Increase majc priority for tablets over file size threshold #5026
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CompactionManager.mainLoop runs in the TabletServer looking for tablets that need to be compacted. It ends up calling CompactionService.submitCompaction at some interval for each hosted Tablet. CompactionService.getCompactionPlan will make a CompactionPlan for the tablet and log a warning if no CompactionPlan is created but the number of files is larger than TSERV_SCAN_MAX_OPENFILES. When there are no compactions running for the tablet and no plan is calculated, then DefaultCompactionPlanner.makePlan takes into account TABLE_FILE_MAX and TSERV_SCAN_MAX_OPENFILES and will create a system compaction that considers all of the files and calculates which ones need to be compacted to get below the limit. Finally, a priority is calculated by calling CompactionJobPrioritizer.createPriority. However, given that this compaction is a SYSTEM compaction it will have a lower priority than all current USER compactions and it's priority will still be based on the total number of files. Given that the TABLE_FILE_MAX is per-table it's possible to have two tablets from different tables and the tablet that is over the size threshold has a lower priority than the tablet that is not over the size threshold. This change modifies CompactionJobPrioritizer.createPriority to take into account whether or not the tablet is over the threshold to give it a higher priority. Closes apache#4610
dlmarion
commented
Oct 31, 2024
| // Given that tablets with too many files cause several problems, | ||
| // boost their priority to the maximum allowed value. | ||
| if (condition == Condition.TABLET_OVER_SIZE) { | ||
| return Short.MAX_VALUE; |
There was a problem hiding this comment.
Could possibly increment a metric for this condition, although I'm not sure how useful it would be. I think knowing that all of the external compactors and compaction threads in the tservers are fully utilized, along with a backlog of waiting compactions, is a more useful indication that more capacity is needed.
There was a problem hiding this comment.
In your changes to the monitor its now scanning the metadata table periodically. In that scan it could compute the number of tablets over the max per table and display that on the monitor.
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Outdated
Show resolved
Hide resolved
|
May still want to leave #4610 open after merging this. This change is nice in that it helps compactions run faster for tablets over the threshold. There is still the problem that scan servers cache the file list. So if a scan server caches the list of files of a tablet w/ too many files for 5 mins then that tablet can not be scanned for that 5 min period even if it does compact. Not sure of the best way to handle this, scan servers could possibly poll the tablets files a bit more frequently when a tablet is in this condition. |
keith-turner
reviewed
Nov 4, 2024
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/spi/compaction/CompactionPlanner.java
Show resolved
Hide resolved
|
@keith-turner - I think I addressed all of your suggestions in the latest commit. |
ctubbsii
reviewed
Nov 6, 2024
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Outdated
Show resolved
Hide resolved
keith-turner
reviewed
Nov 8, 2024
| Range<Short> range = null; | ||
| Function<Range<Short>,Short> func = normalPriorityFunction; | ||
| if (Namespace.ACCUMULO.id() == nsId) { | ||
| // Handle system tables |
There was a problem hiding this comment.
There could be chop compactions when merging the metadata table and these may end w/ a null range causing an exception. Need to add handling for kind of chop. Will have to drop this in the 3.1 code.
There was a problem hiding this comment.
In 74d66b2 I added code to treat CHOP as USER compaction kind.
core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Show resolved
Hide resolved
core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java
Outdated
Show resolved
Hide resolved
keith-turner
approved these changes
Nov 8, 2024
core/src/test/java/org/apache/accumulo/core/util/compaction/CompactionPrioritizerTest.java
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/util/compaction/CompactionJobPrioritizer.java
Show resolved
Hide resolved
dlmarion
added a commit
to dlmarion/accumulo
that referenced
this pull request
Nov 12, 2024
The test was timing out because it never started running compactions created by the test method. Instead, the compactor process was running compactions created by the previous test method because the previous test created a table with a lot of files, started a user compaction, then cancelled the user compaction. The recent changes in apache#5026 caused a bunch of system compactions to be generated for the table. The two test methods share the same compaction queue, so the compactor was busy running the system compactions. To fix this issue I backported a property added in apache#3955 that makes the compactor cancel check method time configurable and I deleted the table in the test method that created a lot of files. Closes apache#5052
dlmarion
added a commit
that referenced
this pull request
Nov 14, 2024
The test was timing out because it never started running compactions created by the test method. Instead, the compactor process was running compactions created by the previous test method because the previous test created a table with a lot of files, started a user compaction, then cancelled the user compaction. The recent changes in #5026 caused a bunch of system compactions to be generated for the table. The two test methods share the same compaction queue, so the compactor was busy running the system compactions. To fix this issue I backported a property added in #3955 that makes the compactor cancel check method time configurable and I deleted the table in the test method that created a lot of files. Closes #5052
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CompactionManager.mainLoop runs in the TabletServer looking for tablets that need to be compacted. It ends up calling CompactionService.submitCompaction at some interval for each hosted Tablet. CompactionService.getCompactionPlan will make a CompactionPlan for the tablet and log a warning if no CompactionPlan is created but the number of files is larger than TSERV_SCAN_MAX_OPENFILES. When there are no compactions running for the tablet and no plan is calculated, then DefaultCompactionPlanner.makePlan takes into account TABLE_FILE_MAX and TSERV_SCAN_MAX_OPENFILES and will create a system compaction that considers all of the files and calculates which ones need to be compacted to get below the limit. Finally, a priority is calculated by calling CompactionJobPrioritizer.createPriority. However, given that this compaction is a SYSTEM compaction it will have a lower priority than all current USER compactions and it's priority will still be based on the total number of files. Given that the TABLE_FILE_MAX is per-table it's possible to have two tablets from different tables and the tablet that is over the size threshold has a lower priority than the tablet that is not over the size threshold. This change modifies
CompactionJobPrioritizer.createPriority to take into account whether or not the tablet is over the threshold to give it a higher priority.
Closes #4610