diff --git a/file/writable_file_writer.cc b/file/writable_file_writer.cc index 4fadf1d71a..4758a2f0aa 100644 --- a/file/writable_file_writer.cc +++ b/file/writable_file_writer.cc @@ -205,15 +205,17 @@ IOStatus WritableFileWriter::Pad(const IOOptions& opts, assert(pad_bytes < kDefaultPageSize); size_t left = pad_bytes; size_t cap = buf_.Capacity() - buf_.CurrentSize(); - size_t pad_start = buf_.CurrentSize(); + const size_t pad_start = buf_.CurrentSize(); // Assume pad_bytes is small compared to buf_ capacity. So we always // use buf_ rather than write directly to file in certain cases like // Append() does. + size_t actual_pad_bytes = 0; while (left) { size_t append_bytes = std::min(cap, left); buf_.PadWith(append_bytes, 0); left -= append_bytes; + actual_pad_bytes += append_bytes; if (left > 0) { IOStatus s = Flush(io_options); if (!s.ok()) { @@ -226,10 +228,13 @@ IOStatus WritableFileWriter::Pad(const IOOptions& opts, pending_sync_ = true; uint64_t cur_size = filesize_.load(std::memory_order_acquire); filesize_.store(cur_size + pad_bytes, std::memory_order_release); + + Slice data(buf_.BufferStart() + pad_start, actual_pad_bytes); + UpdateFileChecksum(data); if (perform_data_verification_) { buffered_data_crc32c_checksum_ = crc32c::Extend(buffered_data_crc32c_checksum_, - buf_.BufferStart() + pad_start, pad_bytes); + buf_.BufferStart() + pad_start, actual_pad_bytes); } return IOStatus::OK(); } diff --git a/table/table_test.cc b/table/table_test.cc index 02a8d899d7..ba44b95b32 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -5505,6 +5505,29 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) { table_reader.reset(); } +TEST_P(BlockBasedTableTest, FixBlockAlignMismatchedFileChecksums) { + Options options; + options.create_if_missing = true; + options.compression = kNoCompression; + options.file_checksum_gen_factory = GetFileChecksumGenCrc32cFactory(); + BlockBasedTableOptions bbto; + bbto.block_align = true; + bbto.block_size = 1024; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + const std::string kDBPath = + test::PerThreadDBPath("block_align_padded_bytes_verify_file_checksums"); + ASSERT_OK(DestroyDB(kDBPath, options)); + DB* db; + ASSERT_OK(DB::Open(options, kDBPath, &db)); + ASSERT_OK(db->Put(WriteOptions(), "k1", "v1")); + ASSERT_OK(db->Flush(FlushOptions())); + // Before the fix, VerifyFileChecksums() will fail as padded bytes from + // aligning blocks are used to generate the checksum to compare against the + // one not generated by padded bytes + ASSERT_OK(db->VerifyFileChecksums(ReadOptions())); + delete db; +} + TEST_P(BlockBasedTableTest, PropertiesBlockRestartPointTest) { BlockBasedTableOptions bbto = GetBlockBasedTableOptions(); bbto.block_align = true; diff --git a/unreleased_history/bug_fixes/block_align_checksum_mismatch.md b/unreleased_history/bug_fixes/block_align_checksum_mismatch.md new file mode 100644 index 0000000000..b784e32c5c --- /dev/null +++ b/unreleased_history/bug_fixes/block_align_checksum_mismatch.md @@ -0,0 +1 @@ +Fix a bug causing `VerifyFileChecksums()` to return false-positive corruption under `BlockBasedTableOptions::block_align=true`