Skip to content

Commit dbbffbd

Browse files
committed
Mark the log_number file number used
Summary: VersionSet::next_file_number_ is always assumed to be strictly greater than VersionSet::log_number_. In our new recovery code, we artificially set log_number_ to be (log_number + 1), so that once we flush, we don't recover from the same log file again (this is important because of merge operator non-idempotence) When we set VersionSet::log_number_ to (log_number + 1), we also have to mark that file number used, such that next_file_number_ is increased to a legal level. Otherwise, VersionSet might assert. This has not be a problem so far because here's what happens: 1. assume next_file_number is 5, we're recovering log_number 10 2. in DBImpl::Recover() we call MarkFileNumberUsed with 10. This will set VersionSet::next_file_number_ to 11. 3. If there are some updates, we will call WriteTable0ForRecovery(), which will use file number 11 as a new table file and advance VersionSet::next_file_number_ to 12. 4. When we LogAndApply() with log_number 11, assertion is true: assert(11 <= 12); However, this was a lucky occurrence. Even though this diff doesn't cause a bug, I think the issue is important to fix. Test Plan: In column families I have different recovery logic and this code path asserted. When adding MarkFileNumberUsed(log_number + 1) assert is gone. Reviewers: dhruba, kailiu Reviewed By: kailiu CC: leveldb Differential Revision: https://reviews.facebook.net/D15783
1 parent 56bea9f commit dbbffbd

File tree

1 file changed

+5
-0
lines changed

1 file changed

+5
-0
lines changed

db/db_impl.cc

+5
Original file line numberDiff line numberDiff line change
@@ -1066,6 +1066,11 @@ Status DBImpl::RecoverLogFile(uint64_t log_number, SequenceNumber* max_sequence,
10661066
// Since we already recovered log_number, we want all logs
10671067
// with numbers `<= log_number` (includes this one) to be ignored
10681068
edit.SetLogNumber(log_number + 1);
1069+
// we must mark the next log number as used, even though it's
1070+
// not actually used. that is because VersionSet assumes
1071+
// VersionSet::next_file_number_ always to be strictly greater than any log
1072+
// number
1073+
versions_->MarkFileNumberUsed(log_number + 1);
10691074
status = versions_->LogAndApply(&edit, &mutex_);
10701075
}
10711076

0 commit comments

Comments
 (0)