Updates DefaultCompactionPlanner to honor table.file.max prop#4127
Updates DefaultCompactionPlanner to honor table.file.max prop#4127keith-turner merged 4 commits intoapache:2.1from
Conversation
This change updates the DefaultCompactionPlanner to honor the table.file.max property. When a tablet has more files than is configured in table.file.max and its not compacting, it will try to find a compaction ratio that will cause a compaction. Using a lower compaction ratio will find files of a similar size to compact. This avoids the problem with deprecated DefautlCompactionStrategy where it may select a 1K and 1G file for compaction because it selected the N smallest files that would bring the tablet under the table.file.max limit.
| maxTabletFiles); | ||
| } | ||
|
|
||
| log.trace( |
There was a problem hiding this comment.
This may be better if logged at info, maybe even warn?
The case for info would be to make sure it is showing up in the logs so that it can be traced why the compactions are occurring when they do not meet the configured compaction ration.
The case for warn to would be to call attention that the current compaction configuration may not be adequate - maybe they would want to raise the number of files, increase the allowed file size, or something else?
There was a problem hiding this comment.
This may be better if logged at info, maybe even warn?
I kept going back and forth on what to log this at. I did have it at debug, then switched it trace. It probably should be debug, info, or warn. I suppose if its too chatty we could consider modifying it then.
There was a problem hiding this comment.
Changed it to info. Was thinking warn suited the case where its above table.file.max and nothing is being found.
| * <p> | ||
| * Starting with Accumulo 2.1.3, this plugin will use the table config option | ||
| * {@code "table.file.max"}. When a tablet has no compactions running, its number of files exceeds | ||
| * table.file.max, and system compactions are not finding anything to compact then this plugin will | ||
| * try to find a lower compaction ratio that will result in a compaction. For example if the | ||
| * compaction ratio is set to 3, a tablet has 20 files, table.file.max is 15 and no compactions are | ||
| * planned, then this plugin will find the largest compaction ratio less than 3 that results in a | ||
| * compaction. | ||
| * </p> | ||
| * |
There was a problem hiding this comment.
The wording is a bit awkward, suggest changing it to make the criteria clearer.
This is a rough draft suggestion. I'd recommend changing formatting as needed.
| * <p> | |
| * Starting with Accumulo 2.1.3, this plugin will use the table config option | |
| * {@code "table.file.max"}. When a tablet has no compactions running, its number of files exceeds | |
| * table.file.max, and system compactions are not finding anything to compact then this plugin will | |
| * try to find a lower compaction ratio that will result in a compaction. For example if the | |
| * compaction ratio is set to 3, a tablet has 20 files, table.file.max is 15 and no compactions are | |
| * planned, then this plugin will find the largest compaction ratio less than 3 that results in a | |
| * compaction. | |
| * </p> | |
| * | |
| * <p> | |
| * Starting with Accumulo 2.1.3, this plugin will use the table config option | |
| * {@code "table.file.max"}. | |
| * When the following three conditions are met, then this plugin will try to find a lower compaction ratio that will result in a compaction: | |
| * 1. When a tablet has no compactions running | |
| * 2. Its number of files exceed table.file.max | |
| * 3. System compactions are not finding anything to compact. | |
| * For example, given a tablet with 20 files, and table.file.max is 15 and no compactions are planned. If the compaction ratio is set to 3, then this plugin will find the largest compaction ratio less than 3 that results in a | |
| * compaction. | |
| * </p> | |
| * |
There was a problem hiding this comment.
Thats easier to understand. I updated the javadoc, made it use HTML for the list and added a 4th condition that the code was checking for.
ddanielr
left a comment
ddanielr
left a comment
There was a problem hiding this comment.
Minor suggestion about documentation restructuring but overall these changes look good.
Since this PR does change compactions, should this change be included in 2.1.3 or should it wait for 2.1.4?
This increases the value of table.file.max.prop to 1001, which is the value already used by testErrorDuringUserCompaction, so that the tests get the expected values
|
This looks like it has caused a couple test failures in 2.1 and main for |
5 participants
This change updates the DefaultCompactionPlanner to honor the table.file.max property. When a tablet has more files than is configured in table.file.max and its not compacting, it will try to find a compaction ratio that will cause a compaction.
Using a lower compaction ratio will find files of a similar size to compact. This avoids the problem with deprecated DefautlCompactionStrategy where it may select a 1K and 1G file for compaction because it selected the N smallest files that would bring the tablet under the table.file.max limit.