forked from facebook/rocksdb
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[RocksDB] Fix a race condition in GetSortedWalFiles
Summary: This patch fixed a race condition where a log file is moved to archived dir in the middle of GetSortedWalFiles. Without the fix, the log file would be missed in the result, which leads to transaction log iterator gap. A test utility SyncPoint is added to help reproducing the race condition. Test Plan: TransactionLogIteratorRace; make check Reviewers: dhruba, ljin Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D17121
- Loading branch information
Showing
6 changed files
with
241 additions
and
19 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
// Copyright (c) 2014, Facebook, Inc. All rights reserved. | ||
// This source code is licensed under the BSD-style license found in the | ||
// LICENSE file in the root directory of this source tree. An additional grant | ||
// of patent rights can be found in the PATENTS file in the same directory. | ||
|
||
#include "util/sync_point.h" | ||
|
||
namespace rocksdb { | ||
|
||
SyncPoint* SyncPoint::GetInstance() { | ||
static SyncPoint sync_point; | ||
return &sync_point; | ||
} | ||
|
||
void SyncPoint::LoadDependency(const std::vector<Dependency>& dependencies) { | ||
successors_.clear(); | ||
predecessors_.clear(); | ||
cleared_points_.clear(); | ||
for (const auto& dependency : dependencies) { | ||
successors_[dependency.predecessor].push_back(dependency.successor); | ||
predecessors_[dependency.successor].push_back(dependency.predecessor); | ||
} | ||
} | ||
|
||
bool SyncPoint::PredecessorsAllCleared(const std::string& point) { | ||
for (const auto& pred : predecessors_[point]) { | ||
if (cleared_points_.count(pred) == 0) { | ||
return false; | ||
} | ||
} | ||
return true; | ||
} | ||
|
||
void SyncPoint::EnableProcessing() { | ||
std::unique_lock<std::mutex> lock(mutex_); | ||
enabled_ = true; | ||
} | ||
|
||
void SyncPoint::DisableProcessing() { | ||
std::unique_lock<std::mutex> lock(mutex_); | ||
enabled_ = false; | ||
} | ||
|
||
void SyncPoint::ClearTrace() { | ||
std::unique_lock<std::mutex> lock(mutex_); | ||
cleared_points_.clear(); | ||
} | ||
|
||
void SyncPoint::Process(const std::string& point) { | ||
std::unique_lock<std::mutex> lock(mutex_); | ||
|
||
if (!enabled_) return; | ||
|
||
while (!PredecessorsAllCleared(point)) { | ||
cv_.wait(lock); | ||
} | ||
|
||
cleared_points_.insert(point); | ||
cv_.notify_all(); | ||
} | ||
|
||
} // namespace rocksdb |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
// Copyright (c) 2014, Facebook, Inc. All rights reserved. | ||
// This source code is licensed under the BSD-style license found in the | ||
// LICENSE file in the root directory of this source tree. An additional grant | ||
// of patent rights can be found in the PATENTS file in the same directory. | ||
#pragma once | ||
|
||
#include <condition_variable> | ||
#include <mutex> | ||
#include <string> | ||
#include <unordered_set> | ||
#include <unordered_map> | ||
#include <vector> | ||
|
||
namespace rocksdb { | ||
|
||
// This class provides facility to reproduce race conditions deterministically | ||
// in unit tests. | ||
// Developer could specify sync points in the codebase via TEST_SYNC_POINT. | ||
// Each sync point represents a position in the execution stream of a thread. | ||
// In the unit test, 'Happens After' relationship among sync points could be | ||
// setup via SyncPoint::LoadDependency, to reproduce a desired interleave of | ||
// threads execution. | ||
// Refer to (DBTest,TransactionLogIteratorRace), for an exmaple use case. | ||
|
||
class SyncPoint { | ||
public: | ||
static SyncPoint* GetInstance(); | ||
|
||
struct Dependency { | ||
std::string predecessor; | ||
std::string successor; | ||
}; | ||
// call once at the beginning of a test to setup the dependency between | ||
// sync points | ||
void LoadDependency(const std::vector<Dependency>& dependencies); | ||
|
||
// enable sync point processing (disabled on startup) | ||
void EnableProcessing(); | ||
|
||
// disable sync point processing | ||
void DisableProcessing(); | ||
|
||
// remove the execution trace of all sync points | ||
void ClearTrace(); | ||
|
||
// triggered by TEST_SYNC_POINT, blocking execution until all predecessors | ||
// are executed. | ||
void Process(const std::string& point); | ||
|
||
// TODO: it might be useful to provide a function that blocks until all | ||
// sync points are cleared. | ||
|
||
private: | ||
bool PredecessorsAllCleared(const std::string& point); | ||
|
||
// successor/predecessor map loaded from LoadDependency | ||
std::unordered_map<std::string, std::vector<std::string>> successors_; | ||
std::unordered_map<std::string, std::vector<std::string>> predecessors_; | ||
|
||
std::mutex mutex_; | ||
std::condition_variable cv_; | ||
// sync points that have been passed through | ||
std::unordered_set<std::string> cleared_points_; | ||
bool enabled_ = false; | ||
}; | ||
|
||
} // namespace rocksdb | ||
|
||
// Use TEST_SYNC_POINT to specify sync points inside code base. | ||
// Sync points can have happens-after depedency on other sync points, | ||
// configured at runtime via SyncPoint::LoadDependency. This could be | ||
// utilized to re-produce race conditions between threads. | ||
// See TransactionLogIteratorRace in db_test.cc for an example use case. | ||
// TEST_SYNC_POINT is no op in release build. | ||
#ifdef NDEBUG | ||
#define TEST_SYNC_POINT(x) | ||
#else | ||
#define TEST_SYNC_POINT(x) rocksdb::SyncPoint::GetInstance()->Process(x) | ||
#endif |