From 269fc6ca9416129248db5ca57050cd5d39d177c8 Mon Sep 17 00:00:00 2001 From: David Grogan Date: Mon, 10 Feb 2014 11:36:06 -0800 Subject: Release LevelDB 1.16 - Make Log::Reader not report a corruption when the last record in a log file is truncated. - Fix issue 224: variable created but not utilized. - Remove comment that referenced a removed feature. --- Makefile | 2 +- db/log_reader.cc | 23 +++++++++++++++-------- db/log_test.cc | 40 +++++++++++++++++++++++++++++++++++----- db/repair.cc | 1 - include/leveldb/c.h | 1 - include/leveldb/db.h | 2 +- 6 files changed, 52 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 344ff29..a94f1a2 100644 --- a/Makefile +++ b/Makefile @@ -72,7 +72,7 @@ SHARED = $(SHARED1) else # Update db.h if you change these. SHARED_MAJOR = 1 -SHARED_MINOR = 15 +SHARED_MINOR = 16 SHARED1 = libleveldb.$(PLATFORM_SHARED_EXT) SHARED2 = $(SHARED1).$(SHARED_MAJOR) SHARED3 = $(SHARED1).$(SHARED_MAJOR).$(SHARED_MINOR) diff --git a/db/log_reader.cc b/db/log_reader.cc index b35f115..4919216 100644 --- a/db/log_reader.cc +++ b/db/log_reader.cc @@ -133,7 +133,9 @@ bool Reader::ReadRecord(Slice* record, std::string* scratch) { case kEof: if (in_fragmented_record) { - ReportCorruption(scratch->size(), "partial record without end(3)"); + // This can be caused by the writer dying immediately after + // writing a physical record but before completing the next; don't + // treat it as a corruption, just ignore the entire logical record. scratch->clear(); } return false; @@ -193,13 +195,12 @@ unsigned int Reader::ReadPhysicalRecord(Slice* result) { eof_ = true; } continue; - } else if (buffer_.size() == 0) { - // End of file - return kEof; } else { - size_t drop_size = buffer_.size(); + // Note that if buffer_ is non-empty, we have a truncated header at the + // end of the file, which can be caused by the writer crashing in the + // middle of writing the header. Instead of considering this an error, + // just report EOF. buffer_.clear(); - ReportCorruption(drop_size, "truncated record at end of file"); return kEof; } } @@ -213,8 +214,14 @@ unsigned int Reader::ReadPhysicalRecord(Slice* result) { if (kHeaderSize + length > buffer_.size()) { size_t drop_size = buffer_.size(); buffer_.clear(); - ReportCorruption(drop_size, "bad record length"); - return kBadRecord; + if (!eof_) { + ReportCorruption(drop_size, "bad record length"); + return kBadRecord; + } + // If the end of the file has been reached without reading |length| bytes + // of payload, assume the writer died in the middle of writing the record. + // Don't report a corruption. + return kEof; } if (type == kZeroType && length == 0) { diff --git a/db/log_test.cc b/db/log_test.cc index 4c5cf87..91d3caa 100644 --- a/db/log_test.cc +++ b/db/log_test.cc @@ -351,20 +351,32 @@ TEST(LogTest, BadRecordType) { ASSERT_EQ("OK", MatchError("unknown record type")); } -TEST(LogTest, TruncatedTrailingRecord) { +TEST(LogTest, TruncatedTrailingRecordIsIgnored) { Write("foo"); ShrinkSize(4); // Drop all payload as well as a header byte ASSERT_EQ("EOF", Read()); - ASSERT_EQ(kHeaderSize - 1, DroppedBytes()); - ASSERT_EQ("OK", MatchError("truncated record at end of file")); + // Truncated last record is ignored, not treated as an error. + ASSERT_EQ(0, DroppedBytes()); + ASSERT_EQ("", ReportMessage()); } TEST(LogTest, BadLength) { + const int kPayloadSize = kBlockSize - kHeaderSize; + Write(BigString("bar", kPayloadSize)); + Write("foo"); + // Least significant size byte is stored in header[4]. + IncrementByte(4, 1); + ASSERT_EQ("foo", Read()); + ASSERT_EQ(kBlockSize, DroppedBytes()); + ASSERT_EQ("OK", MatchError("bad record length")); +} + +TEST(LogTest, BadLengthAtEndIsIgnored) { Write("foo"); ShrinkSize(1); ASSERT_EQ("EOF", Read()); - ASSERT_EQ(kHeaderSize + 2, DroppedBytes()); - ASSERT_EQ("OK", MatchError("bad record length")); + ASSERT_EQ(0, DroppedBytes()); + ASSERT_EQ("", ReportMessage()); } TEST(LogTest, ChecksumMismatch) { @@ -415,6 +427,24 @@ TEST(LogTest, UnexpectedFirstType) { ASSERT_EQ("OK", MatchError("partial record without end")); } +TEST(LogTest, MissingLastIsIgnored) { + Write(BigString("bar", kBlockSize)); + // Remove the LAST block, including header. + ShrinkSize(14); + ASSERT_EQ("EOF", Read()); + ASSERT_EQ("", ReportMessage()); + ASSERT_EQ(0, DroppedBytes()); +} + +TEST(LogTest, PartialLastIsIgnored) { + Write(BigString("bar", kBlockSize)); + // Cause a bad record length in the LAST block. + ShrinkSize(1); + ASSERT_EQ("EOF", Read()); + ASSERT_EQ("", ReportMessage()); + ASSERT_EQ(0, DroppedBytes()); +} + TEST(LogTest, ErrorJoinsRecords) { // Consider two fragmented records: // first(R1) last(R1) first(R2) last(R2) diff --git a/db/repair.cc b/db/repair.cc index 96c9b37..7727faf 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -242,7 +242,6 @@ class Repairer { } void ExtractMetaData() { - std::vector kept; for (size_t i = 0; i < table_numbers_.size(); i++) { ScanTable(table_numbers_[i]); } diff --git a/include/leveldb/c.h b/include/leveldb/c.h index 1fa5886..1048fe3 100644 --- a/include/leveldb/c.h +++ b/include/leveldb/c.h @@ -9,7 +9,6 @@ Does not support: . getters for the option types . custom comparators that implement key shortening - . capturing post-write-snapshot . custom iter, db, env, cache implementations using just the C bindings Some conventions: diff --git a/include/leveldb/db.h b/include/leveldb/db.h index 5ffb29d..4f3a851 100644 --- a/include/leveldb/db.h +++ b/include/leveldb/db.h @@ -14,7 +14,7 @@ namespace leveldb { // Update Makefile if you change these static const int kMajorVersion = 1; -static const int kMinorVersion = 15; +static const int kMinorVersion = 16; struct Options; struct ReadOptions; -- cgit v1.2.1