Skip to content

Commit

Permalink
Fix deadlock in WAL sync
Browse files Browse the repository at this point in the history
Summary:
MarkLogsSynced() was doing `logs_.erase(it++);`. The standard is saying:

```
all iterators and references are invalidated, unless the erased members are at an end (front or back) of the deque (in which case only iterators and references to the erased members are invalidated)
```

Because `it` is an iterator to the first element of the container, it is
invalidated, only one iteration is executed and `log.getting_synced = false;`
is not being done, so `while (logs_.front().getting_synced)` in `WriteImpl()`
is not terminating.

Test Plan: make db_bench && ./db_bench --benchmarks=fillsync

Reviewers: igor, rven, IslamAbdelRahman, anthony, kradhakrishnan, yhchiang, sdong, tnovak

Reviewed By: tnovak

Subscribers: kolmike, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D45807
  • Loading branch information
4tXJ7f committed Aug 29, 2015
1 parent 72a9b73 commit effd9dd
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
3 changes: 2 additions & 1 deletion db/db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2020,12 +2020,13 @@ void DBImpl::MarkLogsSynced(
assert(log.getting_synced);
if (status.ok() && logs_.size() > 1) {
logs_to_free_.push_back(log.ReleaseWriter());
logs_.erase(it++);
it = logs_.erase(it);
} else {
log.getting_synced = false;
++it;
}
}
assert(logs_.empty() || (logs_.size() == 1 && !logs_[0].getting_synced));
log_sync_cv_.SignalAll();
}

Expand Down
24 changes: 24 additions & 0 deletions db/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4745,6 +4745,30 @@ TEST_F(DBTest, PurgeInfoLogs) {
}
}

TEST_F(DBTest, SyncMultipleLogs) {
const uint64_t kNumBatches = 2;
const int kBatchSize = 1000;

Options options = CurrentOptions();
options.create_if_missing = true;
options.write_buffer_size = 4096;
Reopen(options);

WriteBatch batch;
WriteOptions wo;
wo.sync = true;

for (uint64_t b = 0; b < kNumBatches; b++) {
batch.Clear();
for (int i = 0; i < kBatchSize; i++) {
batch.Put(Key(i), DummyString(128));
}

dbfull()->Write(wo, &batch);
}

ASSERT_OK(dbfull()->SyncWAL());
}

//
// Test WAL recovery for the various modes available
Expand Down

0 comments on commit effd9dd

Please sign in to comment.