Specify largest_seqno in VerifyChecksum (#9919)
Summary: `VerifyChecksum()` does not specify `largest_seqno` when creating a `TableReader`. As a result, the `TableReader` uses the `TableReaderOptions` default value (0) for `largest_seqno`. This causes the following error when the file has a nonzero global seqno in its properties: ``` Corruption: An external sst file with version 2 have global seqno property with value , while largest seqno in the file is 0 ``` This PR fixes this by specifying `largest_seqno` in `VerifyChecksumInternal` with `largest_seqno` from the file metadata. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9919 Test Plan: `make check` Reviewed By: ajkr Differential Revision: D36028824 Pulled By: cbi42 fbshipit-source-id: 428d028a79386f46ef97bb6b6051dc76c83e1f2b
This commit is contained in:
parent
2b5c29f9f3
commit
37f490834d
@ -40,7 +40,8 @@ Status VerifySstFileChecksum(const Options& options,
|
||||
Status VerifySstFileChecksum(const Options& options,
|
||||
const EnvOptions& env_options,
|
||||
const ReadOptions& read_options,
|
||||
const std::string& file_path) {
|
||||
const std::string& file_path,
|
||||
const SequenceNumber& largest_seqno) {
|
||||
std::unique_ptr<FSRandomAccessFile> file;
|
||||
uint64_t file_size;
|
||||
InternalKeyComparator internal_comparator(options.comparator);
|
||||
@ -61,12 +62,13 @@ Status VerifySstFileChecksum(const Options& options,
|
||||
nullptr /* stats */, 0 /* hist_type */, nullptr /* file_read_hist */,
|
||||
ioptions.rate_limiter.get()));
|
||||
const bool kImmortal = true;
|
||||
auto reader_options = TableReaderOptions(
|
||||
ioptions, options.prefix_extractor, env_options, internal_comparator,
|
||||
false /* skip_filters */, !kImmortal, false /* force_direct_prefetch */,
|
||||
-1 /* level */);
|
||||
reader_options.largest_seqno = largest_seqno;
|
||||
s = ioptions.table_factory->NewTableReader(
|
||||
TableReaderOptions(ioptions, options.prefix_extractor, env_options,
|
||||
internal_comparator, false /* skip_filters */,
|
||||
!kImmortal, false /* force_direct_prefetch */,
|
||||
-1 /* level */),
|
||||
std::move(file_reader), file_size, &table_reader,
|
||||
reader_options, std::move(file_reader), file_size, &table_reader,
|
||||
false /* prefetch_index_and_filter_in_cache */);
|
||||
if (!s.ok()) {
|
||||
return s;
|
||||
|
@ -5123,8 +5123,8 @@ Status DBImpl::VerifyChecksumInternal(const ReadOptions& read_options,
|
||||
fmeta->file_checksum_func_name, fname,
|
||||
read_options);
|
||||
} else {
|
||||
s = ROCKSDB_NAMESPACE::VerifySstFileChecksum(opts, file_options_,
|
||||
read_options, fname);
|
||||
s = ROCKSDB_NAMESPACE::VerifySstFileChecksum(
|
||||
opts, file_options_, read_options, fname, fd.largest_seqno);
|
||||
}
|
||||
RecordTick(stats_, VERIFY_CHECKSUM_READ_BYTES,
|
||||
IOSTATS(bytes_read) - prev_bytes_read);
|
||||
|
@ -1844,6 +1844,28 @@ TEST_F(ExternalSSTFileBasicTest, FailIfNotBottommostLevel) {
|
||||
}
|
||||
}
|
||||
|
||||
TEST_F(ExternalSSTFileBasicTest, VerifyChecksum) {
|
||||
const std::string kPutVal = "put_val";
|
||||
const std::string kIngestedVal = "ingested_val";
|
||||
|
||||
ASSERT_OK(Put("k", kPutVal, WriteOptions()));
|
||||
ASSERT_OK(Flush());
|
||||
|
||||
std::string external_file = sst_files_dir_ + "/file_to_ingest.sst";
|
||||
{
|
||||
SstFileWriter sst_file_writer{EnvOptions(), CurrentOptions()};
|
||||
|
||||
ASSERT_OK(sst_file_writer.Open(external_file));
|
||||
ASSERT_OK(sst_file_writer.Put("k", kIngestedVal));
|
||||
ASSERT_OK(sst_file_writer.Finish());
|
||||
}
|
||||
|
||||
ASSERT_OK(db_->IngestExternalFile(db_->DefaultColumnFamily(), {external_file},
|
||||
IngestExternalFileOptions()));
|
||||
|
||||
ASSERT_OK(db_->VerifyChecksum());
|
||||
}
|
||||
|
||||
INSTANTIATE_TEST_CASE_P(ExternalSSTFileBasicTest, ExternalSSTFileBasicTest,
|
||||
testing::Values(std::make_tuple(true, true),
|
||||
std::make_tuple(true, false),
|
||||
|
@ -518,7 +518,8 @@ Status VerifySstFileChecksum(const Options& options,
|
||||
Status VerifySstFileChecksum(const Options& options,
|
||||
const EnvOptions& env_options,
|
||||
const ReadOptions& read_options,
|
||||
const std::string& file_path);
|
||||
const std::string& file_path,
|
||||
const SequenceNumber& largest_seqno = 0);
|
||||
#endif // ROCKSDB_LITE
|
||||
|
||||
} // namespace ROCKSDB_NAMESPACE
|
||||
|
Loading…
Reference in New Issue
Block a user