diff --git a/db/compact_files_test.cc b/db/compact_files_test.cc index 129b29c99ff2..95b812df6557 100644 --- a/db/compact_files_test.cc +++ b/db/compact_files_test.cc @@ -441,6 +441,49 @@ TEST_F(CompactFilesTest, SentinelCompressionType) { } } +TEST_F(CompactFilesTest, CompressionWithBlockAlign) { + Options options; + options.compression = CompressionType::kNoCompression; + options.create_if_missing = true; + options.disable_auto_compactions = true; + + std::shared_ptr collector(new FlushedFileCollector()); + options.listeners.push_back(collector); + + { + BlockBasedTableOptions bbto; + bbto.block_align = true; + options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + } + + std::unique_ptr db; + { + DB* _db = nullptr; + ASSERT_OK(DB::Open(options, db_name_, &_db)); + db.reset(_db); + } + + ASSERT_OK(db->Put(WriteOptions(), "key", "val")); + ASSERT_OK(db->Flush(FlushOptions())); + + // Ensure background work is fully finished including listener callbacks + // before accessing listener state. + ASSERT_OK( + static_cast_with_check(db.get())->TEST_WaitForBackgroundWork()); + auto l0_files = collector->GetFlushedFiles(); + ASSERT_EQ(1, l0_files.size()); + + // We can run this test even without Snappy support because we expect the + // `CompactFiles()` to fail before actually invoking Snappy compression. + CompactionOptions compaction_opts; + compaction_opts.compression = CompressionType::kSnappyCompression; + ASSERT_TRUE(db->CompactFiles(compaction_opts, l0_files, 1 /* output_level */) + .IsInvalidArgument()); + + compaction_opts.compression = CompressionType::kDisableCompressionOption; + ASSERT_OK(db->CompactFiles(compaction_opts, l0_files, 1 /* output_level */)); +} + TEST_F(CompactFilesTest, GetCompactionJobInfo) { Options options; options.create_if_missing = true; diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index eb2ae4ddba9d..fc4610756406 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -626,6 +626,15 @@ struct BlockBasedTableBuilder::Rep { } else { base_context_checksum = 0; } + + if (alignment > 0 && compression_type != kNoCompression) { + // With better sanitization in upper layers + // (`BlockBasedTableFactory::ValidateOptions()` and + // `CompactionPicker::CompactFiles()`), we would not need to handle this + // case here and could change it to an assertion instead. + SetStatus(Status::InvalidArgument( + "Enable block_align, but compression enabled")); + } } Rep(const Rep&) = delete; diff --git a/table/table_test.cc b/table/table_test.cc index ba44b95b3270..6d17a5fa7f2c 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -3188,6 +3188,7 @@ TEST_P(BlockBasedTableTest, BlockCacheLookupSeqScans) { Options options; BlockBasedTableOptions table_options = GetBlockBasedTableOptions(); options.create_if_missing = true; + options.compression = kNoCompression; options.statistics = CreateDBStatistics(); table_options.index_type = BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch; @@ -3196,6 +3197,8 @@ TEST_P(BlockBasedTableTest, BlockCacheLookupSeqScans) { table_options.filter_policy.reset(NewBloomFilterPolicy(10, true)); table_options.block_align = true; options.table_factory.reset(new BlockBasedTableFactory(table_options)); + ASSERT_OK(options.table_factory->ValidateOptions( + DBOptions(options), ColumnFamilyOptions(options))); TableConstructor c(BytewiseComparator()); GenerateKVMap(&c); @@ -3326,6 +3329,7 @@ TEST_P(BlockBasedTableTest, BlockCacheLookupAsyncScansSeek) { std::unique_ptr env( new CompositeEnvWrapper(c.env_, FileSystem::Default())); options.env = env.get(); + options.compression = kNoCompression; options.statistics = CreateDBStatistics(); c.env_ = env.get(); @@ -3338,6 +3342,8 @@ TEST_P(BlockBasedTableTest, BlockCacheLookupAsyncScansSeek) { table_options.filter_policy.reset(NewBloomFilterPolicy(10, true)); table_options.block_align = true; options.table_factory.reset(new BlockBasedTableFactory(table_options)); + ASSERT_OK(options.table_factory->ValidateOptions( + DBOptions(options), ColumnFamilyOptions(options))); GenerateKVMap(&c); @@ -5425,6 +5431,8 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) { Options options; options.compression = kNoCompression; options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + ASSERT_OK(options.table_factory->ValidateOptions( + DBOptions(options), ColumnFamilyOptions(options))); const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options); InternalKeyComparator ikc(options.comparator); @@ -5475,7 +5483,10 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) { std::unique_ptr table_reader; bbto.block_align = false; Options options2; + options2.compression = kNoCompression; options2.table_factory.reset(NewBlockBasedTableFactory(bbto)); + ASSERT_OK(options2.table_factory->ValidateOptions( + DBOptions(options2), ColumnFamilyOptions(options2))); ImmutableOptions ioptions2(options2); const MutableCFOptions moptions2(options2); @@ -5514,6 +5525,8 @@ TEST_P(BlockBasedTableTest, FixBlockAlignMismatchedFileChecksums) { bbto.block_align = true; bbto.block_size = 1024; options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + ASSERT_OK(options.table_factory->ValidateOptions( + DBOptions(options), ColumnFamilyOptions(options))); const std::string kDBPath = test::PerThreadDBPath("block_align_padded_bytes_verify_file_checksums"); ASSERT_OK(DestroyDB(kDBPath, options)); @@ -5539,6 +5552,8 @@ TEST_P(BlockBasedTableTest, PropertiesBlockRestartPointTest) { Options options; options.compression = kNoCompression; options.table_factory.reset(NewBlockBasedTableFactory(bbto)); + ASSERT_OK(options.table_factory->ValidateOptions( + DBOptions(options), ColumnFamilyOptions(options))); const ImmutableOptions ioptions(options); const MutableCFOptions moptions(options);