Fix incorrect input file expansion in SetupOtherFilesWithRoundRobinExpansion (#14436)

Summary:
### Context/Summary:
**_See below for an example of the bug:_**

L1 has 5 SST files at indices [0, 1, 2, 3, 4] where files at indices 1/2/3
share user key boundaries (e.g., file[1].largest and file[2].smallest have
the same user key).

Round-robin picks file[1] at start_index=1.

Before fix:
  1. ExpandInputsToCleanCut expands {file[1]} → {file[1], file[2], file[3]}
  2. Loop starts at i=2 (start_index+1), adds file[2] → duplicate!
  3. start_level_inputs_ = {file[1], file[2], file[3], file[2]}
  4. GetRange uses back()=file[2], returns [file[1].smallest, file[2].largest]
  5. ExpandInputsToCleanCut converges on {file[1], file[2]}, missing file[3]
  6. AssertCleanCut crashes (debug) or data corruption (release)

After fix:
  1. ExpandInputsToCleanCut expands {file[1]} → {file[1], file[2], file[3]}
  2. Find last file = file[3] at index 3, loop starts at i=4
  3. start_level_inputs_ = {file[1], file[2], file[3], file[4]} (no duplicates)
  4. GetRange correctly returns [file[1].smallest, file[4].largest]

_**More details:**_

When round-robin compaction picks a file, PickFileToCompact calls ExpandInputsToCleanCut which may expand start_level_inputs_ from 1 file to N files (when adjacent files share user key boundaries). Then SetupOtherFilesWithRoundRobinExpansion loops from start_index + 1 to add more files, not knowing the expansion already happened. This re-adds files already in start_level_inputs_, creating duplicates.

The duplicates corrupt GetRange, which trusts inputs.back()->largest for non-L0 levels. With a duplicate earlier file at back(), GetRange returns a truncated range. ExpandInputsToCleanCut then converges on an incomplete file set, violating the clean-cut invariant.

In debug builds this crashes at AssertCleanCut. In release builds the compaction proceeds with a non-clean-cut input, causing data corruption (newer data in a later level while older data remains in an earlier level).

The fix finds the position of the last file already in start_level_inputs_ and starts the loop after it, avoiding duplicates. When start_level_inputs_ has only 1 file (no expansion happened), the behavior is unchanged.

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

Test Plan:
New test RoundRobinCleanCutWithSharedBoundary:
- Without fix: crashes at AssertCleanCut in SetupOtherFilesWithRoundRobinExpansion
```
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from DBCompactionTest
[ RUN      ] DBCompactionTest.RoundRobinCleanCutWithSharedBoundary
db_compaction_test: db/compaction/compaction_picker.cc:81: void rocksdb::AssertCleanCut(const rocksdb::InternalKeyComparator*, rocksdb::VersionStorageInfo*, rocksdb::CompactionInputFiles*, int, rocksdb::Logger*): Assertion `false' failed.
Received signal 6 (Aborted)
Invoking GDB for stack trace...
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/local/fbcode/platform010/lib/libthread_db.so.1".
0x00007fd9ba8f0e13 in __GI___wait4 (pid=1004850, stat_loc=0x7fd9b8bfb2ac, options=0, usage=0x0) at ../sysdeps/unix/sysv/linux/wait4.c:30
30      ../sysdeps/unix/sysv/linux/wait4.c: No such file or directory.
https://github.com/facebook/rocksdb/issues/4  __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:45
45      pthread_kill.c: No such file or directory.
https://github.com/facebook/rocksdb/issues/5  __GI___pthread_kill (threadid=<optimized out>, signo=6) at pthread_kill.c:62
62      in pthread_kill.c
https://github.com/facebook/rocksdb/issues/6  0x00007fd9ba8444ad in __GI_raise (sig=6) at ../sysdeps/posix/raise.c:26
26      ../sysdeps/posix/raise.c: No such file or directory.
https://github.com/facebook/rocksdb/issues/7  0x00007fd9ba82c433 in __GI_abort () at abort.c:79
79      abort.c: No such file or directory.
https://github.com/facebook/rocksdb/issues/8  0x00007fd9ba83bc28 in __assert_fail_base (fmt=0x7fd9ba9e11d8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x7fd9bcc04819 "false", file=0x7fd9bcc04700 "db/compaction/compaction_picker.cc", line=81, function=<optimized out>) at assert.c:92
92      assert.c: No such file or directory.
https://github.com/facebook/rocksdb/issues/9  0x00007fd9ba83bc93 in __GI___assert_fail (assertion=0x7fd9bcc04819 "false", file=0x7fd9bcc04700 "db/compaction/compaction_picker.cc", line=81, function=0x7fd9bcc04780 "void rocksdb::AssertCleanCut(const rocksdb::InternalKeyComparator*, rocksdb::VersionStorageInfo*, rocksdb::CompactionInputFiles*, int, rocksdb::Logger*)") at assert.c:101
101     in assert.c
https://github.com/facebook/rocksdb/issues/10 0x00007fd9bc2f8181 in rocksdb::AssertCleanCut (icmp=0x7fd9b9a7a840, vstorage=0x7fd9b9b6d040, inputs=0x7fd9b8bfc690, level=1, logger=0x7fd9b9aa2790) at db/compaction/compaction_picker.cc:81
81            assert(false);
https://github.com/facebook/rocksdb/issues/11 0x00007fd9bc2f8fa6 in rocksdb::CompactionPicker::ExpandInputsToCleanCut (this=0x7fd9b9b32900, vstorage=0x7fd9b9b6d040, inputs=0x7fd9b8bfc690, next_smallest=0x0) at db/compaction/compaction_picker.cc:310
310       AssertCleanCut(icmp_, vstorage, inputs, level, ioptions_.logger);
https://github.com/facebook/rocksdb/issues/12 0x00007fd9bc30cbfb in rocksdb::(anonymous namespace)::LevelCompactionBuilder::SetupOtherFilesWithRoundRobinExpansion (this=0x7fd9b8bfc910) at db/compaction/compaction_picker_level.cc:423
```
- With fix: compaction completes, data correctness verified

Reviewed By: joshkang97

Differential Revision: D95718659

Pulled By: hx235

fbshipit-source-id: 6ef125455ef5ae8a07c323835ff25588dbbb3634

db/compaction/compaction_picker_level.cc

@@ -404,7 +404,15 @@ void LevelCompactionBuilder::SetupOtherFilesWithRoundRobinExpansion() {
   tmp_start_level_inputs = start_level_inputs_;
   // TODO (zichen): Future parallel round-robin may also need to update this
   // Constraint 1b (only expand till the end)
-  for (size_t i = start_index + 1; i < level_files.size(); i++) {
+  size_t next_index = start_index + 1;
+  const FileMetaData* last_file = start_level_inputs_.files.back();
+  for (size_t j = next_index; j < level_files.size(); j++) {
+    if (level_files[j] == last_file) {
+      next_index = j + 1;
+      break;
+    }
+  }
+  for (size_t i = next_index; i < level_files.size(); i++) {
     auto* f = level_files[i];
     if (f->being_compacted) {
       // Constraint 1a

db/db_compaction_test.cc

@@ -11671,6 +11671,70 @@ TEST_F(DBCompactionTest, PeriodicTask) {
   ASSERT_EQ(listener->num_periodic_compactions, 1);
   Close();
 }
+
+// Regression test for a bug in SetupOtherFilesWithRoundRobinExpansion where
+// duplicate files are added to the compaction input, corrupting
+// ExpandInputsToCleanCut and violating the clean-cut invariant. The bug
+// requires: (1) kRoundRobin compaction priority, (2) files at a non-L0 level
+// with shared user key boundaries (adjacent files whose boundary keys share
+// the same user key), and (3) ExpandInputsToCleanCut expanding the initially
+// picked file to include multiple adjacent files in PickFileToCompact.
+TEST_F(DBCompactionTest, RoundRobinCleanCutWithSharedBoundary) {
+  Options options = CurrentOptions();
+  options.compaction_style = kCompactionStyleLevel;
+  options.compaction_pri = kRoundRobin;
+  options.level_compaction_dynamic_level_bytes = false;
+  options.max_bytes_for_level_base = 100;
+  options.disable_auto_compactions = true;
+
+  DestroyAndReopen(options);
+
+  std::vector<const Snapshot*> snapshots;
+  for (int v = 0; v < 5; v++) {
+    for (int k = 0; k < 3; k++) {
+      ASSERT_OK(Put("key" + std::to_string(k), "v" + std::to_string(v)));
+    }
+    snapshots.push_back(db_->GetSnapshot());
+    ASSERT_OK(Flush());
+  }
+
+  // Force L0->L1 compaction output to split every 3 keys. With 3 keys x 5
+  // versions (15 KVs) sorted by (user_key asc, seq desc), splitting every 3
+  // creates 5 files where adjacent files share boundaries across different
+  // user keys (e.g., File0 ends with key0, File1 starts with key0 and ends
+  // with key1, etc.). This chain of 4+ shared boundaries across 3 different
+  // user keys is needed so that ExpandInputsToCleanCut expands the picked
+  // file to multiple files, and the duplicate in the round-robin loop causes
+  // GetRange to return a truncated range that drops files from the set.
+  std::atomic<int> key_count{0};
+  SyncPoint::GetInstance()->SetCallBack(
+      "CompactionOutputs::ShouldStopBefore::manual_decision", [&](void* arg) {
+        auto* p = static_cast<std::pair<bool*, const Slice>*>(arg);
+        int n = key_count.fetch_add(1);
+        if (n > 0 && n % 3 == 0) {
+          *(p->first) = true;
+        }
+      });
+  SyncPoint::GetInstance()->EnableProcessing();
+  ASSERT_OK(dbfull()->TEST_CompactRange(0, nullptr, nullptr));
+  SyncPoint::GetInstance()->DisableProcessing();
+  SyncPoint::GetInstance()->ClearAllCallBacks();
+
+  ColumnFamilyMetaData cf_meta;
+  db_->GetColumnFamilyMetaData(&cf_meta);
+  ASSERT_EQ(cf_meta.levels[1].files.size(), 5U);
+
+  for (auto s : snapshots) {
+    db_->ReleaseSnapshot(s);
+  }
+
+  ASSERT_OK(dbfull()->SetOptions({{"disable_auto_compactions", "false"}}));
+  ASSERT_OK(dbfull()->TEST_WaitForCompact());
+
+  for (int k = 0; k < 3; k++) {
+    ASSERT_EQ(Get("key" + std::to_string(k)), "v4");
+  }
+}
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {

unreleased_history/bug_fixes/fix_rr_duplicate_files.md

@@ -0,0 +1 @@
+Fix a bug in round-robin compaction that missed selecting input files that are needed to guarantee data correctness and cause crashing in debug builds or silent data corruption in release builds for Get().