Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 2638: Compaction Limits #2670

Merged
merged 3 commits into from
Apr 6, 2021

Conversation

klwilson227
Copy link
Contributor

This resolves issue 2638, by allowing a customer to set a limit on the duration of the major and minor compaction runs. This allows the customer to balance the needs for compaction against the need for normal garbage collection. Unbounded execution of compaction can starve the garbage collection process, and may lead to extended periods of high disk IO. By setting a upper bound on the execution duration for the garbage collection, we can ensure a balance is maintained between both GC and Compaction processes.

Logging has been improved to allow a user to see the complete usage of the buckets, vs. those that were compacted, so that they can decide on any further tuning that may be required.

Test cases have been added to ensure limits are enforced properly and that defaulting of the values is correctly implemented.

conf/bk_server.conf Outdated Show resolved Hide resolved
conf/bk_server.conf Outdated Show resolved Hide resolved
Comment on lines +463 to +464
long end = start;
long timeDiff = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the end and the timeDiff here? Seems we can only break the following loop if (currentTime-start) >= limit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timediff is held for reporting after loop exit when debugging is enabled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see.

}
if (meta.getUsage() >= threshold || (limit > 0 && timeDiff > limit) || !running) {
// We allow the usage limit calculation to continue so that we get a accurate
// report of where the usage was prior to running compaction.
continue;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should break the loop if exceed the max compaction time? all the remaining logs will be skipped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The consumer, would not get any indication of how may logs were not compacted if we skip out of loop. This ensures that the exit report of the usage buckets give a complete picture of what was there and what was compacted. The usage gathering from what I saw as extremely fast compared to the compaction. The logs show that this does not take a appreciable amount of time. However, a consumer whom is tuning the system may need this insight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants