Feedback: fix API comment && remove unnecessary check

This commit is contained in:
Hui Xiao 2022-05-16 16:12:02 -07:00
parent eeff4a8ffd
commit 6a51af16b3
3 changed files with 39 additions and 34 deletions

View File

@ -317,8 +317,8 @@ struct BlockBasedTableOptions {
// `options_overrides[role].f != kFallback`, we use // `options_overrides[role].f != kFallback`, we use
// `options_overrides[role].f` // `options_overrides[role].f`
// 2. Otherwise, if `options[role].f != kFallback`, we use `options[role].f` // 2. Otherwise, if `options[role].f != kFallback`, we use `options[role].f`
// 3. Otherwise, we follow the compatible existing behavior for `f` (see below // 3. Otherwise, we follow the compatible existing behavior for `f` (see
// for more) // each feature's comment for more)
// //
// `cache_usage_options` currently supports specifying options for the // `cache_usage_options` currently supports specifying options for the
// following features: // following features:
@ -329,42 +329,55 @@ struct BlockBasedTableOptions {
// available), by updating a dynamical charge to the block cache loosely based // available), by updating a dynamical charge to the block cache loosely based
// on the actual memory usage of that area. // on the actual memory usage of that area.
// //
// Existing behavior: // (a) CacheEntryRole::kCompressionDictionaryBuildingBuffer
// (1) CacheEntryRole::kCompressionDictionaryBuildingBuffer : kEnabled // (i) If kEnabled:
// Charge memory usage of the buffered data used as training samples for // Charge memory usage of the buffered data used as training samples for
// dictionary compression. // dictionary compression.
// If such memory usage exceeds the avaible space left in the block cache // If such memory usage exceeds the avaible space left in the block cache
// at some point (i.e, causing a cache full under // at some point (i.e, causing a cache full under
// `LRUCacheOptions::strict_capacity_limit` = true), the data will then be // `LRUCacheOptions::strict_capacity_limit` = true), the data will then be
// unbuffered. // unbuffered.
// (ii) If kDisabled:
// Does not charge the memory usage mentioned above.
// (iii) Compatible existing behavior:
// Same as kEnabled.
// //
// (2) CacheEntryRole::kFilterConstruction : kDisabled // (b) CacheEntryRole::kFilterConstruction
// Same as kDisabled - does not charge memory usage of Bloom Filter // (i) If kEnabled:
// (format_version >= 5) and Ribbon Filter construction. If enabled and if // Charge memory usage of Bloom Filter
// additional temporary memory of Ribbon Filter exceeds the avaible space left // (format_version >= 5) and Ribbon Filter construction.
// in the block cache at some point (i.e, causing a cache full under // If additional temporary memory of Ribbon Filter exceeds the avaible
// `LRUCacheOptions::strict_capacity_limit` = true), construction will fall // space left in the block cache at some point (i.e, causing a cache full
// back to Bloom Filter. // under `LRUCacheOptions::strict_capacity_limit` = true),
// construction will fall back to Bloom Filter.
// (ii) If kDisabled:
// Does not charge the memory usage mentioned above.
// (iii) Compatible existing behavior:
// Same as kDisabled.
// //
// (3) CacheEntryRole::kBlockBasedTableReader : kDisabled // (c) CacheEntryRole::kBlockBasedTableReader
// Same as kDisabled - does not charge memory usage of table properties + // (i) If kEnabled:
// index block/filter block/uncompression dictionary when stored in table // Charge memory usage of table properties +
// reader (i.e, BlockBasedTableOptions::cache_index_and_filter_blocks == // index block/filter block/uncompression dictionary (when stored in table
// false) // reader i.e, BlockBasedTableOptions::cache_index_and_filter_blocks ==
// + some internal data structures during table reader creation. // false) + some internal data structures during table reader creation.
// If enabled and if such a table reader exceeds // If such a table reader exceeds
// the avaible space left in the block cache at some point (i.e, causing // the avaible space left in the block cache at some point (i.e, causing
// a cache full under `LRUCacheOptions::strict_capacity_limit` = true), // a cache full under `LRUCacheOptions::strict_capacity_limit` = true),
// creation will fail with Status::MemoryLimit(). // creation will fail with Status::MemoryLimit().
// (ii) If kDisabled:
// Does not charge the memory usage mentioned above.
// (iii) Compatible existing behavior:
// Same as kDisabled.
// //
// (4) Other CacheEntryRole : Not supported // (d) Other CacheEntryRole
// Not supported.
// `Status::kNotSupported` will be returned if // `Status::kNotSupported` will be returned if
// `CacheEntryRoleOptions::charged` is set to {`kEnabled`, `kDisabled`}. // `CacheEntryRoleOptions::charged` is set to {`kEnabled`, `kDisabled`}.
// //
// //
// 2. More to come ... // 2. More to come ...
// //
//
CacheUsageOptions cache_usage_options; CacheUsageOptions cache_usage_options;
// Note: currently this option requires kTwoLevelIndexSearch to be set as // Note: currently this option requires kTwoLevelIndexSearch to be set as

View File

@ -462,14 +462,10 @@ struct BlockBasedTableBuilder::Rep {
compression_opts.max_dict_buffer_bytes); compression_opts.max_dict_buffer_bytes);
} }
const auto options_overrides_iter =
table_options.cache_usage_options.options_overrides.find(
CacheEntryRole::kCompressionDictionaryBuildingBuffer);
const auto compress_dict_build_buffer_charged = const auto compress_dict_build_buffer_charged =
options_overrides_iter != table_options.cache_usage_options.options_overrides
table_options.cache_usage_options.options_overrides.end() .at(CacheEntryRole::kCompressionDictionaryBuildingBuffer)
? options_overrides_iter->second.charged .charged;
: table_options.cache_usage_options.options.charged;
if (table_options.block_cache && if (table_options.block_cache &&
(compress_dict_build_buffer_charged == (compress_dict_build_buffer_charged ==
CacheEntryRoleOptions::Decision::kEnabled || CacheEntryRoleOptions::Decision::kEnabled ||

View File

@ -422,14 +422,10 @@ BlockBasedTableFactory::BlockBasedTableFactory(
InitializeOptions(); InitializeOptions();
RegisterOptions(&table_options_, &block_based_table_type_info); RegisterOptions(&table_options_, &block_based_table_type_info);
const auto options_overrides_iter =
table_options_.cache_usage_options.options_overrides.find(
CacheEntryRole::kBlockBasedTableReader);
const auto table_reader_charged = const auto table_reader_charged =
options_overrides_iter != table_options_.cache_usage_options.options_overrides
table_options_.cache_usage_options.options_overrides.end() .at(CacheEntryRole::kBlockBasedTableReader)
? options_overrides_iter->second.charged .charged;
: table_options_.cache_usage_options.options.charged;
if (table_options_.block_cache && if (table_options_.block_cache &&
table_reader_charged == CacheEntryRoleOptions::Decision::kEnabled) { table_reader_charged == CacheEntryRoleOptions::Decision::kEnabled) {
table_reader_cache_res_mgr_.reset(new ConcurrentCacheReservationManager( table_reader_cache_res_mgr_.reset(new ConcurrentCacheReservationManager(