c70bae1b05
Summary:
Previously the shutdown process did not properly wait for all
`compaction_thread_limiter` tokens to be released before proceeding to
delete the DB's C++ objects. When this happened, we saw tests like
"DBCompactionTest.CompactionLimiter" flake with the following error:
```
virtual
rocksdb::ConcurrentTaskLimiterImpl::~ConcurrentTaskLimiterImpl():
Assertion `outstanding_tasks_ == 0' failed.
```
There is a case where a token can still be alive even after the shutdown
process has waited for BG work to complete. In particular, this happens
because the shutdown process only waits for flush/compaction scheduled/unscheduled counters to all
reach zero. These counters are decremented in `BackgroundCallCompaction()`
functions. However, tokens are released in `BGWork*Compaction()` functions, which
actually wrap the `BackgroundCallCompaction()` function.
A simple sleep could repro the race condition:
```
$ diff --git a/db/db_impl/db_impl_compaction_flush.cc
b/db/db_impl/db_impl_compaction_flush.cc
index 806bc548a..ba59efa89 100644
--- a/db/db_impl/db_impl_compaction_flush.cc
+++ b/db/db_impl/db_impl_compaction_flush.cc
@@ -2442,6 +2442,7 @@ void DBImpl::BGWorkCompaction(void* arg) {
static_cast<PrepickedCompaction*>(ca.prepicked_compaction);
static_cast_with_check<DBImpl>(ca.db)->BackgroundCallCompaction(
prepicked_compaction, Env::Priority::LOW);
+ sleep(1);
delete prepicked_compaction;
}
$ ./db_compaction_test --gtest_filter=DBCompactionTest.CompactionLimiter
db_compaction_test: util/concurrent_task_limiter_impl.cc:24: virtual rocksdb::ConcurrentTaskLimiterImpl::~ConcurrentTaskLimiterImpl(): Assertion `outstanding_tasks_ == 0' failed.
Received signal 6 (Aborted)
#0 /usr/local/fbcode/platform007/lib/libc.so.6(gsignal+0xcf) [0x7f02673c30ff] ?? ??:0
https://github.com/facebook/rocksdb/issues/1 /usr/local/fbcode/platform007/lib/libc.so.6(abort+0x134) [0x7f02673ac934] ?? ??:0
...
```
Pull Request resolved: https://github.com/facebook/rocksdb/pull/8253
Test Plan: sleeps to expose race conditions
Reviewed By: akankshamahajan15
Differential Revision: D28168064
Pulled By: ajkr
fbshipit-source-id: 9e5167c74398d323e7975980c5cc00f450631160
73 lines
2.2 KiB
C++
73 lines
2.2 KiB
C++
// Copyright (c) 2011-present, Facebook, Inc. All rights reserved.
|
|
// This source code is licensed under both the GPLv2 (found in the
|
|
// COPYING file in the root directory) and Apache 2.0 License
|
|
// (found in the LICENSE.Apache file in the root directory).
|
|
//
|
|
// Copyright (c) 2011 The LevelDB Authors. All rights reserved.
|
|
// Use of this source code is governed by a BSD-style license that can be
|
|
// found in the LICENSE file. See the AUTHORS file for names of contributors.
|
|
|
|
#pragma once
|
|
#include <atomic>
|
|
#include <memory>
|
|
|
|
#include "rocksdb/env.h"
|
|
#include "rocksdb/concurrent_task_limiter.h"
|
|
|
|
namespace ROCKSDB_NAMESPACE {
|
|
|
|
class TaskLimiterToken;
|
|
|
|
class ConcurrentTaskLimiterImpl : public ConcurrentTaskLimiter {
|
|
public:
|
|
explicit ConcurrentTaskLimiterImpl(const std::string& name,
|
|
int32_t max_outstanding_task);
|
|
// No copying allowed
|
|
ConcurrentTaskLimiterImpl(const ConcurrentTaskLimiterImpl&) = delete;
|
|
ConcurrentTaskLimiterImpl& operator=(const ConcurrentTaskLimiterImpl&) =
|
|
delete;
|
|
|
|
virtual ~ConcurrentTaskLimiterImpl();
|
|
|
|
virtual const std::string& GetName() const override;
|
|
|
|
virtual void SetMaxOutstandingTask(int32_t limit) override;
|
|
|
|
virtual void ResetMaxOutstandingTask() override;
|
|
|
|
virtual int32_t GetOutstandingTask() const override;
|
|
|
|
// Request token for adding a new task.
|
|
// If force == true, it requests a token bypassing throttle.
|
|
// Returns nullptr if it got throttled.
|
|
virtual std::unique_ptr<TaskLimiterToken> GetToken(bool force);
|
|
|
|
private:
|
|
friend class TaskLimiterToken;
|
|
|
|
std::string name_;
|
|
std::atomic<int32_t> max_outstanding_tasks_;
|
|
std::atomic<int32_t> outstanding_tasks_;
|
|
};
|
|
|
|
class TaskLimiterToken {
|
|
public:
|
|
explicit TaskLimiterToken(ConcurrentTaskLimiterImpl* limiter)
|
|
: limiter_(limiter), released_(false) {}
|
|
~TaskLimiterToken();
|
|
// Releases the token from the `ConcurrentTaskLimiterImpl` if not already
|
|
// released.
|
|
// Not thread-safe.
|
|
void ReleaseOnce();
|
|
|
|
private:
|
|
ConcurrentTaskLimiterImpl* limiter_;
|
|
bool released_;
|
|
|
|
// no copying allowed
|
|
TaskLimiterToken(const TaskLimiterToken&) = delete;
|
|
void operator=(const TaskLimiterToken&) = delete;
|
|
};
|
|
|
|
} // namespace ROCKSDB_NAMESPACE
|