From 875bfd75a09b4a456333702d538ce019c0a63716 Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Wed, 19 Jan 2022 10:10:34 -0800 Subject: [PATCH] Add API warning for `Iterator::Refresh()` with range tombstones (#9398) Summary: Need this until we properly return an error or fix the combination. Reported in https://github.com/facebook/rocksdb/issues/9255. Pull Request resolved: https://github.com/facebook/rocksdb/pull/9398 Reviewed By: riversand963 Differential Revision: D33641396 Pulled By: ajkr fbshipit-source-id: 9fe804108f7b93912f5b9c7252ac49acedc4f805 --- HISTORY.md | 1 + include/rocksdb/db.h | 4 ++++ include/rocksdb/iterator.h | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index cfc40d0bc..0cea930e5 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -6,6 +6,7 @@ * Made the Env class extend the Customizable class. Implementations need to be registered with the ObjectRegistry and to implement a Name() method in order to be created via this method. * `Options::OldDefaults` is marked deprecated, as it is no longer maintained. * Add ObjectLibrary::AddFactory and ObjectLibrary::PatternEntry classes. This method and associated class are the preferred mechanism for registering factories with the ObjectLibrary going forward. The ObjectLibrary::Register method, which uses regular expressions and may be problematic, is deprecated and will be in a future release. +* Added API warning against using `Iterator::Refresh()` together with `DB::DeleteRange()`, which are incompatible and have always risked causing the refreshed iterator to return incorrect results. ### Behavior Changes * `DB::DestroyColumnFamilyHandle()` will return Status::InvalidArgument() if called with `DB::DefaultColumnFamily()`. diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h index 750380f46..c6cb973d9 100644 --- a/include/rocksdb/db.h +++ b/include/rocksdb/db.h @@ -403,6 +403,10 @@ class DB { // If "end_key" comes before "start_key" according to the user's comparator, // a `Status::InvalidArgument` is returned. // + // WARNING: Do not use `Iterator::Refresh()` API on DBs where `DeleteRange()` + // has been used or will be used. This feature combination is neither + // supported nor programmatically prevented. + // // This feature is now usable in production, with the following caveats: // 1) Accumulating many range tombstones in the memtable will degrade read // performance; this can be avoided by manually flushing occasionally. diff --git a/include/rocksdb/iterator.h b/include/rocksdb/iterator.h index eb3f42acd..d0a72c322 100644 --- a/include/rocksdb/iterator.h +++ b/include/rocksdb/iterator.h @@ -92,6 +92,10 @@ class Iterator : public Cleanable { // If supported, renew the iterator to represent the latest state. The // iterator will be invalidated after the call. Not supported if // ReadOptions.snapshot is given when creating the iterator. + // + // WARNING: Do not use `Iterator::Refresh()` API on DBs where `DeleteRange()` + // has been used or will be used. This feature combination is neither + // supported nor programmatically prevented. virtual Status Refresh() { return Status::NotSupported("Refresh() is not supported"); }