andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove TaskLimiterToken::ReleaseOnce for fix (#8567)

Browse Source 
Summary:
Rare TSAN and valgrind failures are caused by unnecessary
reading of a field on the TaskLimiterToken::limiter_ for an assertion
after the token has been released and the limiter destroyed. To simplify
we can simply destroy the token before triggering DB shutdown
(potentially destroying the limiter). This makes the ReleaseOnce logic
unnecessary.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8567

Test Plan: watch for more failures in CI

Reviewed By: ajkr

Differential Revision: D29811795

Pulled By: pdillinger

fbshipit-source-id: 135549ebb98fe4f176d1542ed85d5bd6350a40b3
This commit is contained in:
Peter Dillinger 2021-07-21 17:36:48 -07:00 committed by Facebook GitHub Bot
parent 9b41082d4a
commit 84eef260de
3 changed files with 8 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
db/db_impl/db_impl_compaction_flush.cc
View File

@ -2785,9 +2785,11 @@ void DBImpl::BackgroundCallCompaction(PrepickedCompaction* prepicked_compaction,
if (prepicked_compaction != nullptr && if (prepicked_compaction != nullptr &&
prepicked_compaction->task_token != nullptr) { prepicked_compaction->task_token != nullptr) {
// Releasing task tokens affects the DB state, so must be done before we // Releasing task tokens affects (and asserts on) the DB state, so
// potentially signal the DB close process to proceed below. // must be done before we potentially signal the DB close process to
prepicked_compaction->task_token->ReleaseOnce(); // proceed below.
prepicked_compaction->task_token.reset();
;
} }
if (made_progress || if (made_progress ||

9
util/concurrent_task_limiter_impl.cc
View File

@ -59,14 +59,9 @@ ConcurrentTaskLimiter* NewConcurrentTaskLimiter(
return new ConcurrentTaskLimiterImpl(name, limit); return new ConcurrentTaskLimiterImpl(name, limit);
} }
void TaskLimiterToken::ReleaseOnce() { TaskLimiterToken::~TaskLimiterToken() {
if (!released_) { --limiter_->outstanding_tasks_;
--limiter_->outstanding_tasks_;
released_ = true;
}
assert(limiter_->outstanding_tasks_ >= 0); assert(limiter_->outstanding_tasks_ >= 0);
} }
TaskLimiterToken::~TaskLimiterToken() { ReleaseOnce(); }
} // namespace ROCKSDB_NAMESPACE } // namespace ROCKSDB_NAMESPACE

7
util/concurrent_task_limiter_impl.h
View File

@ -53,16 +53,11 @@ class ConcurrentTaskLimiterImpl : public ConcurrentTaskLimiter {
class TaskLimiterToken { class TaskLimiterToken {
public: public:
explicit TaskLimiterToken(ConcurrentTaskLimiterImpl* limiter) explicit TaskLimiterToken(ConcurrentTaskLimiterImpl* limiter)
: limiter_(limiter), released_(false) {} : limiter_(limiter) {}
~TaskLimiterToken(); ~TaskLimiterToken();
// Releases the token from the `ConcurrentTaskLimiterImpl` if not already
// released.
// Not thread-safe.
void ReleaseOnce();
private: private:
ConcurrentTaskLimiterImpl* limiter_; ConcurrentTaskLimiterImpl* limiter_;
bool released_;
// no copying allowed // no copying allowed
TaskLimiterToken(const TaskLimiterToken&) = delete; TaskLimiterToken(const TaskLimiterToken&) = delete;