Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removing some deprecated APIs in 7.0 #9389

Closed
pdillinger opened this issue Jan 15, 2022 · 2 comments
Closed

Removing some deprecated APIs in 7.0 #9389

pdillinger opened this issue Jan 15, 2022 · 2 comments
Assignees
Labels
announcement rocksdb-7.0 PRs with breaking API changes that need to land in the next major release, 7.0.

Comments

@pdillinger
Copy link
Contributor

pdillinger commented Jan 15, 2022

In the upcoming 7.0 major release we plan to remove some APIs marked "deprecated." We have have not committed to which will or will not be removed, but let us know in comments if you have particular concerns (such as deprecated API with no workable alternative).

Deprecated options to remove:

  • max_mem_compaction_level
  • soft_rate_limit
  • hard_rate_limit
  • rate_limit_delay_max_milliseconds
  • purge_redundant_kvs_while_flush
  • base_background_compactions
  • skip_log_error_on_recovery
  • preserve_deletes
  • iter_start_seqnum
  • table_cache_remove_scan_count_limit

Deprecated APIs to remove:

  • ROCKSDB_DEPRECATED_FUNC DB::AddFile() overloads
  • ROCKSDB_DEPRECATED_FUNC DB::CompactRange() overloads
  • ROCKSDB_DEPRECATED_FUNC DB::GetApproximateSizes() overloads
@pdillinger pdillinger self-assigned this Jan 15, 2022
@pdillinger pdillinger added announcement rocksdb-7.0 PRs with breaking API changes that need to land in the next major release, 7.0. labels Jan 15, 2022
@pdillinger pdillinger changed the title Planned major release: removing some deprecated APIs Removing some deprecated APIs in 7.0 Jan 15, 2022
@riversand963
Copy link
Contributor

riversand963 commented Jan 18, 2022

Repeating here to boost this post:

This is the current list we plan to remove.

Deprecated options to remove:

max_mem_compaction_level

  • soft_rate_limit
  • hard_rate_limit
  • rate_limit_delay_max_milliseconds
  • purge_redundant_kvs_while_flush
  • base_background_compactions
  • skip_log_error_on_recovery
  • preserve_deletes
  • iter_start_seqnum
  • table_cache_remove_scan_count_limit

Deprecated APIs to remove:

  • ROCKSDB_DEPRECATED_FUNC DB::AddFile() overloads
  • ROCKSDB_DEPRECATED_FUNC DB::CompactRange() overloads
  • ROCKSDB_DEPRECATED_FUNC DB::GetApproximateSizes() overloads

@pdillinger
Copy link
Contributor Author

pdillinger commented Jan 22, 2022

(Will also update the description with these)

Deprecated override implementations to remove (derived classes must now implement):

Env::Name
EnvWrapper::Name
FileSystemWrapper::Name
Statistics::Name
FilterBitsBuilder::ApproximateNumEntries
FilterBitsBuilder::EstimateEntriesAdded
FilterPolicy::GetBuilderWithContext

Deprecated APIs to remove:

ObjectLibrary::Register (uses regexes)
FilterBitsBuilder::CalculateNumEntry
FilterPolicy::CreateFilter (for slow, deprecated block-based filter only)
FilterPolicy::KeyMayMatch (for slow, deprecated block-based filter only)
FilterPolicy::GetFilterBitsBuilder
NewExperimentalRibbonFilterPolicy

Deprecated type alias to remove:

BackupableDBOptions (now BackupEngineOptions)

  • C API: renamed rocksdb_backupable_db_* to rocksdb_backup_engine_*
  • Java API: renamed BackupableDBOptions to BackupEngineOptions

Deprecated headers to remove:

utilities/backupable_db.h (obsolete with backup_engine.h)
utilities/regex.h (obsolete with ObjectLibrary::Register removal)
utilities/utility_db.h (only contains obsolete)

pdillinger added a commit to pdillinger/rocksdb that referenced this issue Jan 25, 2022
Summary: This also removes the obsolete names BackupableDBOptions
and UtilityDB. API users must now use BackupEngineOptions and
DBWithTTL::Open. In C API, `rocksdb_backupable_db_*` is replaced
`rocksdb_backup_engine_*`. Similar renaming in Java API.

In reference to facebook#9389

Test Plan: CI
facebook-github-bot pushed a commit that referenced this issue Jan 27, 2022
…9439)

