Remove skip_table_builder_flush and default it to true

Summary:
This option is needed to be enabled for Direct IO
and I cannot think of a reason where we need to disable it

remove it and default it to true
Closes https://github.com/facebook/rocksdb/pull/1944

Differential Revision: D4641088

Pulled By: IslamAbdelRahman

fbshipit-source-id: d7085b9
This commit is contained in:
Islam AbdelRahman 2017-03-02 16:45:07 -08:00 committed by Facebook Github Bot
parent cc253982dd
commit f89b3893c0
10 changed files with 4 additions and 40 deletions

View File

@ -1439,11 +1439,6 @@ void rocksdb_block_based_options_set_pin_l0_filter_and_index_blocks_in_cache(
options->rep.pin_l0_filter_and_index_blocks_in_cache = v; options->rep.pin_l0_filter_and_index_blocks_in_cache = v;
} }
void rocksdb_block_based_options_set_skip_table_builder_flush(
rocksdb_block_based_table_options_t* options, unsigned char v) {
options->rep.skip_table_builder_flush = v;
}
void rocksdb_options_set_block_based_table_factory( void rocksdb_options_set_block_based_table_factory(
rocksdb_options_t *opt, rocksdb_options_t *opt,
rocksdb_block_based_table_options_t* table_options) { rocksdb_block_based_table_options_t* table_options) {

View File

@ -130,7 +130,6 @@
[TableOptions/BlockBasedTable "default"] [TableOptions/BlockBasedTable "default"]
format_version=2 format_version=2
whole_key_filtering=true whole_key_filtering=true
skip_table_builder_flush=false
no_block_cache=false no_block_cache=false
checksum=kCRC32c checksum=kCRC32c
filter_policy=rocksdb.BuiltinBloomFilter filter_policy=rocksdb.BuiltinBloomFilter

View File

@ -502,9 +502,6 @@ rocksdb_block_based_options_set_cache_index_and_filter_blocks(
extern ROCKSDB_LIBRARY_API void extern ROCKSDB_LIBRARY_API void
rocksdb_block_based_options_set_pin_l0_filter_and_index_blocks_in_cache( rocksdb_block_based_options_set_pin_l0_filter_and_index_blocks_in_cache(
rocksdb_block_based_table_options_t*, unsigned char); rocksdb_block_based_table_options_t*, unsigned char);
extern ROCKSDB_LIBRARY_API void
rocksdb_block_based_options_set_skip_table_builder_flush(
rocksdb_block_based_table_options_t* options, unsigned char);
extern ROCKSDB_LIBRARY_API void rocksdb_options_set_block_based_table_factory( extern ROCKSDB_LIBRARY_API void rocksdb_options_set_block_based_table_factory(
rocksdb_options_t* opt, rocksdb_block_based_table_options_t* table_options); rocksdb_options_t* opt, rocksdb_block_based_table_options_t* table_options);

View File

@ -163,20 +163,6 @@ struct BlockBasedTableOptions {
// This must generally be true for gets to be efficient. // This must generally be true for gets to be efficient.
bool whole_key_filtering = true; bool whole_key_filtering = true;
// If true, block will not be explicitly flushed to disk during building
// a SstTable. Instead, buffer in WritableFileWriter will take
// care of the flushing when it is full.
//
// This option helps a lot when direct I/O writes
// (use_direct_writes = true) is used, since it avoids small
// direct disk write.
//
// User may also adjust writable_file_max_buffer_size to optimize disk I/O
// size.
//
// Default: false
bool skip_table_builder_flush = false;
// Verify that decompressing the compressed block gives back the input. This // Verify that decompressing the compressed block gives back the input. This
// is a verification mode that we use to detect bugs in compression // is a verification mode that we use to detect bugs in compression
// algorithms. // algorithms.

View File

@ -781,9 +781,6 @@ void BlockBasedTableBuilder::Flush() {
if (!ok()) return; if (!ok()) return;
if (r->data_block.empty()) return; if (r->data_block.empty()) return;
WriteBlock(&r->data_block, &r->pending_handle, true /* is_data_block */); WriteBlock(&r->data_block, &r->pending_handle, true /* is_data_block */);
if (ok() && !r->table_options.skip_table_builder_flush) {
r->status = r->file->Flush();
}
if (r->filter_block != nullptr) { if (r->filter_block != nullptr) {
r->filter_block->StartBlock(r->offset); r->filter_block->StartBlock(r->offset);
} }

View File

@ -189,9 +189,6 @@ std::string BlockBasedTableFactory::GetPrintableTableOptions() const {
snprintf(buffer, kBufferSize, " whole_key_filtering: %d\n", snprintf(buffer, kBufferSize, " whole_key_filtering: %d\n",
table_options_.whole_key_filtering); table_options_.whole_key_filtering);
ret.append(buffer); ret.append(buffer);
snprintf(buffer, kBufferSize, " skip_table_builder_flush: %d\n",
table_options_.skip_table_builder_flush);
ret.append(buffer);
snprintf(buffer, kBufferSize, " format_version: %d\n", snprintf(buffer, kBufferSize, " format_version: %d\n",
table_options_.format_version); table_options_.format_version);
ret.append(buffer); ret.append(buffer);

View File

@ -425,9 +425,6 @@ DEFINE_int32(random_access_max_buffer_size, 1024 * 1024,
DEFINE_int32(writable_file_max_buffer_size, 1024 * 1024, DEFINE_int32(writable_file_max_buffer_size, 1024 * 1024,
"Maximum write buffer for Writable File"); "Maximum write buffer for Writable File");
DEFINE_bool(skip_table_builder_flush, false, "Skip flushing block in "
"table builder ");
DEFINE_int32(bloom_bits, -1, "Bloom filter bits per key. Negative means" DEFINE_int32(bloom_bits, -1, "Bloom filter bits per key. Negative means"
" use default settings."); " use default settings.");
DEFINE_double(memtable_bloom_size_ratio, 0, DEFINE_double(memtable_bloom_size_ratio, 0,
@ -2917,8 +2914,6 @@ class Benchmark {
block_based_options.index_block_restart_interval = block_based_options.index_block_restart_interval =
FLAGS_index_block_restart_interval; FLAGS_index_block_restart_interval;
block_based_options.filter_policy = filter_policy_; block_based_options.filter_policy = filter_policy_;
block_based_options.skip_table_builder_flush =
FLAGS_skip_table_builder_flush;
block_based_options.format_version = 2; block_based_options.format_version = 2;
block_based_options.read_amp_bytes_per_bit = FLAGS_read_amp_bytes_per_bit; block_based_options.read_amp_bytes_per_bit = FLAGS_read_amp_bytes_per_bit;
if (FLAGS_read_cache_path != "") { if (FLAGS_read_cache_path != "") {

View File

@ -646,8 +646,8 @@ static std::unordered_map<std::string, OptionTypeInfo>
{offsetof(struct BlockBasedTableOptions, whole_key_filtering), {offsetof(struct BlockBasedTableOptions, whole_key_filtering),
OptionType::kBoolean, OptionVerificationType::kNormal, false, 0}}, OptionType::kBoolean, OptionVerificationType::kNormal, false, 0}},
{"skip_table_builder_flush", {"skip_table_builder_flush",
{offsetof(struct BlockBasedTableOptions, skip_table_builder_flush), {0, OptionType::kBoolean, OptionVerificationType::kDeprecated, false,
OptionType::kBoolean, OptionVerificationType::kNormal, false, 0}}, 0}},
{"format_version", {"format_version",
{offsetof(struct BlockBasedTableOptions, format_version), {offsetof(struct BlockBasedTableOptions, format_version),
OptionType::kUInt32T, OptionVerificationType::kNormal, false, 0}}, OptionType::kUInt32T, OptionVerificationType::kNormal, false, 0}},

View File

@ -158,7 +158,7 @@ TEST_F(OptionsSettableTest, BlockBasedTableOptionsAllFieldsSettable) {
"index_per_partition=4;" "index_per_partition=4;"
"index_block_restart_interval=4;" "index_block_restart_interval=4;"
"filter_policy=bloomfilter:4:true;whole_key_filtering=1;" "filter_policy=bloomfilter:4:true;whole_key_filtering=1;"
"skip_table_builder_flush=1;format_version=1;" "format_version=1;"
"hash_index_allow_collision=false;" "hash_index_allow_collision=false;"
"verify_compression=true;read_amp_bytes_per_bit=0", "verify_compression=true;read_amp_bytes_per_bit=0",
new_bbto)); new_bbto));

View File

@ -437,8 +437,7 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) {
"checksum=kxxHash;hash_index_allow_collision=1;no_block_cache=1;" "checksum=kxxHash;hash_index_allow_collision=1;no_block_cache=1;"
"block_cache=1M;block_cache_compressed=1k;block_size=1024;" "block_cache=1M;block_cache_compressed=1k;block_size=1024;"
"block_size_deviation=8;block_restart_interval=4;" "block_size_deviation=8;block_restart_interval=4;"
"filter_policy=bloomfilter:4:true;whole_key_filtering=1;" "filter_policy=bloomfilter:4:true;whole_key_filtering=1;",
"skip_table_builder_flush=1",
&new_opt)); &new_opt));
ASSERT_TRUE(new_opt.cache_index_and_filter_blocks); ASSERT_TRUE(new_opt.cache_index_and_filter_blocks);
ASSERT_EQ(new_opt.index_type, BlockBasedTableOptions::kHashSearch); ASSERT_EQ(new_opt.index_type, BlockBasedTableOptions::kHashSearch);
@ -453,7 +452,6 @@ TEST_F(OptionsTest, GetBlockBasedTableOptionsFromString) {
ASSERT_EQ(new_opt.block_size_deviation, 8); ASSERT_EQ(new_opt.block_size_deviation, 8);
ASSERT_EQ(new_opt.block_restart_interval, 4); ASSERT_EQ(new_opt.block_restart_interval, 4);
ASSERT_TRUE(new_opt.filter_policy != nullptr); ASSERT_TRUE(new_opt.filter_policy != nullptr);
ASSERT_TRUE(new_opt.skip_table_builder_flush);
// unknown option // unknown option
ASSERT_NOK(GetBlockBasedTableOptionsFromString(table_opt, ASSERT_NOK(GetBlockBasedTableOptionsFromString(table_opt,