Skip to content

Commit

Permalink
Fix a group commit bug in LogAndApply
Browse files Browse the repository at this point in the history
Summary:
EncodeTo(&record) does not overwrite, it appends to it.

This means that group commit log and apply will look something like:
record1
record1record2
record1record2record3

I'm surprised this didn't show up in production, but I think the reason is that MANIFEST group commit almost never happens.

This bug turned up in column family work, where opening a database failed with "adding a same column family twice".

Test Plan: Tested the change in column family branch and observed that the problem is gone (with db_stress)

Reviewers: dhruba, haobo

Reviewed By: dhruba

CC: leveldb

Differential Revision: https://reviews.facebook.net/D16461
  • Loading branch information
igorcanadi committed Mar 4, 2014
1 parent 97eddef commit 5142b37
Showing 1 changed file with 13 additions and 4 deletions.
17 changes: 13 additions & 4 deletions db/version_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1568,9 +1568,9 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu,

// Write new record to MANIFEST log
if (s.ok()) {
std::string record;
for (unsigned int i = 0; i < batch_edits.size(); i++) {
batch_edits[i]->EncodeTo(&record);
for (auto& e : batch_edits) {
std::string record;
e->EncodeTo(&record);
s = descriptor_log_->AddRecord(record);
if (!s.ok()) {
break;
Expand All @@ -1589,7 +1589,16 @@ Status VersionSet::LogAndApply(VersionEdit* edit, port::Mutex* mu,
}
if (!s.ok()) {
Log(options_->info_log, "MANIFEST write: %s\n", s.ToString().c_str());
if (ManifestContains(record)) {
bool all_records_in = true;
for (auto& e : batch_edits) {
std::string record;
e->EncodeTo(&record);
if (!ManifestContains(record)) {
all_records_in = false;
break;
}
}
if (all_records_in) {
Log(options_->info_log,
"MANIFEST contains log record despite error; advancing to new "
"version to prevent mismatch between in-memory and logged state"
Expand Down

0 comments on commit 5142b37

Please sign in to comment.