-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Introduce bottom-pri thread pool for large universal compactions #2580
Conversation
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@ajkr updated the pull request - view changes - changes since last import |
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
ping @siying, zippydb still wants this by end of July, AFAIK. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job. Only one comment.
db/db_impl_compaction_flush.cc
Outdated
@@ -1603,6 +1641,28 @@ Status DBImpl::BackgroundCompaction(bool* made_progress, | |||
|
|||
// Clear Instrument | |||
ThreadStatusUtil::ResetThreadStatus(); | |||
} else if (env_->GetBackgroundThreads(Env::Priority::BOTTOM) > 0 && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put this Env function call as the last condition? There is uncertainty about how expensive this call can be, because Env is pluggable. If we only check it if it is a full compaction in universal, we call it much infrequently.
3fcb282
to
59d5ac0
Compare
@ajkr updated the pull request - view changes - changes since last import |
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@ajkr updated the pull request - view changes - changes since last import |
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@ajkr updated the pull request - view changes - changes since last import |
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
When we had a single thread pool for compactions, a thread could be busy for a long time (minutes) executing a compaction involving the bottom level. In multi-instance setups, the entire thread pool could be consumed by such bottom-level compactions. Then, top-level compactions (e.g., a few L0 files) would be blocked for a long time ("head-of-line blocking"). Such top-level compactions are critical to prevent compaction stalls as they can quickly reduce number of L0 files / sorted runs.
This diff introduces a bottom-priority queue for universal compactions including the bottom level. This alleviates the head-of-line blocking situation for fast, top-level compactions.
Env::Priority::BOTTOM
thread pool. This feature is only enabled if user explicitly configures it to have a positive number of threads.ThreadPoolImpl
's default thread limit from one to zero. This change is invisible to users as we callIncBackgroundThreadsIfNeeded
on the low-pri/high-pri pools duringDB::Open
with values of at least one. It is necessary, though, for bottom-pri to start with zero threads so the feature is disabled by default.ManualCompaction
into two parts inPrepickedCompaction
.PrepickedCompaction
is used for any compaction that's picked outside of its execution thread, either manual or automatic.BGWorkBottomCompaction
).bg_bottom_compaction_scheduled_
so we can wait for bottom-level compactions to finish. We don't count them against the background jobs limits. So users of this feature will get an extra compaction for free.Test Plan:
before:
after: