From c20a7cd6c7669fd7a493ff9ed71edf03bcf08c8d Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Thu, 25 Mar 2021 14:58:23 -0700 Subject: [PATCH] Apply `sample_for_compression` to all block-based tables (#8105) Summary: Previously it only applied to block-based tables generated by flush. This restriction was undocumented and blocked a new use case. Now compression sampling applies to all block-based tables we generate when it is enabled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/8105 Test Plan: new unit test Reviewed By: riversand963 Differential Revision: D27317275 Pulled By: ajkr fbshipit-source-id: cd9fcc5178d6515e8cb59c6facb5ac01893cb5b0 --- HISTORY.md | 3 + db/builder.cc | 26 ++--- db/builder.h | 2 - db/compaction/compaction_job.cc | 1 - db/db_impl/db_impl_open.cc | 1 - db/db_properties_test.cc | 107 ++++++++++++++++++ db/flush_job.cc | 3 +- db/repair.cc | 13 +-- db/table_properties_collector.cc | 8 +- db/table_properties_collector.h | 12 +- db/table_properties_collector_test.cc | 9 +- db/version_set_test.cc | 2 +- include/rocksdb/table_properties.h | 6 +- .../block_based/block_based_table_builder.cc | 22 ++-- table/block_based/block_based_table_builder.h | 1 - .../block_based/block_based_table_factory.cc | 1 - .../block_based_table_reader_test.cc | 6 +- .../block_based/data_block_hash_index_test.cc | 5 +- table/block_fetcher_test.cc | 6 +- table/meta_blocks.cc | 8 +- table/meta_blocks.h | 4 +- table/sst_file_dumper.cc | 8 +- table/sst_file_writer.cc | 9 +- table/sst_file_writer_collectors.h | 6 +- table/table_builder.h | 4 +- table/table_reader_bench.cc | 5 +- table/table_test.cc | 52 ++++----- tools/sst_dump_test.cc | 5 +- 28 files changed, 211 insertions(+), 124 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index bc327af13..b00eb2143 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,5 +1,8 @@ # Rocksdb Change Log ## Unreleased +### Behavior Changes +* `ColumnFamilyOptions::sample_for_compression` now takes effect for creation of all block-based tables. Previously it only took effect for block-based tables created by flush. + ### Bug Fixes * Use thread-safe `strerror_r()` to get error messages. diff --git a/db/builder.cc b/db/builder.cc index ad30be735..0e41884ae 100644 --- a/db/builder.cc +++ b/db/builder.cc @@ -52,8 +52,8 @@ TableBuilder* NewTableBuilder( int_tbl_prop_collector_factories, uint32_t column_family_id, const std::string& column_family_name, WritableFileWriter* file, const CompressionType compression_type, - uint64_t sample_for_compression, const CompressionOptions& compression_opts, - int level, const bool skip_filters, const uint64_t creation_time, + const CompressionOptions& compression_opts, int level, + const bool skip_filters, const uint64_t creation_time, const uint64_t oldest_key_time, const uint64_t target_file_size, const uint64_t file_creation_time, const std::string& db_id, const std::string& db_session_id) { @@ -63,10 +63,10 @@ TableBuilder* NewTableBuilder( return ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, internal_comparator, int_tbl_prop_collector_factories, compression_type, - sample_for_compression, compression_opts, - skip_filters, column_family_name, level, - creation_time, oldest_key_time, target_file_size, - file_creation_time, db_id, db_session_id), + compression_opts, skip_filters, column_family_name, + level, creation_time, oldest_key_time, + target_file_size, file_creation_time, db_id, + db_session_id), column_family_id, file); } @@ -85,11 +85,10 @@ Status BuildTable( std::vector snapshots, SequenceNumber earliest_write_conflict_snapshot, SnapshotChecker* snapshot_checker, const CompressionType compression, - uint64_t sample_for_compression, const CompressionOptions& compression_opts, - bool paranoid_file_checks, InternalStats* internal_stats, - TableFileCreationReason reason, IOStatus* io_status, - const std::shared_ptr& io_tracer, EventLogger* event_logger, - int job_id, const Env::IOPriority io_priority, + const CompressionOptions& compression_opts, bool paranoid_file_checks, + InternalStats* internal_stats, TableFileCreationReason reason, + IOStatus* io_status, const std::shared_ptr& io_tracer, + EventLogger* event_logger, int job_id, const Env::IOPriority io_priority, TableProperties* table_properties, int level, const uint64_t creation_time, const uint64_t oldest_key_time, Env::WriteLifeTimeHint write_hint, const uint64_t file_creation_time, const std::string& db_id, @@ -163,9 +162,8 @@ Status BuildTable( builder = NewTableBuilder( ioptions, mutable_cf_options, internal_comparator, int_tbl_prop_collector_factories, column_family_id, - column_family_name, file_writer.get(), compression, - sample_for_compression, compression_opts, level, - false /* skip_filters */, creation_time, oldest_key_time, + column_family_name, file_writer.get(), compression, compression_opts, + level, false /* skip_filters */, creation_time, oldest_key_time, 0 /*target_file_size*/, file_creation_time, db_id, db_session_id); } diff --git a/db/builder.h b/db/builder.h index d7a064fc2..e090e4420 100644 --- a/db/builder.h +++ b/db/builder.h @@ -50,7 +50,6 @@ TableBuilder* NewTableBuilder( int_tbl_prop_collector_factories, uint32_t column_family_id, const std::string& column_family_name, WritableFileWriter* file, const CompressionType compression_type, - const uint64_t sample_for_compression, const CompressionOptions& compression_opts, int level, const bool skip_filters = false, const uint64_t creation_time = 0, const uint64_t oldest_key_time = 0, const uint64_t target_file_size = 0, @@ -80,7 +79,6 @@ extern Status BuildTable( std::vector snapshots, SequenceNumber earliest_write_conflict_snapshot, SnapshotChecker* snapshot_checker, const CompressionType compression, - const uint64_t sample_for_compression, const CompressionOptions& compression_opts, bool paranoid_file_checks, InternalStats* internal_stats, TableFileCreationReason reason, IOStatus* io_status, const std::shared_ptr& io_tracer, diff --git a/db/compaction/compaction_job.cc b/db/compaction/compaction_job.cc index 2dc6167d8..6db50531e 100644 --- a/db/compaction/compaction_job.cc +++ b/db/compaction/compaction_job.cc @@ -1770,7 +1770,6 @@ Status CompactionJob::OpenCompactionOutputFile( cfd->internal_comparator(), cfd->int_tbl_prop_collector_factories(), cfd->GetID(), cfd->GetName(), sub_compact->outfile.get(), sub_compact->compaction->output_compression(), - 0 /*sample_for_compression */, sub_compact->compaction->output_compression_opts(), sub_compact->compaction->output_level(), skip_filters, oldest_ancester_time, 0 /* oldest_key_time */, diff --git a/db/db_impl/db_impl_open.cc b/db/db_impl/db_impl_open.cc index b8b26eeb0..c417ecba8 100644 --- a/db/db_impl/db_impl_open.cc +++ b/db/db_impl/db_impl_open.cc @@ -1358,7 +1358,6 @@ Status DBImpl::WriteLevel0TableForRecovery(int job_id, ColumnFamilyData* cfd, cfd->GetID(), cfd->GetName(), snapshot_seqs, earliest_write_conflict_snapshot, snapshot_checker, GetCompressionFlush(*cfd->ioptions(), mutable_cf_options), - mutable_cf_options.sample_for_compression, mutable_cf_options.compression_opts, paranoid_file_checks, cfd->internal_stats(), TableFileCreationReason::kRecovery, &io_s, io_tracer_, &event_logger_, job_id, Env::IO_HIGH, diff --git a/db/db_properties_test.cc b/db/db_properties_test.cc index d3333fa93..6f4eb777a 100644 --- a/db/db_properties_test.cc +++ b/db/db_properties_test.cc @@ -1175,6 +1175,61 @@ class CountingDeleteTabPropCollectorFactory } }; +class BlockCountingTablePropertiesCollector : public TablePropertiesCollector { + public: + static const std::string kNumSampledBlocksPropertyName; + + const char* Name() const override { + return "BlockCountingTablePropertiesCollector"; + } + + Status Finish(UserCollectedProperties* properties) override { + (*properties)[kNumSampledBlocksPropertyName] = + ToString(num_sampled_blocks_); + return Status::OK(); + } + + Status AddUserKey(const Slice& /*user_key*/, const Slice& /*value*/, + EntryType /*type*/, SequenceNumber /*seq*/, + uint64_t /*file_size*/) override { + return Status::OK(); + } + + void BlockAdd(uint64_t /* block_raw_bytes */, + uint64_t block_compressed_bytes_fast, + uint64_t block_compressed_bytes_slow) override { + if (block_compressed_bytes_fast > 0 || block_compressed_bytes_slow > 0) { + num_sampled_blocks_++; + } + } + + UserCollectedProperties GetReadableProperties() const override { + return UserCollectedProperties{ + {kNumSampledBlocksPropertyName, ToString(num_sampled_blocks_)}, + }; + } + + private: + uint32_t num_sampled_blocks_ = 0; +}; + +const std::string + BlockCountingTablePropertiesCollector::kNumSampledBlocksPropertyName = + "NumSampledBlocks"; + +class BlockCountingTablePropertiesCollectorFactory + : public TablePropertiesCollectorFactory { + public: + const char* Name() const override { + return "BlockCountingTablePropertiesCollectorFactory"; + } + + TablePropertiesCollector* CreateTablePropertiesCollector( + TablePropertiesCollectorFactory::Context /* context */) override { + return new BlockCountingTablePropertiesCollector(); + } +}; + #ifndef ROCKSDB_LITE TEST_F(DBPropertiesTest, GetUserDefinedTableProperties) { Options options = CurrentOptions(); @@ -1413,6 +1468,58 @@ TEST_F(DBPropertiesTest, NeedCompactHintPersistentTest) { } } +// Excluded from RocksDB lite tests due to `GetPropertiesOfAllTables()` usage. +TEST_F(DBPropertiesTest, BlockAddForCompressionSampling) { + // Sampled compression requires at least one of the following four types. + if (!Snappy_Supported() && !Zlib_Supported() && !LZ4_Supported() && + !ZSTD_Supported()) { + return; + } + + Options options = CurrentOptions(); + options.disable_auto_compactions = true; + options.table_properties_collector_factories.emplace_back( + std::make_shared()); + + for (bool sample_for_compression : {false, true}) { + // For simplicity/determinism, sample 100% when enabled, or 0% when disabled + options.sample_for_compression = sample_for_compression ? 1 : 0; + + DestroyAndReopen(options); + + // Setup the following LSM: + // + // L0_0 ["a", "b"] + // L1_0 ["a", "b"] + // + // L0_0 was created by flush. L1_0 was created by compaction. Each file + // contains one data block. + for (int i = 0; i < 3; ++i) { + ASSERT_OK(Put("a", "val")); + ASSERT_OK(Put("b", "val")); + ASSERT_OK(Flush()); + if (i == 1) { + ASSERT_OK(db_->CompactRange(CompactRangeOptions(), nullptr, nullptr)); + } + } + + // A `BlockAdd()` should have been seen for files generated by flush or + // compaction when `sample_for_compression` is enabled. + TablePropertiesCollection file_to_props; + ASSERT_OK(db_->GetPropertiesOfAllTables(&file_to_props)); + ASSERT_EQ(2, file_to_props.size()); + for (const auto& file_and_props : file_to_props) { + auto& user_props = file_and_props.second->user_collected_properties; + ASSERT_TRUE(user_props.find(BlockCountingTablePropertiesCollector:: + kNumSampledBlocksPropertyName) != + user_props.end()); + ASSERT_EQ(user_props.at(BlockCountingTablePropertiesCollector:: + kNumSampledBlocksPropertyName), + ToString(sample_for_compression ? 1 : 0)); + } + } +} + TEST_F(DBPropertiesTest, EstimateNumKeysUnderflow) { Options options = CurrentOptions(); Reopen(options); diff --git a/db/flush_job.cc b/db/flush_job.cc index 4f137b984..fc8f8a9d3 100644 --- a/db/flush_job.cc +++ b/db/flush_job.cc @@ -411,8 +411,7 @@ Status FlushJob::WriteLevel0Table() { cfd_->internal_comparator(), cfd_->int_tbl_prop_collector_factories(), cfd_->GetID(), cfd_->GetName(), existing_snapshots_, earliest_write_conflict_snapshot_, snapshot_checker_, - output_compression_, mutable_cf_options_.sample_for_compression, - mutable_cf_options_.compression_opts, + output_compression_, mutable_cf_options_.compression_opts, mutable_cf_options_.paranoid_file_checks, cfd_->internal_stats(), TableFileCreationReason::kFlush, &io_s, io_tracer_, event_logger_, job_context_->job_id, Env::IO_HIGH, &table_properties_, 0 /* level */, diff --git a/db/repair.cc b/db/repair.cc index df41719df..ea415dfe8 100644 --- a/db/repair.cc +++ b/db/repair.cc @@ -447,13 +447,12 @@ class Repairer { nullptr /* blob_file_additions */, cfd->internal_comparator(), cfd->int_tbl_prop_collector_factories(), cfd->GetID(), cfd->GetName(), {}, kMaxSequenceNumber, snapshot_checker, kNoCompression, - 0 /* sample_for_compression */, CompressionOptions(), false, - nullptr /* internal_stats */, TableFileCreationReason::kRecovery, - &io_s, nullptr /*IOTracer*/, nullptr /* event_logger */, - 0 /* job_id */, Env::IO_HIGH, nullptr /* table_properties */, - -1 /* level */, current_time, 0 /* oldest_key_time */, write_hint, - 0 /* file_creation_time */, "DB Repairer" /* db_id */, - db_session_id_); + CompressionOptions(), false, nullptr /* internal_stats */, + TableFileCreationReason::kRecovery, &io_s, nullptr /*IOTracer*/, + nullptr /* event_logger */, 0 /* job_id */, Env::IO_HIGH, + nullptr /* table_properties */, -1 /* level */, current_time, + 0 /* oldest_key_time */, write_hint, 0 /* file_creation_time */, + "DB Repairer" /* db_id */, db_session_id_); ROCKS_LOG_INFO(db_options_.info_log, "Log #%" PRIu64 ": %d ops saved to Table #%" PRIu64 " %s", log, counter, meta.fd.GetNumber(), diff --git a/db/table_properties_collector.cc b/db/table_properties_collector.cc index d877a52c8..fdf48c927 100644 --- a/db/table_properties_collector.cc +++ b/db/table_properties_collector.cc @@ -43,10 +43,10 @@ Status UserKeyTablePropertiesCollector::InternalAdd(const Slice& key, } void UserKeyTablePropertiesCollector::BlockAdd( - uint64_t bLockRawBytes, uint64_t blockCompressedBytesFast, - uint64_t blockCompressedBytesSlow) { - return collector_->BlockAdd(bLockRawBytes, blockCompressedBytesFast, - blockCompressedBytesSlow); + uint64_t block_raw_bytes, uint64_t block_compressed_bytes_fast, + uint64_t block_compressed_bytes_slow) { + return collector_->BlockAdd(block_raw_bytes, block_compressed_bytes_fast, + block_compressed_bytes_slow); } Status UserKeyTablePropertiesCollector::Finish( diff --git a/db/table_properties_collector.h b/db/table_properties_collector.h index 130eb64d4..23a2853ac 100644 --- a/db/table_properties_collector.h +++ b/db/table_properties_collector.h @@ -27,9 +27,9 @@ class IntTblPropCollector { virtual Status InternalAdd(const Slice& key, const Slice& value, uint64_t file_size) = 0; - virtual void BlockAdd(uint64_t blockRawBytes, - uint64_t blockCompressedBytesFast, - uint64_t blockCompressedBytesSlow) = 0; + virtual void BlockAdd(uint64_t block_raw_bytes, + uint64_t block_compressed_bytes_fast, + uint64_t block_compressed_bytes_slow) = 0; virtual UserCollectedProperties GetReadableProperties() const = 0; @@ -64,9 +64,9 @@ class UserKeyTablePropertiesCollector : public IntTblPropCollector { virtual Status InternalAdd(const Slice& key, const Slice& value, uint64_t file_size) override; - virtual void BlockAdd(uint64_t blockRawBytes, - uint64_t blockCompressedBytesFast, - uint64_t blockCompressedBytesSlow) override; + virtual void BlockAdd(uint64_t block_raw_bytes, + uint64_t block_compressed_bytes_fast, + uint64_t block_compressed_bytes_slow) override; virtual Status Finish(UserCollectedProperties* properties) override; diff --git a/db/table_properties_collector_test.cc b/db/table_properties_collector_test.cc index 4777bdd5e..39a6844b4 100644 --- a/db/table_properties_collector_test.cc +++ b/db/table_properties_collector_test.cc @@ -55,8 +55,7 @@ void MakeBuilder(const Options& options, const ImmutableCFOptions& ioptions, builder->reset(NewTableBuilder( ioptions, moptions, internal_comparator, int_tbl_prop_collector_factories, kTestColumnFamilyId, kTestColumnFamilyName, writable->get(), - options.compression, options.sample_for_compression, - options.compression_opts, unknown_level)); + options.compression, options.compression_opts, unknown_level)); } } // namespace @@ -176,9 +175,9 @@ class RegularKeysStartWithAInternal : public IntTblPropCollector { return Status::OK(); } - void BlockAdd(uint64_t /* blockRawBytes */, - uint64_t /* blockCompressedBytesFast */, - uint64_t /* blockCompressedBytesSlow */) override { + void BlockAdd(uint64_t /* block_raw_bytes */, + uint64_t /* block_compressed_bytes_fast */, + uint64_t /* block_compressed_bytes_slow */) override { // Nothing to do. return; } diff --git a/db/version_set_test.cc b/db/version_set_test.cc index c91b87293..204484dc7 100644 --- a/db/version_set_test.cc +++ b/db/version_set_test.cc @@ -2781,7 +2781,7 @@ class VersionSetTestMissingFiles : public VersionSetTestBase, TableBuilderOptions( immutable_cf_options_, mutable_cf_options_, *internal_comparator_, &int_tbl_prop_collector_factories, kNoCompression, - /*_sample_for_compression=*/0, CompressionOptions(), + CompressionOptions(), /*_skip_filters=*/false, info.column_family, info.level), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, fwriter.get())); diff --git a/include/rocksdb/table_properties.h b/include/rocksdb/table_properties.h index 9f2b64866..524ebdaa6 100644 --- a/include/rocksdb/table_properties.h +++ b/include/rocksdb/table_properties.h @@ -99,9 +99,9 @@ class TablePropertiesCollector { } // Called after each new block is cut - virtual void BlockAdd(uint64_t /* blockRawBytes */, - uint64_t /* blockCompressedBytesFast */, - uint64_t /* blockCompressedBytesSlow */) { + virtual void BlockAdd(uint64_t /* block_raw_bytes */, + uint64_t /* block_compressed_bytes_fast */, + uint64_t /* block_compressed_bytes_slow */) { // Nothing to do here. Callback registers can override. return; } diff --git a/table/block_based/block_based_table_builder.cc b/table/block_based/block_based_table_builder.cc index c84444923..d62d58d0f 100644 --- a/table/block_based/block_based_table_builder.cc +++ b/table/block_based/block_based_table_builder.cc @@ -210,9 +210,9 @@ class BlockBasedTableBuilder::BlockBasedTablePropertiesCollector return Status::OK(); } - virtual void BlockAdd(uint64_t /* blockRawBytes */, - uint64_t /* blockCompressedBytesFast */, - uint64_t /* blockCompressedBytesSlow */) override { + virtual void BlockAdd(uint64_t /* block_raw_bytes */, + uint64_t /* block_compressed_bytes_fast */, + uint64_t /* block_compressed_bytes_slow */) override { // Intentionally left blank. No interest in collecting stats for // blocks. return; @@ -405,7 +405,6 @@ struct BlockBasedTableBuilder::Rep { int_tbl_prop_collector_factories, uint32_t _column_family_id, WritableFileWriter* f, const CompressionType _compression_type, - const uint64_t _sample_for_compression, const CompressionOptions& _compression_opts, const bool skip_filters, const int _level_at_creation, const std::string& _column_family_name, const uint64_t _creation_time, const uint64_t _oldest_key_time, @@ -431,7 +430,7 @@ struct BlockBasedTableBuilder::Rep { range_del_block(1 /* block_restart_interval */), internal_prefix_transform(_moptions.prefix_extractor.get()), compression_type(_compression_type), - sample_for_compression(_sample_for_compression), + sample_for_compression(_moptions.sample_for_compression), compression_opts(_compression_opts), compression_dict(), compression_ctxs(_compression_opts.parallel_threads), @@ -841,7 +840,6 @@ BlockBasedTableBuilder::BlockBasedTableBuilder( int_tbl_prop_collector_factories, uint32_t column_family_id, WritableFileWriter* file, const CompressionType compression_type, - const uint64_t sample_for_compression, const CompressionOptions& compression_opts, const bool skip_filters, const std::string& column_family_name, const int level_at_creation, const uint64_t creation_time, const uint64_t oldest_key_time, @@ -859,12 +857,12 @@ BlockBasedTableBuilder::BlockBasedTableBuilder( sanitized_table_options.format_version = 1; } - rep_ = new Rep( - ioptions, moptions, sanitized_table_options, internal_comparator, - int_tbl_prop_collector_factories, column_family_id, file, - compression_type, sample_for_compression, compression_opts, skip_filters, - level_at_creation, column_family_name, creation_time, oldest_key_time, - target_file_size, file_creation_time, db_id, db_session_id); + rep_ = new Rep(ioptions, moptions, sanitized_table_options, + internal_comparator, int_tbl_prop_collector_factories, + column_family_id, file, compression_type, compression_opts, + skip_filters, level_at_creation, column_family_name, + creation_time, oldest_key_time, target_file_size, + file_creation_time, db_id, db_session_id); if (rep_->filter_builder != nullptr) { rep_->filter_builder->StartBlock(0); diff --git a/table/block_based/block_based_table_builder.h b/table/block_based/block_based_table_builder.h index 99c80f843..a2bae6414 100644 --- a/table/block_based/block_based_table_builder.h +++ b/table/block_based/block_based_table_builder.h @@ -46,7 +46,6 @@ class BlockBasedTableBuilder : public TableBuilder { int_tbl_prop_collector_factories, uint32_t column_family_id, WritableFileWriter* file, const CompressionType compression_type, - const uint64_t sample_for_compression, const CompressionOptions& compression_opts, const bool skip_filters, const std::string& column_family_name, const int level_at_creation, const uint64_t creation_time = 0, const uint64_t oldest_key_time = 0, diff --git a/table/block_based/block_based_table_factory.cc b/table/block_based/block_based_table_factory.cc index 53e139a2d..d7ee41dc9 100644 --- a/table/block_based/block_based_table_factory.cc +++ b/table/block_based/block_based_table_factory.cc @@ -500,7 +500,6 @@ TableBuilder* BlockBasedTableFactory::NewTableBuilder( table_options_, table_builder_options.internal_comparator, table_builder_options.int_tbl_prop_collector_factories, column_family_id, file, table_builder_options.compression_type, - table_builder_options.sample_for_compression, table_builder_options.compression_opts, table_builder_options.skip_filters, table_builder_options.column_family_name, table_builder_options.level, diff --git a/table/block_based/block_based_table_reader_test.cc b/table/block_based/block_based_table_reader_test.cc index 30b979d38..1b91bd30a 100644 --- a/table/block_based/block_based_table_reader_test.cc +++ b/table/block_based/block_based_table_reader_test.cc @@ -66,9 +66,9 @@ class BlockBasedTableReaderTest std::vector> factories; std::unique_ptr table_builder(table_factory_->NewTableBuilder( TableBuilderOptions(ioptions, moptions, comparator, &factories, - compression_type, 0 /* sample_for_compression */, - CompressionOptions(), false /* skip_filters */, - kDefaultColumnFamilyName, -1 /* level */), + compression_type, CompressionOptions(), + false /* skip_filters */, kDefaultColumnFamilyName, + -1 /* level */), 0 /* column_family_id */, writer.get())); // Build table. diff --git a/table/block_based/data_block_hash_index_test.cc b/table/block_based/data_block_hash_index_test.cc index 0a4276fd3..e7c180728 100644 --- a/table/block_based/data_block_hash_index_test.cc +++ b/table/block_based/data_block_hash_index_test.cc @@ -557,9 +557,8 @@ void TestBoundary(InternalKey& ik1, std::string& v1, InternalKey& ik2, builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, internal_comparator, &int_tbl_prop_collector_factories, - options.compression, options.sample_for_compression, - CompressionOptions(), false /* skip_filters */, - column_family_name, level_), + options.compression, CompressionOptions(), + false /* skip_filters */, column_family_name, level_), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, file_writer.get())); diff --git a/table/block_fetcher_test.cc b/table/block_fetcher_test.cc index 9eba26beb..84043b165 100644 --- a/table/block_fetcher_test.cc +++ b/table/block_fetcher_test.cc @@ -100,9 +100,9 @@ class BlockFetcherTest : public testing::Test { std::vector> factories; std::unique_ptr table_builder(table_factory_.NewTableBuilder( TableBuilderOptions(ioptions, moptions, comparator, &factories, - compression_type, 0 /* sample_for_compression */, - CompressionOptions(), false /* skip_filters */, - kDefaultColumnFamilyName, -1 /* level */), + compression_type, CompressionOptions(), + false /* skip_filters */, kDefaultColumnFamilyName, + -1 /* level */), 0 /* column_family_id */, writer.get())); // Build table. diff --git a/table/meta_blocks.cc b/table/meta_blocks.cc index 4c41b7f86..5b5fb057d 100644 --- a/table/meta_blocks.cc +++ b/table/meta_blocks.cc @@ -172,11 +172,11 @@ bool NotifyCollectTableCollectorsOnAdd( void NotifyCollectTableCollectorsOnBlockAdd( const std::vector>& collectors, - const uint64_t blockRawBytes, const uint64_t blockCompressedBytesFast, - const uint64_t blockCompressedBytesSlow) { + const uint64_t block_raw_bytes, const uint64_t block_compressed_bytes_fast, + const uint64_t block_compressed_bytes_slow) { for (auto& collector : collectors) { - collector->BlockAdd(blockRawBytes, blockCompressedBytesFast, - blockCompressedBytesSlow); + collector->BlockAdd(block_raw_bytes, block_compressed_bytes_fast, + block_compressed_bytes_slow); } } diff --git a/table/meta_blocks.h b/table/meta_blocks.h index 319b2c712..4477246c6 100644 --- a/table/meta_blocks.h +++ b/table/meta_blocks.h @@ -86,8 +86,8 @@ bool NotifyCollectTableCollectorsOnAdd( void NotifyCollectTableCollectorsOnBlockAdd( const std::vector>& collectors, - uint64_t blockRawBytes, uint64_t blockCompressedBytesFast, - uint64_t blockCompressedBytesSlow); + uint64_t block_raw_bytes, uint64_t block_compressed_bytes_fast, + uint64_t block_compressed_bytes_slow); // NotifyCollectTableCollectorsOnFinish() triggers the `Finish` event for all // property collectors. The collected properties will be added to `builder`. diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc index 8b1e32114..ef51fb447 100644 --- a/table/sst_file_dumper.cc +++ b/table/sst_file_dumper.cc @@ -274,10 +274,10 @@ Status SstFileDumper::ShowCompressionSize( std::string column_family_name; int unknown_level = -1; - TableBuilderOptions tb_opts( - imoptions, moptions, ikc, &block_based_table_factories, compress_type, - 0 /* sample_for_compression */, compress_opt, false /* skip_filters */, - column_family_name, unknown_level); + TableBuilderOptions tb_opts(imoptions, moptions, ikc, + &block_based_table_factories, compress_type, + compress_opt, false /* skip_filters */, + column_family_name, unknown_level); uint64_t num_data_blocks = 0; std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); diff --git a/table/sst_file_writer.cc b/table/sst_file_writer.cc index bc4361aeb..301a254d1 100644 --- a/table/sst_file_writer.cc +++ b/table/sst_file_writer.cc @@ -210,8 +210,6 @@ Status SstFileWriter::Open(const std::string& file_path) { compression_type = r->mutable_cf_options.compression; compression_opts = r->mutable_cf_options.compression_opts; } - uint64_t sample_for_compression = - r->mutable_cf_options.sample_for_compression; std::vector> int_tbl_prop_collector_factories; @@ -252,10 +250,9 @@ Status SstFileWriter::Open(const std::string& file_path) { } TableBuilderOptions table_builder_options( r->ioptions, r->mutable_cf_options, r->internal_comparator, - &int_tbl_prop_collector_factories, compression_type, - sample_for_compression, compression_opts, r->skip_filters, - r->column_family_name, unknown_level, 0 /* creation_time */, - 0 /* oldest_key_time */, 0 /* target_file_size */, + &int_tbl_prop_collector_factories, compression_type, compression_opts, + r->skip_filters, r->column_family_name, unknown_level, + 0 /* creation_time */, 0 /* oldest_key_time */, 0 /* target_file_size */, 0 /* file_creation_time */, "SST Writer" /* db_id */, db_session_id); FileTypeSet tmp_set = r->ioptions.checksum_handoff_file_types; r->file_writer.reset(new WritableFileWriter( diff --git a/table/sst_file_writer_collectors.h b/table/sst_file_writer_collectors.h index 01ecec971..2dbd611ab 100644 --- a/table/sst_file_writer_collectors.h +++ b/table/sst_file_writer_collectors.h @@ -35,9 +35,9 @@ class SstFileWriterPropertiesCollector : public IntTblPropCollector { return Status::OK(); } - virtual void BlockAdd(uint64_t /* blockRawBytes */, - uint64_t /* blockCompressedBytesFast */, - uint64_t /* blockCompressedBytesSlow */) override { + virtual void BlockAdd(uint64_t /* block_raw_bytes */, + uint64_t /* block_compressed_bytes_fast */, + uint64_t /* block_compressed_bytes_slow */) override { // Intentionally left blank. No interest in collecting stats for // blocks. return; diff --git a/table/table_builder.h b/table/table_builder.h index 36475c143..047166d3a 100644 --- a/table/table_builder.h +++ b/table/table_builder.h @@ -92,7 +92,7 @@ struct TableBuilderOptions { const InternalKeyComparator& _internal_comparator, const std::vector>* _int_tbl_prop_collector_factories, - CompressionType _compression_type, uint64_t _sample_for_compression, + CompressionType _compression_type, const CompressionOptions& _compression_opts, bool _skip_filters, const std::string& _column_family_name, int _level, const uint64_t _creation_time = 0, const int64_t _oldest_key_time = 0, @@ -104,7 +104,6 @@ struct TableBuilderOptions { internal_comparator(_internal_comparator), int_tbl_prop_collector_factories(_int_tbl_prop_collector_factories), compression_type(_compression_type), - sample_for_compression(_sample_for_compression), compression_opts(_compression_opts), skip_filters(_skip_filters), column_family_name(_column_family_name), @@ -122,7 +121,6 @@ struct TableBuilderOptions { const std::vector>* int_tbl_prop_collector_factories; CompressionType compression_type; - uint64_t sample_for_compression; const CompressionOptions& compression_opts; bool skip_filters; // only used by BlockBasedTableBuilder const std::string& column_family_name; diff --git a/table/table_reader_bench.cc b/table/table_reader_bench.cc index 079029108..76559e5f3 100644 --- a/table/table_reader_bench.cc +++ b/table/table_reader_bench.cc @@ -102,9 +102,8 @@ void TableReaderBenchmark(Options& opts, EnvOptions& env_options, tb = opts.table_factory->NewTableBuilder( TableBuilderOptions( ioptions, moptions, ikc, &int_tbl_prop_collector_factories, - CompressionType::kNoCompression, 0 /* sample_for_compression */, - CompressionOptions(), false /* skip_filters */, - kDefaultColumnFamilyName, unknown_level), + CompressionType::kNoCompression, CompressionOptions(), + false /* skip_filters */, kDefaultColumnFamilyName, unknown_level), 0 /* column_family_id */, file_writer.get()); } else { s = DB::Open(opts, dbname, &db); diff --git a/table/table_test.cc b/table/table_test.cc index 74147ecbb..d35712670 100644 --- a/table/table_test.cc +++ b/table/table_test.cc @@ -363,9 +363,9 @@ class TableConstructor : public Constructor { builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, internal_comparator, &int_tbl_prop_collector_factories, - options.compression, options.sample_for_compression, - options.compression_opts, false /* skip_filters */, - column_family_name, level_), + options.compression, options.compression_opts, + false /* skip_filters */, column_family_name, + level_), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, file_writer_.get())); @@ -3325,9 +3325,8 @@ TEST_P(BlockBasedTableTest, NoFileChecksum) { builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, *comparator, &int_tbl_prop_collector_factories, - options.compression, options.sample_for_compression, - options.compression_opts, false /* skip_filters */, - column_family_name, level), + options.compression, options.compression_opts, + false /* skip_filters */, column_family_name, level), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, f.GetFileWriter())); ASSERT_OK(f.ResetTableBuilder(std::move(builder))); @@ -3364,9 +3363,8 @@ TEST_P(BlockBasedTableTest, Crc32cFileChecksum) { builder.reset(ioptions.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, *comparator, &int_tbl_prop_collector_factories, - options.compression, options.sample_for_compression, - options.compression_opts, false /* skip_filters */, - column_family_name, level), + options.compression, options.compression_opts, + false /* skip_filters */, column_family_name, level), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, f.GetFileWriter())); ASSERT_OK(f.ResetTableBuilder(std::move(builder))); @@ -3413,10 +3411,10 @@ TEST_F(PlainTableTest, BasicPlainTableProperties) { std::string column_family_name; int unknown_level = -1; std::unique_ptr builder(factory.NewTableBuilder( - TableBuilderOptions( - ioptions, moptions, ikc, &int_tbl_prop_collector_factories, - kNoCompression, 0 /* sample_for_compression */, CompressionOptions(), - false /* skip_filters */, column_family_name, unknown_level), + TableBuilderOptions(ioptions, moptions, ikc, + &int_tbl_prop_collector_factories, kNoCompression, + CompressionOptions(), false /* skip_filters */, + column_family_name, unknown_level), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, file_writer.get())); @@ -3470,10 +3468,10 @@ TEST_F(PlainTableTest, NoFileChecksum) { f.CreateWriteableFile(); std::unique_ptr builder(factory.NewTableBuilder( - TableBuilderOptions( - ioptions, moptions, ikc, &int_tbl_prop_collector_factories, - kNoCompression, 0 /* sample_for_compression */, CompressionOptions(), - false /* skip_filters */, column_family_name, unknown_level), + TableBuilderOptions(ioptions, moptions, ikc, + &int_tbl_prop_collector_factories, kNoCompression, + CompressionOptions(), false /* skip_filters */, + column_family_name, unknown_level), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, f.GetFileWriter())); ASSERT_OK(f.ResetTableBuilder(std::move(builder))); @@ -3512,10 +3510,10 @@ TEST_F(PlainTableTest, Crc32cFileChecksum) { f.SetFileChecksumGenerator(checksum_crc32c_gen1.release()); std::unique_ptr builder(factory.NewTableBuilder( - TableBuilderOptions( - ioptions, moptions, ikc, &int_tbl_prop_collector_factories, - kNoCompression, 0 /* sample_for_compression */, CompressionOptions(), - false /* skip_filters */, column_family_name, unknown_level), + TableBuilderOptions(ioptions, moptions, ikc, + &int_tbl_prop_collector_factories, kNoCompression, + CompressionOptions(), false /* skip_filters */, + column_family_name, unknown_level), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, f.GetFileWriter())); ASSERT_OK(f.ResetTableBuilder(std::move(builder))); @@ -4081,8 +4079,8 @@ TEST_P(BlockBasedTableTest, DISABLED_TableWithGlobalSeqno) { std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, ikc, &int_tbl_prop_collector_factories, kNoCompression, - 0 /* sample_for_compression */, CompressionOptions(), - false /* skip_filters */, column_family_name, -1), + CompressionOptions(), false /* skip_filters */, + column_family_name, -1), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, file_writer.get())); @@ -4267,8 +4265,8 @@ TEST_P(BlockBasedTableTest, BlockAlignTest) { std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, ikc, &int_tbl_prop_collector_factories, kNoCompression, - 0 /* sample_for_compression */, CompressionOptions(), - false /* skip_filters */, column_family_name, -1), + CompressionOptions(), false /* skip_filters */, + column_family_name, -1), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, file_writer.get())); @@ -4362,8 +4360,8 @@ TEST_P(BlockBasedTableTest, PropertiesBlockRestartPointTest) { std::unique_ptr builder(options.table_factory->NewTableBuilder( TableBuilderOptions(ioptions, moptions, ikc, &int_tbl_prop_collector_factories, kNoCompression, - 0 /* sample_for_compression */, CompressionOptions(), - false /* skip_filters */, column_family_name, -1), + CompressionOptions(), false /* skip_filters */, + column_family_name, -1), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, file_writer.get())); diff --git a/tools/sst_dump_test.cc b/tools/sst_dump_test.cc index d590830a8..6b693ecd1 100644 --- a/tools/sst_dump_test.cc +++ b/tools/sst_dump_test.cc @@ -113,9 +113,8 @@ class SSTDumpToolTest : public testing::Test { tb.reset(opts.table_factory->NewTableBuilder( TableBuilderOptions( imoptions, moptions, ikc, &int_tbl_prop_collector_factories, - CompressionType::kNoCompression, 0 /* sample_for_compression */, - CompressionOptions(), false /* skip_filters */, column_family_name, - unknown_level), + CompressionType::kNoCompression, CompressionOptions(), + false /* skip_filters */, column_family_name, unknown_level), TablePropertiesCollectorFactory::Context::kUnknownColumnFamily, file_writer.get()));