Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race condition with WAL tracking and FlushWAL(true /* sync */) #10185

Closed
wants to merge 14 commits into from
Prev Previous commit
Next Next commit
tracked_synced_size_limit -> pre_sync_size
  • Loading branch information
ajkr committed Jun 17, 2022
commit b1bc913313bcbeb09b1e4e3a74d196700cf03472
14 changes: 4 additions & 10 deletions db/db_impl/db_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1398,12 +1398,9 @@ Status DBImpl::SyncWAL() {
auto& log = *it;
assert(!log.getting_synced);
log.getting_synced = true;
if (it->number == current_log_number) {
// The current log can be appended while we sync it. Such appends are
// not necessarily synced so we need to figure out the size guaranteed
// to be synced ahead of time.
log.tracked_synced_size_limit = log.writer->file()->GetFileSize();
}
// Size is expected to be monotonically increasing.
assert(log.writer->file()->GetFileSize() >= log.pre_sync_size);
log.pre_sync_size = log.writer->file()->GetFileSize();
logs_to_sync.push_back(log.writer);
}

Expand Down Expand Up @@ -1479,9 +1476,7 @@ Status DBImpl::MarkLogsSynced(uint64_t up_to, bool synced_dir) {
assert(wal.getting_synced);
if (immutable_db_options_.track_and_verify_wals_in_manifest &&
wal.writer->file()->GetFileSize() > 0) {
uint64_t tracked_synced_size = std::min(
wal.tracked_synced_size_limit, wal.writer->file()->GetFileSize());
synced_wals.AddWal(wal.number, WalMetadata(tracked_synced_size));
synced_wals.AddWal(wal.number, WalMetadata(wal.pre_sync_size));
}

if (logs_.size() > 1) {
Expand All @@ -1491,7 +1486,6 @@ Status DBImpl::MarkLogsSynced(uint64_t up_to, bool synced_dir) {
it = logs_.erase(it);
} else {
wal.getting_synced = false;
wal.tracked_synced_size_limit = UINT64_MAX;
++it;
}
}
Expand Down
8 changes: 4 additions & 4 deletions db/db_impl/db_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1599,10 +1599,10 @@ class DBImpl : public DB {
log::Writer* writer; // own
// true for some prefix of logs_
bool getting_synced = false;
// This can be used to limit the synced size tracked in MANIFEST. It is
// useful for files that undergo append during sync, in which case the file
// size at tracking time is not necessarily all synced.
uint64_t tracked_synced_size_limit = UINT64_MAX;
// The size of the file before the sync happens. This amount is guaranteed
// to be persisted even if appends happen during sync so it can be used for
// tracking the synced size in MANIFEST.
ajkr marked this conversation as resolved.
Show resolved Hide resolved
uint64_t pre_sync_size = 0;
};

// PurgeFileInfo is a structure to hold information of files to be deleted in
Expand Down
3 changes: 3 additions & 0 deletions db/db_impl/db_impl_compaction_flush.cc
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ IOStatus DBImpl::SyncClosedLogs(JobContext* job_context) {
auto& log = *it;
assert(!log.getting_synced);
log.getting_synced = true;
// Size is expected to be monotonically increasing.
assert(log.writer->file()->GetFileSize() >= log.pre_sync_size);
log.pre_sync_size = log.writer->file()->GetFileSize();
logs_to_sync.push_back(log.writer);
}

Expand Down
3 changes: 3 additions & 0 deletions db/db_impl/db_impl_write.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,9 @@ Status DBImpl::PreprocessWrite(const WriteOptions& write_options,
// Note: there does not seem to be a reason to set this early before we
// actually write to the WAL
log.getting_synced = true;
// Size is expected to be monotonically increasing.
assert(log.writer->file()->GetFileSize() >= log.pre_sync_size);
log.pre_sync_size = log.writer->file()->GetFileSize();
}
} else {
*need_log_sync = false;
Expand Down