Summary:
Regexes are considered potentially problematic for use in
registering RocksDB extensions, so we are removing
ObjectLibrary::Register() and the Regex public API it depended on (now
unused).

In reference to #9389

Why?
* The power of Regexes can make it hard to reason about which extension
will match what. (The replacement API isn't perfect, but we are at least
"holding the line" on patterns we have seen in practice.)
* It is easy to make regexes that don't quite mean what you think they
mean, such as forgetting that the `.` in `foo.bar` can match any character
or that matching is nondeterministic, as in `a:b:42` matching `.*:[0-9]+`.
* Some regexes and implementations can have disastrously bad
performance. This might not be much practical concern for ObjectLibray
here, but we don't want to encourage potentially dangerous further use
in production code. (Testing code is fine. See TestRegex.)

Pull Request resolved: #9439

Test Plan: CI

Reviewed By: mrambacher

Differential Revision: D33792342

Pulled By: pdillinger

fbshipit-source-id: 4f64dcb04764e639162c8977a5fa196f67754cec
facebook-github-bot pushed a commit that referenced this issue Jan 27, 2022
Summary:
This also removes the obsolete names BackupableDBOptions
and UtilityDB. API users must now use BackupEngineOptions and
DBWithTTL::Open. In C API, `rocksdb_backupable_db_*` is replaced
`rocksdb_backup_engine_*`. Similar renaming in Java API.

In reference to #9389

Pull Request resolved: #9438

Test Plan: CI

Reviewed By: mrambacher

Differential Revision: D33780269

Pulled By: pdillinger

fbshipit-source-id: 4a6cfc5c1b4c78bcad790b9d3dd13c5fdf4a1fac
facebook-github-bot pushed a commit that referenced this issue Feb 8, 2022
Summary:
* Inefficient block-based filter is no longer customizable in the public
API, though (for now) can still be enabled.
  * Removed deprecated FilterPolicy::CreateFilter() and
  FilterPolicy::KeyMayMatch()
  * Removed `rocksdb_filterpolicy_create()` from C API
* Change meaning of nullptr return from GetBuilderWithContext() from "use
block-based filter" to "generate no filter in this case." This is a
cleaner solution to the proposal in #8250.
  * Also, when user specifies bits_per_key < 0.5, we now round this down
  to "no filter" because we expect a filter with >= 80% FP rate is
  unlikely to be worth the CPU cost of accessing it (esp with
  cache_index_and_filter_blocks=1 or partition_filters=1).
  * bits_per_key >= 0.5 and < 1.0 is still rounded up to 1.0 (for 62% FP
  rate)
  * This also gives us some support for configuring filters from OPTIONS
  file as currently saved: `filter_policy=rocksdb.BuiltinBloomFilter`.
  Opening from such an options file will enable reading filters (an
  improvement) but not writing new ones. (See Customizable follow-up
  below.)
* Also removed deprecated functions
  * FilterBitsBuilder::CalculateNumEntry()
  * FilterPolicy::GetFilterBitsBuilder()
  * NewExperimentalRibbonFilterPolicy()
* Remove default implementations of
  * FilterBitsBuilder::EstimateEntriesAdded()
  * FilterBitsBuilder::ApproximateNumEntries()
  * FilterPolicy::GetBuilderWithContext()
* Remove support for "filter_policy=experimental_ribbon" configuration
string.
* Allow "filter_policy=bloomfilter:n" without bool to discourage use of
block-based filter.

Some pieces for #9389

Likely follow-up (later PRs):
* Refactoring toward FilterPolicy Customizable, so that we can generate
filters with same configuration as before when configuring from options
file.
* Remove support for user enabling block-based filter (ignore `bool
use_block_based_builder`)
  * Some months after this change, we could even remove read support for
  block-based filter, because it is not critical to DB data
  preservation.
* Make FilterBitsBuilder::FinishV2 to avoid `using
FilterBitsBuilder::Finish` mess and add support for specifying a
MemoryAllocator (for cache warming)

Pull Request resolved: #9501

Test Plan:
A number of obsolete tests deleted and new tests or test
cases added or updated.

Reviewed By: hx235

Differential Revision: D34008011

Pulled By: pdillinger

fbshipit-source-id: a39a720457c354e00d5b59166b686f7f59e392aa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
announcement rocksdb-7.0 PRs with breaking API changes that need to land in the next major release, 7.0.
Projects
None yet
Development

No branches or pull requests

2 participants