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

Delayed write: needs_flush_speedup doesn't actually work #21

Closed
Tracked by #19
isaac-io opened this issue Jun 22, 2022 · 0 comments
Closed
Tracked by #19

Delayed write: needs_flush_speedup doesn't actually work #21

isaac-io opened this issue Jun 22, 2022 · 0 comments
Assignees

Comments

@isaac-io
Copy link
Contributor

needs_flush_speedup is considered in a branch of the code that's effectively dead code.

DBImpl::MaybeScheduleFlushOrCompaction() {
  ...
  bool is_flush_pool_empty =
        env_->GetBackgroundThreads(Env::Priority::HIGH) == 0;
    if (!is_flush_pool_empty) {
      ...
    } else {
  //    // this code is never reached  //     //
      while (unscheduled_flushes_ > 0 &&
            (bg_flush_scheduled_ + bg_compaction_scheduled_ <
                  bg_job_limits.max_flushes ||
              (needs_flush_speedup_ &&
              bg_flush_scheduled_ <= bg_job_limits.max_flushes))) {
        bg_flush_scheduled_++;
        FlushThreadArg* fta = new FlushThreadArg;
        fta->db_ = this;
        fta->thread_pri_ = Env::Priority::LOW;
        env_->Schedule(&DBImpl::BGWorkFlush, fta, Env::Priority::LOW, this,
                      &DBImpl::UnscheduleFlushCallback);
        --unscheduled_flushes_;
    }
}

The reason is that env_->GetBackgroundThreads() returns the pool capacity, not the amount of available slots, so the flush pool will never actually be empty (the intention in RocksDB here was to support a case where flushes are supposed to have the same priority as compactions and thus max_background_flushes is set to 0, but our use of needs_flush_speedup needs to know if there are high priority jobs available, regardless of capacity allocation).

Expected behavior

Available capacity is considered for scheduling based on flush urgency.

Actual behavior

Allocated capacity is considered.

Steps to reproduce the behavior

N/A

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

No branches or pull requests

2 participants