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

Remove a race condition between lsdir and rm (#4440)

Browse Source 
Summary:
In DBCompactionTestWithParam::ManualLevelCompactionOutputPathId, there is
a race condition between `DBTestBase::GetSstFileCount` and
`DBImpl::PurgeObsoleteFiles`. The following graph explains why.

```
Timeline  db_compact_test_t              bg_flush_t         bg_compact_t
    |  [initiate bg flush and
    |      start waiting]
    |                                     flush
    |                                     DeleteObsoleteFiles
    |  [waken up by bg_flush_t which
    |   signaled in DeleteObsoleteFiles]
    |
    |  [initiate compaction and
    |   start waiting]
    |
    |                                                         [compact,
    |                                                          set manual.done to true]
    |                                   [signal at the end of
    |                                    BackgroundCallFlush]
    |
    |  [waken up by bg_flush_t
    |   which signaled before
    |   returning from
    |   BackgroundCallFlush]
    |
    |  Check manual.done is true
    |
    |  GetSstFileCount    <-- race condition -->           PurgeObsoleteFiles
    V
```
Pull Request resolved: https://github.com/facebook/rocksdb/pull/4440

Differential Revision: D10122628

Pulled By: riversand963

fbshipit-source-id: 3ede73c39fee6ad804dc6ac1ed84759c7e63977f
This commit is contained in:
Yanqin Jin 2018-10-01 11:56:09 -07:00 committed by Facebook Github Bot
parent ac6f435a9a
commit be5cc4c7b8
1 changed files with 3 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
db/db_compaction_test.cc
View File

@ -2477,6 +2477,7 @@ TEST_P(DBCompactionTestWithParam, ManualLevelCompactionOutputPathId) {
// Compaction range overlaps files // Compaction range overlaps files
Compact(1, "p1", "p9", 1); Compact(1, "p1", "p9", 1);
ASSERT_OK(dbfull()->TEST_WaitForCompact());
ASSERT_EQ("0,1", FilesPerLevel(1)); ASSERT_EQ("0,1", FilesPerLevel(1));
ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path)); ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path));
ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path)); ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path));
@ -2492,6 +2493,7 @@ TEST_P(DBCompactionTestWithParam, ManualLevelCompactionOutputPathId) {
// Compact just the new range // Compact just the new range
Compact(1, "b", "f", 1); Compact(1, "b", "f", 1);
ASSERT_OK(dbfull()->TEST_WaitForCompact());
ASSERT_EQ("0,2", FilesPerLevel(1)); ASSERT_EQ("0,2", FilesPerLevel(1));
ASSERT_EQ(2, GetSstFileCount(options.db_paths[1].path)); ASSERT_EQ(2, GetSstFileCount(options.db_paths[1].path));
ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path)); ASSERT_EQ(0, GetSstFileCount(options.db_paths[0].path));
@ -2508,6 +2510,7 @@ TEST_P(DBCompactionTestWithParam, ManualLevelCompactionOutputPathId) {
compact_options.target_path_id = 1; compact_options.target_path_id = 1;
compact_options.exclusive_manual_compaction = exclusive_manual_compaction_; compact_options.exclusive_manual_compaction = exclusive_manual_compaction_;
db_->CompactRange(compact_options, handles_[1], nullptr, nullptr); db_->CompactRange(compact_options, handles_[1], nullptr, nullptr);
ASSERT_OK(dbfull()->TEST_WaitForCompact());
ASSERT_EQ("0,1", FilesPerLevel(1)); ASSERT_EQ("0,1", FilesPerLevel(1));
ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path)); ASSERT_EQ(1, GetSstFileCount(options.db_paths[1].path));