Skip to content

Commit

Permalink
Remove a couple deprecated convenience.h APIs (facebook#11120)
Browse files Browse the repository at this point in the history
Summary:
**Context/Summary:**
As instructed by convenience.h comments, a few deprecated APIs are removed.

Pull Request resolved: facebook#11120

Test Plan:
- make check & CI
- eyeball check on test semantics.

Reviewed By: pdillinger

Differential Revision: D42937507

Pulled By: hx235

fbshipit-source-id: a9e4709387da01b1d0e9148c2e210f02e9746ee1
  • Loading branch information
hx235 authored and facebook-github-bot committed Feb 7, 2023
1 parent b5827c8 commit 6650ca2
Show file tree
Hide file tree
Showing 8 changed files with 301 additions and 354 deletions.
1 change: 1 addition & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
### Public API Changes
* Completely removed the following deprecated/obsolete statistics: the tickers `BLOCK_CACHE_INDEX_BYTES_EVICT`, `BLOCK_CACHE_FILTER_BYTES_EVICT`, `BLOOM_FILTER_MICROS`, `NO_FILE_CLOSES`, `STALL_L0_SLOWDOWN_MICROS`, `STALL_MEMTABLE_COMPACTION_MICROS`, `STALL_L0_NUM_FILES_MICROS`, `RATE_LIMIT_DELAY_MILLIS`, `NO_ITERATORS`, `NUMBER_FILTERED_DELETES`, `WRITE_TIMEDOUT`, `BLOB_DB_GC_NUM_KEYS_OVERWRITTEN`, `BLOB_DB_GC_NUM_KEYS_EXPIRED`, `BLOB_DB_GC_BYTES_OVERWRITTEN`, `BLOB_DB_GC_BYTES_EXPIRED`, `BLOCK_CACHE_COMPRESSION_DICT_BYTES_EVICT` as well as the histograms `STALL_L0_SLOWDOWN_COUNT`, `STALL_MEMTABLE_COMPACTION_COUNT`, `STALL_L0_NUM_FILES_COUNT`, `HARD_RATE_LIMIT_DELAY_COUNT`, `SOFT_RATE_LIMIT_DELAY_COUNT`, `BLOB_DB_GC_MICROS`, and `NUM_DATA_BLOCKS_READ_PER_LEVEL`. Note that as a result, the C++ enum values of the still supported statistics have changed. Developers are advised to not rely on the actual numeric values.
* Deprecated IngestExternalFileOptions::write_global_seqno and change default to false. This option only needs to be set to true to generate a DB compatible with RocksDB versions before 5.16.0.
* Remove deprecated APIs `GetColumnFamilyOptionsFrom{Map|String}(const ColumnFamilyOptions&, ..)`, `GetDBOptionsFrom{Map|String}(const DBOptions&, ..)`, `GetBlockBasedTableOptionsFrom{Map|String}(const BlockBasedTableOptions& table_options, ..)` and ` GetPlainTableOptionsFrom{Map|String}(const PlainTableOptions& table_options,..)`.

### Build Changes
* The `make` build now builds a shared library by default instead of a static library. Use `LIB_MODE=static` to override.
Expand Down
91 changes: 18 additions & 73 deletions include/rocksdb/convenience.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,10 @@ struct ConfigOptions {
// "kCompactionStyleNone".
//

// Take a default ColumnFamilyOptions "base_options" in addition to a
// map "opts_map" of option name to option value to construct the new
// ColumnFamilyOptions "new_options".
// Take a ConfigOptions `config_options` and a ColumnFamilyOptions
// "base_options" as the default option in addition to a map "opts_map" of
// option name to option value to construct the new ColumnFamilyOptions
// "new_options".
//
// Below are the instructions of how to config some non-primitive-typed
// options in ColumnFamilyOptions:
Expand Down Expand Up @@ -238,11 +239,6 @@ struct ConfigOptions {
// cf_opt.compression_opts.strategy = 6;
// cf_opt.compression_opts.max_dict_bytes = 7;
//
// The GetColumnFamilyOptionsFromMap(ConfigOptions, ...) should be used; the
// alternative signature may be deprecated in a future release. The equivalent
// functionality can be achieved by setting the corresponding options in
// the ConfigOptions parameter.
//
// @param config_options controls how the map is processed.
// @param base_options the default options of the output "new_options".
// @param opts_map an option name to value map for specifying how "new_options"
Expand All @@ -267,15 +263,10 @@ Status GetColumnFamilyOptionsFromMap(
const ColumnFamilyOptions& base_options,
const std::unordered_map<std::string, std::string>& opts_map,
ColumnFamilyOptions* new_options);
Status GetColumnFamilyOptionsFromMap(
const ColumnFamilyOptions& base_options,
const std::unordered_map<std::string, std::string>& opts_map,
ColumnFamilyOptions* new_options, bool input_strings_escaped = false,
bool ignore_unknown_options = false);

// Take a default DBOptions "base_options" in addition to a
// map "opts_map" of option name to option value to construct the new
// DBOptions "new_options".
// Take a ConfigOptions `config_options` and a DBOptions "base_options" as the
// default option in addition to a map "opts_map" of option name to option value
// to construct the new DBOptions "new_options".
//
// Below are the instructions of how to config some non-primitive-typed
// options in DBOptions:
Expand All @@ -286,11 +277,6 @@ Status GetColumnFamilyOptionsFromMap(
// - Passing {"rate_limiter_bytes_per_sec", "1024"} is equivalent to
// passing NewGenericRateLimiter(1024) to rate_limiter_bytes_per_sec.
//
// The GetDBOptionsFromMap(ConfigOptions, ...) should be used; the
// alternative signature may be deprecated in a future release. The equivalent
// functionality can be achieved by setting the corresponding options in
// the ConfigOptions parameter.
//
// @param config_options controls how the map is processed.
// @param base_options the default options of the output "new_options".
// @param opts_map an option name to value map for specifying how "new_options"
Expand All @@ -314,15 +300,11 @@ Status GetDBOptionsFromMap(
const ConfigOptions& cfg_options, const DBOptions& base_options,
const std::unordered_map<std::string, std::string>& opts_map,
DBOptions* new_options);
Status GetDBOptionsFromMap(
const DBOptions& base_options,
const std::unordered_map<std::string, std::string>& opts_map,
DBOptions* new_options, bool input_strings_escaped = false,
bool ignore_unknown_options = false);

// Take a default BlockBasedTableOptions "table_options" in addition to a
// map "opts_map" of option name to option value to construct the new
// BlockBasedTableOptions "new_table_options".
// Take a ConfigOptions `config_options` and a BlockBasedTableOptions
// "table_options" as the default option in addition to a map "opts_map" of
// option name to option value to construct the new BlockBasedTableOptions
// "new_table_options".
//
// Below are the instructions of how to config some non-primitive-typed
// options in BlockBasedTableOptions:
Expand All @@ -345,11 +327,6 @@ Status GetDBOptionsFromMap(
// - Passing {"block_cache", "1M"} in GetBlockBasedTableOptionsFromMap is
// equivalent to setting block_cache using NewLRUCache(1024 * 1024).
//
// The GetBlockBasedTableOptionsFromMap(ConfigOptions, ...) should be used;
// the alternative signature may be deprecated in a future release. The
// equivalent functionality can be achieved by setting the corresponding
// options in the ConfigOptions parameter.
//
// @param config_options controls how the map is processed.
// @param table_options the default options of the output "new_table_options".
// @param opts_map an option name to value map for specifying how
Expand All @@ -369,20 +346,11 @@ Status GetBlockBasedTableOptionsFromMap(
const BlockBasedTableOptions& table_options,
const std::unordered_map<std::string, std::string>& opts_map,
BlockBasedTableOptions* new_table_options);
Status GetBlockBasedTableOptionsFromMap(
const BlockBasedTableOptions& table_options,
const std::unordered_map<std::string, std::string>& opts_map,
BlockBasedTableOptions* new_table_options,
bool input_strings_escaped = false, bool ignore_unknown_options = false);

// Take a default PlainTableOptions "table_options" in addition to a
// map "opts_map" of option name to option value to construct the new
// PlainTableOptions "new_table_options".
//
// The GetPlainTableOptionsFromMap(ConfigOptions, ...) should be used; the
// alternative signature may be deprecated in a future release. The equivalent
// functionality can be achieved by setting the corresponding options in
// the ConfigOptions parameter.
// Take a ConfigOptions `config_options` and a default PlainTableOptions
// "table_options" as the default option in addition to a map "opts_map" of
// option name to option value to construct the new PlainTableOptions
// "new_table_options".
//
// @param config_options controls how the map is processed.
// @param table_options the default options of the output "new_table_options".
Expand All @@ -402,43 +370,26 @@ Status GetPlainTableOptionsFromMap(
const ConfigOptions& config_options, const PlainTableOptions& table_options,
const std::unordered_map<std::string, std::string>& opts_map,
PlainTableOptions* new_table_options);
Status GetPlainTableOptionsFromMap(
const PlainTableOptions& table_options,
const std::unordered_map<std::string, std::string>& opts_map,
PlainTableOptions* new_table_options, bool input_strings_escaped = false,
bool ignore_unknown_options = false);

// Take a string representation of option names and values, apply them into the
// base_options, and return the new options as a result. The string has the
// following format:
// Take a ConfigOptions `config_options`, a string representation of option
// names and values, apply them into the base_options, and return the new
// options as a result. The string has the following format:
// "write_buffer_size=1024;max_write_buffer_number=2"
// Nested options config is also possible. For example, you can define
// BlockBasedTableOptions as part of the string for block-based table factory:
// "write_buffer_size=1024;block_based_table_factory={block_size=4k};"
// "max_write_buffer_num=2"
//
//
// The GetColumnFamilyOptionsFromString(ConfigOptions, ...) should be used; the
// alternative signature may be deprecated in a future release. The equivalent
// functionality can be achieved by setting the corresponding options in
// the ConfigOptions parameter.
Status GetColumnFamilyOptionsFromString(const ConfigOptions& config_options,
const ColumnFamilyOptions& base_options,
const std::string& opts_str,
ColumnFamilyOptions* new_options);
Status GetColumnFamilyOptionsFromString(const ColumnFamilyOptions& base_options,
const std::string& opts_str,
ColumnFamilyOptions* new_options);

Status GetDBOptionsFromString(const ConfigOptions& config_options,
const DBOptions& base_options,
const std::string& opts_str,
DBOptions* new_options);

Status GetDBOptionsFromString(const DBOptions& base_options,
const std::string& opts_str,
DBOptions* new_options);

Status GetStringFromDBOptions(const ConfigOptions& config_options,
const DBOptions& db_options,
std::string* opts_str);
Expand All @@ -458,17 +409,11 @@ Status GetStringFromCompressionType(std::string* compression_str,

std::vector<CompressionType> GetSupportedCompressions();

Status GetBlockBasedTableOptionsFromString(
const BlockBasedTableOptions& table_options, const std::string& opts_str,
BlockBasedTableOptions* new_table_options);
Status GetBlockBasedTableOptionsFromString(
const ConfigOptions& config_options,
const BlockBasedTableOptions& table_options, const std::string& opts_str,
BlockBasedTableOptions* new_table_options);

Status GetPlainTableOptionsFromString(const PlainTableOptions& table_options,
const std::string& opts_str,
PlainTableOptions* new_table_options);
Status GetPlainTableOptionsFromString(const ConfigOptions& config_options,
const PlainTableOptions& table_options,
const std::string& opts_str,
Expand Down
12 changes: 10 additions & 2 deletions java/rocksjni/options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3990,9 +3990,13 @@ jlong Java_org_rocksdb_ColumnFamilyOptions_getColumnFamilyOptionsFromProps__Ljav
}

auto* cf_options = new ROCKSDB_NAMESPACE::ColumnFamilyOptions();
ROCKSDB_NAMESPACE::ConfigOptions config_options;
config_options.input_strings_escaped = false;
config_options.ignore_unknown_options = false;
ROCKSDB_NAMESPACE::Status status =
ROCKSDB_NAMESPACE::GetColumnFamilyOptionsFromString(
ROCKSDB_NAMESPACE::ColumnFamilyOptions(), opt_string, cf_options);
config_options, ROCKSDB_NAMESPACE::ColumnFamilyOptions(), opt_string,
cf_options);

env->ReleaseStringUTFChars(jopt_string, opt_string);

Expand Down Expand Up @@ -5848,9 +5852,13 @@ jlong Java_org_rocksdb_DBOptions_getDBOptionsFromProps__Ljava_lang_String_2(
return 0;
}

const ROCKSDB_NAMESPACE::DBOptions base_options;
auto* db_options = new ROCKSDB_NAMESPACE::DBOptions();
ROCKSDB_NAMESPACE::ConfigOptions config_options(base_options);
config_options.input_strings_escaped = false;
config_options.ignore_unknown_options = false;
ROCKSDB_NAMESPACE::Status status = ROCKSDB_NAMESPACE::GetDBOptionsFromString(
ROCKSDB_NAMESPACE::DBOptions(), opt_string, db_options);
config_options, base_options, opt_string, db_options);

env->ReleaseStringUTFChars(jopt_string, opt_string);

Expand Down
46 changes: 0 additions & 46 deletions options/options_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -658,18 +658,6 @@ Status GetStringFromCompressionType(std::string* compression_str,
}
}

Status GetColumnFamilyOptionsFromMap(
const ColumnFamilyOptions& base_options,
const std::unordered_map<std::string, std::string>& opts_map,
ColumnFamilyOptions* new_options, bool input_strings_escaped,
bool ignore_unknown_options) {
ConfigOptions config_options;
config_options.ignore_unknown_options = ignore_unknown_options;
config_options.input_strings_escaped = input_strings_escaped;
return GetColumnFamilyOptionsFromMap(config_options, base_options, opts_map,
new_options);
}

Status GetColumnFamilyOptionsFromMap(
const ConfigOptions& config_options,
const ColumnFamilyOptions& base_options,
Expand All @@ -691,17 +679,6 @@ Status GetColumnFamilyOptionsFromMap(
}
}

Status GetColumnFamilyOptionsFromString(
const ColumnFamilyOptions& base_options,
const std::string& opts_str,
ColumnFamilyOptions* new_options) {
ConfigOptions config_options;
config_options.input_strings_escaped = false;
config_options.ignore_unknown_options = false;
return GetColumnFamilyOptionsFromString(config_options, base_options,
opts_str, new_options);
}

Status GetColumnFamilyOptionsFromString(const ConfigOptions& config_options,
const ColumnFamilyOptions& base_options,
const std::string& opts_str,
Expand All @@ -716,18 +693,6 @@ Status GetColumnFamilyOptionsFromString(const ConfigOptions& config_options,
new_options);
}

Status GetDBOptionsFromMap(
const DBOptions& base_options,
const std::unordered_map<std::string, std::string>& opts_map,
DBOptions* new_options, bool input_strings_escaped,
bool ignore_unknown_options) {
ConfigOptions config_options(base_options);
config_options.input_strings_escaped = input_strings_escaped;
config_options.ignore_unknown_options = ignore_unknown_options;
return GetDBOptionsFromMap(config_options, base_options, opts_map,
new_options);
}

Status GetDBOptionsFromMap(
const ConfigOptions& config_options, const DBOptions& base_options,
const std::unordered_map<std::string, std::string>& opts_map,
Expand All @@ -746,17 +711,6 @@ Status GetDBOptionsFromMap(
}
}

Status GetDBOptionsFromString(const DBOptions& base_options,
const std::string& opts_str,
DBOptions* new_options) {
ConfigOptions config_options(base_options);
config_options.input_strings_escaped = false;
config_options.ignore_unknown_options = false;

return GetDBOptionsFromString(config_options, base_options, opts_str,
new_options);
}

Status GetDBOptionsFromString(const ConfigOptions& config_options,
const DBOptions& base_options,
const std::string& opts_str,
Expand Down
17 changes: 14 additions & 3 deletions options/options_settable_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,8 +167,13 @@ TEST_F(OptionsSettableTest, BlockBasedTableOptionsAllFieldsSettable) {
kBbtoExcluded);

// Need to update the option string if a new option is added.
ConfigOptions config_options;
config_options.input_strings_escaped = false;
config_options.ignore_unknown_options = false;
config_options.invoke_prepare_options = false;
config_options.ignore_unsupported_options = false;
ASSERT_OK(GetBlockBasedTableOptionsFromString(
*bbto,
config_options, *bbto,
"cache_index_and_filter_blocks=1;"
"cache_index_and_filter_blocks_with_high_priority=true;"
"metadata_cache_options={top_level_index_pinning=kFallback;"
Expand Down Expand Up @@ -273,8 +278,11 @@ TEST_F(OptionsSettableTest, DBOptionsAllFieldsSettable) {
FillWithSpecialChar(new_options_ptr, sizeof(DBOptions), kDBOptionsExcluded);

// Need to update the option string if a new option is added.
ConfigOptions config_options(*options);
config_options.input_strings_escaped = false;
config_options.ignore_unknown_options = false;
ASSERT_OK(
GetDBOptionsFromString(*options,
GetDBOptionsFromString(config_options, *options,
"wal_bytes_per_sync=4295048118;"
"delete_obsolete_files_period_micros=4294967758;"
"WAL_ttl_seconds=4295008036;"
Expand Down Expand Up @@ -461,8 +469,11 @@ TEST_F(OptionsSettableTest, ColumnFamilyOptionsAllFieldsSettable) {
kColumnFamilyOptionsExcluded);

// Need to update the option string if a new option is added.
ConfigOptions config_options;
config_options.input_strings_escaped = false;
config_options.ignore_unknown_options = false;
ASSERT_OK(GetColumnFamilyOptionsFromString(
*options,
config_options, *options,
"compaction_filter_factory=mpudlojcujCompactionFilterFactory;"
"table_factory=PlainTable;"
"prefix_extractor=rocksdb.CappedPrefix.13;"
Expand Down
Loading

0 comments on commit 6650ca2

Please sign in to comment.