From bfd00bba9c75fffc95d53c741c6939c2ba5223f3 Mon Sep 17 00:00:00 2001 From: Peter Dillinger Date: Thu, 15 Feb 2024 11:23:48 -0800 Subject: [PATCH] Use format_version=6 by default (#12352) Summary: It's in production for a large storage service, and it was initially released 6 months ago (8.6.0). IMHO that's enough room for "easy downgrade" to most any user's previously integrated version, even if they only update a few times a year. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12352 Test Plan: tests updated, including format capatibility test table_test: ApproximateOffsetOfCompressed is affected because adding index block to metaindex adds about 13 bytes to SST files in format_version 6. This test has historically been problematic and one reason is that, apparently, not only could it pass/fail depending on snappy compression version, but also how long your host name is, because of db_host_id. I've cleared that out for the test, which takes care of format_version=6 and hopefully improves long-term reliability. Suggested follow-up: FinishImpl in table_test.cc takes a table_options that is ignored in some cases and might not match the ioptions.table_factory configuration unless the caller is very careful. This should be cleaned up somehow. Reviewed By: anand1976 Differential Revision: D53786884 Pulled By: pdillinger fbshipit-source-id: 1964cbd40d3ab0a821fdc01c458031df716fcf51 --- include/rocksdb/table.h | 10 ++++++++- .../org/rocksdb/BlockBasedTableConfig.java | 2 +- table/block_fetcher_test.cc | 21 +++++++++++++------ table/table_test.cc | 2 ++ tools/check_format_compatible.sh | 4 ++-- .../behavior_changes/format6.md | 1 + 6 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 unreleased_history/behavior_changes/format6.md diff --git a/include/rocksdb/table.h b/include/rocksdb/table.h index 5a4e8070af5..b0b276a6396 100644 --- a/include/rocksdb/table.h +++ b/include/rocksdb/table.h @@ -518,7 +518,15 @@ struct BlockBasedTableOptions { // 6 -- Modified the file footer and checksum matching so that SST data // misplaced within or between files is as likely to fail checksum // verification as random corruption. Also checksum-protects SST footer. - uint32_t format_version = 5; + // Can be read by RocksDB versions >= 8.6.0. + // + // Using the default setting of format_version is strongly recommended, so + // that available enhancements are adopted eventually and automatically. The + // default setting will only update to the latest after thorough production + // validation and sufficient time and number of releases have elapsed + // (6 months recommended) to ensure a clean downgrade/revert path for users + // who might only upgrade a few times per year. + uint32_t format_version = 6; // Store index blocks on disk in compressed format. Changing this option to // false will avoid the overhead of decompression if index blocks are evicted diff --git a/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java b/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java index a41fe50ca53..d066131458c 100644 --- a/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java +++ b/java/src/main/java/org/rocksdb/BlockBasedTableConfig.java @@ -37,7 +37,7 @@ public BlockBasedTableConfig() { wholeKeyFiltering = true; verifyCompression = false; readAmpBytesPerBit = 0; - formatVersion = 5; + formatVersion = 6; enableIndexCompression = true; blockAlign = false; indexShortening = IndexShorteningMode.kShortenSeparators; diff --git a/table/block_fetcher_test.cc b/table/block_fetcher_test.cc index d8f6e362018..ca8fcb7e505 100644 --- a/table/block_fetcher_test.cc +++ b/table/block_fetcher_test.cc @@ -107,11 +107,16 @@ class BlockFetcherTest : public testing::Test { // Get handle of the index block. Footer footer; - ReadFooter(file.get(), &footer); - const BlockHandle& index_handle = footer.index_handle(); - // FIXME: index handle will need to come from metaindex for - // format_version >= 6 when that becomes the default - ASSERT_FALSE(index_handle.IsNull()); + uint64_t file_size = 0; + ReadFooter(file.get(), &footer, &file_size); + + // Index handle comes from metaindex for format_version >= 6 + ASSERT_TRUE(footer.index_handle().IsNull()); + + BlockHandle index_handle; + ASSERT_OK(FindMetaBlockInFile( + file.get(), file_size, kBlockBasedTableMagicNumber, + ImmutableOptions(options_), {}, kIndexBlockName, &index_handle)); CompressionType compression_type; FetchBlock(file.get(), index_handle, BlockType::kIndex, @@ -286,13 +291,17 @@ class BlockFetcherTest : public testing::Test { return internal_key.Encode().ToString(); } - void ReadFooter(RandomAccessFileReader* file, Footer* footer) { + void ReadFooter(RandomAccessFileReader* file, Footer* footer, + uint64_t* file_size_out = nullptr) { uint64_t file_size = 0; ASSERT_OK(env_->GetFileSize(file->file_name(), &file_size)); IOOptions opts; ASSERT_OK(ReadFooterFromFile(opts, file, *fs_, nullptr /* prefetch_buffer */, file_size, footer, kBlockBasedTableMagicNumber)); + if (file_size_out) { + *file_size_out = file_size; + } } // NOTE: compression_type returns the compression type of the fetched block diff --git a/table/table_test.cc b/table/table_test.cc index b452f0d2b53..9526bd9f01d 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -4706,8 +4706,10 @@ static void DoCompressionTest(CompressionType comp) { Options options; test::PlainInternalKeyComparator ikc(options.comparator); options.compression = comp; + options.db_host_id = ""; BlockBasedTableOptions table_options; table_options.block_size = 1024; + options.table_factory.reset(new BlockBasedTableFactory(table_options)); const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); c.Finish(options, ioptions, moptions, table_options, ikc, &keys, &kvmap); diff --git a/tools/check_format_compatible.sh b/tools/check_format_compatible.sh index e4343a626c5..c0be0439ba9 100755 --- a/tools/check_format_compatible.sh +++ b/tools/check_format_compatible.sh @@ -125,7 +125,7 @@ EOF # To check for DB forward compatibility with loading options (old version # reading data from new), as well as backward compatibility -declare -a db_forward_with_options_refs=("6.27.fb" "6.28.fb" "6.29.fb" "7.0.fb" "7.1.fb" "7.2.fb" "7.3.fb" "7.4.fb" "7.5.fb" "7.6.fb" "7.7.fb" "7.8.fb" "7.9.fb" "7.10.fb" "8.0.fb" "8.1.fb" "8.2.fb" "8.3.fb" "8.4.fb" "8.5.fb" "8.6.fb" "8.7.fb" "8.8.fb" "8.9.fb" "8.10.fb" "8.11.fb") +declare -a db_forward_with_options_refs=("8.6.fb" "8.7.fb" "8.8.fb" "8.9.fb" "8.10.fb" "8.11.fb") # To check for DB forward compatibility without loading options (in addition # to the "with loading options" set), as well as backward compatibility declare -a db_forward_no_options_refs=() # N/A at the moment @@ -133,7 +133,7 @@ declare -a db_forward_no_options_refs=() # N/A at the moment # To check for SST ingestion backward compatibility (new version reading # data from old) (ldb ingest_extern_sst added in 5.16.x, back-ported to # 5.14.x, 5.15.x) -declare -a ext_backward_only_refs=("5.14.fb" "5.15.fb" "5.16.fb" "5.17.fb" "5.18.fb" "6.0.fb" "6.1.fb" "6.2.fb" "6.3.fb" "6.4.fb" "6.5.fb" "6.6.fb" "6.7.fb" "6.8.fb" "6.9.fb" "6.10.fb" "6.11.fb" "6.12.fb" "6.13.fb" "6.14.fb" "6.15.fb" "6.16.fb" "6.17.fb" "6.18.fb" "6.19.fb" "6.20.fb" "6.21.fb" "6.22.fb" "6.23.fb" "6.24.fb" "6.25.fb" "6.26.fb") +declare -a ext_backward_only_refs=("5.14.fb" "5.15.fb" "5.16.fb" "5.17.fb" "5.18.fb" "6.0.fb" "6.1.fb" "6.2.fb" "6.3.fb" "6.4.fb" "6.5.fb" "6.6.fb" "6.7.fb" "6.8.fb" "6.9.fb" "6.10.fb" "6.11.fb" "6.12.fb" "6.13.fb" "6.14.fb" "6.15.fb" "6.16.fb" "6.17.fb" "6.18.fb" "6.19.fb" "6.20.fb" "6.21.fb" "6.22.fb" "6.23.fb" "6.24.fb" "6.25.fb" "6.26.fb" "6.27.fb" "6.28.fb" "6.29.fb" "7.0.fb" "7.1.fb" "7.2.fb" "7.3.fb" "7.4.fb" "7.5.fb" "7.6.fb" "7.7.fb" "7.8.fb" "7.9.fb" "7.10.fb" "8.0.fb" "8.1.fb" "8.2.fb" "8.3.fb" "8.4.fb" "8.5.fb") # To check for SST ingestion forward compatibility (old version reading # data from new) as well as backward compatibility declare -a ext_forward_refs=("${db_forward_no_options_refs[@]}" "${db_forward_with_options_refs[@]}") diff --git a/unreleased_history/behavior_changes/format6.md b/unreleased_history/behavior_changes/format6.md new file mode 100644 index 00000000000..e52df488fc0 --- /dev/null +++ b/unreleased_history/behavior_changes/format6.md @@ -0,0 +1 @@ +format\_version=6 is the new default setting in BlockBasedTableOptions, for more robust data integrity checking. DBs and SST files written with this setting cannot be read by RocksDB versions before 8.6.0.