From f246e56d0a73b5839ff95b24b4761406f850a263 Mon Sep 17 00:00:00 2001
From: Andrew Kryczka <andrewkr@fb.com>
Date: Fri, 1 Apr 2022 10:30:17 -0700
Subject: [PATCH] Fix a few documentation errors including in public APIs
 (#9789)

Summary:
The internal WriteBatch doc wrongly indicated which optypes are followed by varstring. Updated some optypes according to the following code: https://github.com/facebook/rocksdb/blob/76383bea5df1136c95babf5f9f40b24f85e9ad8e/db/write_batch.cc#L418-L429

The `Iterator::Refresh()` + `DeleteRange()` bug was fixed in https://github.com/facebook/rocksdb/issues/9258; removed the warnings.

`GetMergeOperands()` does populate `*number_of_operands` including upon successful return: https://github.com/facebook/rocksdb/blob/76383bea5df1136c95babf5f9f40b24f85e9ad8e/db/db_impl/db_impl.cc#L1917-L1919

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9789

Reviewed By: riversand963

Differential Revision: D35303421

Pulled By: ajkr

fbshipit-source-id: 9b0e1be5f6b2e2b31461e6c33ecb5f5381824452
---
 db/write_batch.cc          |  9 +++++----
 include/rocksdb/db.h       | 26 +++++++++++++-------------
 include/rocksdb/iterator.h |  4 ----
 3 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/db/write_batch.cc b/db/write_batch.cc
index f3b800e73..8a590a7f2 100644
--- a/db/write_batch.cc
+++ b/db/write_batch.cc
@@ -22,12 +22,13 @@
 //    kTypeColumnFamilySingleDeletion varint32 varstring
 //    kTypeColumnFamilyRangeDeletion varint32 varstring varstring
 //    kTypeColumnFamilyMerge varint32 varstring varstring
-//    kTypeBeginPrepareXID varstring
-//    kTypeEndPrepareXID
+//    kTypeBeginPrepareXID
+//    kTypeEndPrepareXID varstring
 //    kTypeCommitXID varstring
+//    kTypeCommitXIDAndTimestamp varstring varstring
 //    kTypeRollbackXID varstring
-//    kTypeBeginPersistedPrepareXID varstring
-//    kTypeBeginUnprepareXID varstring
+//    kTypeBeginPersistedPrepareXID
+//    kTypeBeginUnprepareXID
 //    kTypeNoop
 // varstring :=
 //    len: varint32
diff --git a/include/rocksdb/db.h b/include/rocksdb/db.h
index 778d64f08..0f9901f27 100644
--- a/include/rocksdb/db.h
+++ b/include/rocksdb/db.h
@@ -438,10 +438,6 @@ 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.
@@ -541,16 +537,20 @@ class DB {
     return Get(options, DefaultColumnFamily(), key, value, timestamp);
   }
 
-  // Returns all the merge operands corresponding to the key. If the
-  // number of merge operands in DB is greater than
-  // merge_operands_options.expected_max_number_of_operands
-  // no merge operands are returned and status is Incomplete. Merge operands
-  // returned are in the order of insertion.
-  // merge_operands- Points to an array of at-least
+  // Populates the `merge_operands` array with all the merge operands in the DB
+  // for `key`. The `merge_operands` array will be populated in the order of
+  // insertion. The number of entries populated in `merge_operands` will be
+  // assigned to `*number_of_operands`.
+  //
+  // If the number of merge operands in DB for `key` is greater than
+  // `merge_operands_options.expected_max_number_of_operands`,
+  // `merge_operands` is not populated and the return value is
+  // `Status::Incomplete`. In that case, `*number_of_operands` will be assigned
+  // the number of merge operands found in the DB for `key`.
+  //
+  // `merge_operands`- Points to an array of at-least
   //             merge_operands_options.expected_max_number_of_operands and the
-  //             caller is responsible for allocating it. If the status
-  //             returned is Incomplete then number_of_operands will contain
-  //             the total number of merge operands found in DB for key.
+  //             caller is responsible for allocating it.
   virtual Status GetMergeOperands(
       const ReadOptions& options, ColumnFamilyHandle* column_family,
       const Slice& key, PinnableSlice* merge_operands,
diff --git a/include/rocksdb/iterator.h b/include/rocksdb/iterator.h
index d0a72c322..eb3f42acd 100644
--- a/include/rocksdb/iterator.h
+++ b/include/rocksdb/iterator.h
@@ -92,10 +92,6 @@ 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");
   }