Prevent the user from setting block_restart_interval to less than 1

Summary: If block_restart_interval gets set to less than 1 an assert will be triggered in BlockBuilder::BlockBuilder().  This prevents the user from doing this by silently setting any value less than 1 to 1.

Test Plan: Added a test (in BlockBasedTableTest in table_test) that checks invalid values to make sure that they are reset to the expected values.  The block_restart_interval value is checked along with block_size_deviation which also silently sets the value if it is outside a specific range.

Reviewers: yoshinorim, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D52509
main
Jay Edgar 9 years ago
parent 4041903ecd
commit 7699439b7c
  1. 3
      include/rocksdb/table.h
  2. 3
      table/block_based_table_factory.cc
  3. 44
      table/table_test.cc

@ -116,7 +116,8 @@ struct BlockBasedTableOptions {
// Number of keys between restart points for delta encoding of keys.
// This parameter can be changed dynamically. Most clients should
// leave this parameter alone.
// leave this parameter alone. The minimum value allowed is 1. Any smaller
// value will be silently overwritten with 1.
int block_restart_interval = 16;
// Use delta encoding to compress keys in blocks.

@ -39,6 +39,9 @@ BlockBasedTableFactory::BlockBasedTableFactory(
table_options_.block_size_deviation > 100) {
table_options_.block_size_deviation = 0;
}
if (table_options_.block_restart_interval < 1) {
table_options_.block_restart_interval = 1;
}
}
Status BlockBasedTableFactory::NewTableReader(

@ -1698,6 +1698,50 @@ TEST_F(BlockBasedTableTest, FilterBlockInBlockCache) {
props.AssertFilterBlockStat(0, 0);
}
void ValidateBlockSizeDeviation(int value, int expected) {
BlockBasedTableOptions table_options;
table_options.block_size_deviation = value;
BlockBasedTableFactory* factory = new BlockBasedTableFactory(table_options);
const BlockBasedTableOptions* normalized_table_options =
(const BlockBasedTableOptions*)factory->GetOptions();
ASSERT_EQ(normalized_table_options->block_size_deviation, expected);
delete factory;
}
void ValidateBlockRestartInterval(int value, int expected) {
BlockBasedTableOptions table_options;
table_options.block_restart_interval = value;
BlockBasedTableFactory* factory = new BlockBasedTableFactory(table_options);
const BlockBasedTableOptions* normalized_table_options =
(const BlockBasedTableOptions*)factory->GetOptions();
ASSERT_EQ(normalized_table_options->block_restart_interval, expected);
delete factory;
}
TEST_F(BlockBasedTableTest, InvalidOptions) {
// invalid values for block_size_deviation (<0 or >100) are silently set to 0
ValidateBlockSizeDeviation(-10, 0);
ValidateBlockSizeDeviation(-1, 0);
ValidateBlockSizeDeviation(0, 0);
ValidateBlockSizeDeviation(1, 1);
ValidateBlockSizeDeviation(99, 99);
ValidateBlockSizeDeviation(100, 100);
ValidateBlockSizeDeviation(101, 0);
ValidateBlockSizeDeviation(1000, 0);
// invalid values for block_restart_interval (<1) are silently set to 1
ValidateBlockRestartInterval(-10, 1);
ValidateBlockRestartInterval(-1, 1);
ValidateBlockRestartInterval(0, 1);
ValidateBlockRestartInterval(1, 1);
ValidateBlockRestartInterval(2, 2);
ValidateBlockRestartInterval(1000, 1000);
}
TEST_F(BlockBasedTableTest, BlockReadCountTest) {
// bloom_filter_type = 0 -- block-based filter
// bloom_filter_type = 0 -- full filter

Loading…
Cancel
Save