Address reverse MultiScan follow-up comments (#14922)

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

- Preserve source compatibility for `Scan::Reset` by restoring the default `reverse` argument.
- Make `HasMoreScanRanges()` defensive for reverse prepared scans instead of asserting.
- Restore broader forward coverage in `MultiScanAsyncIOTest` while keeping reverse coverage on the representative async IO matrix.
- Let `--multiscan_reverse` drive reverse MultiScan stress coverage independently of `--test_backward_scan`.
- Add the reverse MultiScan public API release note.

Reviewed By: xingbowang

Differential Revision: D111127403

fbshipit-source-id: 1a17d0a36aaf66e8cbc51f445d0f85e145fd653c

db_stress_tool/db_stress_gflags.cc

@@ -1725,7 +1725,8 @@ DEFINE_bool(use_multiscan, false,
 
 DEFINE_bool(multiscan_reverse, false,
             "If set with use_multiscan, scan each MultiScan range in reverse "
-            "using SeekForPrev and Prev.");
+            "using SeekForPrev and Prev. This does not require "
+            "test_backward_scan.");
 
 DEFINE_bool(multiscan_use_async_io, false,
             "If set, enable async_io for MultiScan operations.");

db_stress_tool/db_stress_test_base.cc

@@ -2237,8 +2237,7 @@ Status StressTest::TestMultiScan(ThreadState* thread,
   std::vector<std::string> end_key_strs;
   // TODO support reverse BytewiseComparator in the stress test
   MultiScanArgs scan_opts(options_.comparator);
-  const bool reverse_multiscan =
-      FLAGS_multiscan_reverse && FLAGS_test_backward_scan;
+  const bool reverse_multiscan = FLAGS_multiscan_reverse;
   scan_opts.reverse = reverse_multiscan;
   scan_opts.use_async_io =
       FLAGS_multiscan_use_async_io &&

include/rocksdb/multi_scan.h

@@ -74,7 +74,7 @@ class Scan {
   explicit Scan(Iterator* db_iter, bool reverse = false)
       : db_iter_(db_iter), reverse_(reverse) {}
 
-  void Reset(Iterator* db_iter, bool reverse) {
+  void Reset(Iterator* db_iter, bool reverse = false) {
     db_iter_ = db_iter;
     reverse_ = reverse;
   }

table/block_based/block_based_table_reader_test.cc

@@ -1215,8 +1215,16 @@ TEST_P(BlockBasedTableReaderMultiScanAsyncIOTest, MultiScanPrepare) {
     }
     ASSERT_OK(iter->status());
   };
+  const bool run_reverse =
+      compression_type_ == kNoCompression && !use_direct_reads_ &&
+      param.index_type == BlockBasedTableOptions::IndexType::kBinarySearch &&
+      param.compression_parallel_threads == 1 &&
+      param.compression_dict_bytes == 0;
 
   for (const bool reverse : {false, true}) {
+    if (reverse && !run_reverse) {
+      continue;
+    }
     SCOPED_TRACE(reverse ? "reverse" : "forward");
     options_.statistics = CreateDBStatistics();
     std::string table_name = "BlockBasedTableReaderTest_NewIterator" +
@@ -2401,12 +2409,10 @@ INSTANTIATE_TEST_CASE_P(
     BlockBasedTableReaderMultiScanAsyncIOTest,
     ::testing::ValuesIn(
         BlockBasedTableReaderTestParamBuilder()
-            .WithCompressionTypes({kNoCompression})
-            .WithUseDirectReadFlags({false})
-            .WithIndexTypes({BlockBasedTableOptions::IndexType::kBinarySearch})
-            .WithParallelCompressionThreadCounts({1})
-            .WithCompressionDictByteCounts({0})
-            .WithComparators({BytewiseComparator()})
+            .WithIndexTypes(
+                {BlockBasedTableOptions::IndexType::kBinarySearch,
+                 BlockBasedTableOptions::IndexType::kHashSearch,
+                 BlockBasedTableOptions::IndexType::kTwoLevelIndexSearch})
             .WithFillCacheFlags(Bool())
             .WithUseAsyncIoFlags(IOUringFlags())
             .build()));

table/block_based/multi_scan_index_iterator.h

@@ -95,7 +95,11 @@ class MultiScanIndexIterator : public InternalIteratorBase<IndexValue> {
 
   // Returns true if there are more scan ranges after the current one.
   bool HasMoreScanRanges() const {
-    assert(!scan_opts_->reverse);
+    // Reverse MultiScan advances ranges through SeekForPrev(), not this
+    // forward-only range transition path.
+    if (scan_opts_->reverse) {
+      return false;
+    }
     return next_scan_idx_ < block_index_ranges_per_scan_.size();
   }
 

unreleased_history/public_api_changes/reverse_multiscan.md

@@ -0,0 +1 @@
+Added `MultiScanArgs::reverse` to support reverse MultiScan reads over bounded scan ranges.