Eviction scan fix

src/bucket/Bucket.cpp

@@ -838,20 +838,22 @@ mergeCasesWithEqualKeys(MergeCounters& mc, BucketInputIterator& oi,
 
 bool
 Bucket::scanForEviction(AbstractLedgerTxn& ltx, EvictionIterator& iter,
-                        uint64_t& bytesToScan, uint32_t& maxEntriesToEvict,
-                        uint32_t ledgerSeq,
+                        uint64_t& bytesToScan,
+                        uint32_t& remainingEntriesToEvict, uint32_t ledgerSeq,
                         medida::Counter& entriesEvictedCounter,
                         medida::Counter& bytesScannedForEvictionCounter,
                         std::optional<EvictionMetrics>& metrics)
 {
     ZoneScoped;
-    if (isEmpty())
+    if (isEmpty() ||
+        protocolVersionIsBefore(getBucketVersion(shared_from_this()),
+                                SOROBAN_PROTOCOL_VERSION))
     {
-        // EOF
+        // EOF, skip to next bucket
         return false;
     }
 
-    if (maxEntriesToEvict == 0 || bytesToScan == 0)
+    if (remainingEntriesToEvict == 0 || bytesToScan == 0)
     {
         // Reached end of scan region
         return true;
@@ -908,7 +910,7 @@ Bucket::scanForEviction(AbstractLedgerTxn& ltx, EvictionIterator& iter,
                     ltx.erase(ttlKey);
                     ltx.erase(LedgerEntryKey(le));
                     entriesEvictedCounter.inc();
-                    --maxEntriesToEvict;
+                    --remainingEntriesToEvict;
                 }
 
                 stream.seek(initialStreamPos);
@@ -925,6 +927,10 @@ Bucket::scanForEviction(AbstractLedgerTxn& ltx, EvictionIterator& iter,
             bytesToScan = 0;
             return true;
         }
+        else if (remainingEntriesToEvict == 0)
+        {
+            return true;
+        }
 
         bytesToScan -= bytesRead;
     }

src/bucket/Bucket.h

@@ -137,14 +137,15 @@ class Bucket : public std::enable_shared_from_this<Bucket>,
     static std::string randomBucketName(std::string const& tmpDir);
     static std::string randomBucketIndexName(std::string const& tmpDir);
 
-    // Returns false if eof reached, true otherwise. Modifies iter as the bucket
-    // is scanned. Also modifies bytesToScan and maxEntriesToEvict such that
-    // after this function returns:
+    // Returns false if eof reached or if Bucket protocol version < 20, true
+    // otherwise. Modifies iter as the bucket is scanned. Also modifies
+    // bytesToScan and remainingEntriesToEvict such that after this function
+    // returns:
     // bytesToScan -= amount_bytes_scanned
-    // maxEntriesToEvict -= entries_evicted
+    // remainingEntriesToEvict -= entries_evicted
     bool scanForEviction(AbstractLedgerTxn& ltx, EvictionIterator& iter,
-                         uint64_t& bytesToScan, uint32_t& maxEntriesToEvict,
-                         uint32_t ledgerSeq,
+                         uint64_t& bytesToScan,
+                         uint32_t& remainingEntriesToEvict, uint32_t ledgerSeq,
                          medida::Counter& entriesEvictedCounter,
                          medida::Counter& bytesScannedForEvictionCounter,
                          std::optional<EvictionMetrics>& metrics);

src/bucket/test/BucketListTests.cpp

@@ -936,7 +936,7 @@ TEST_CASE_VERSIONS("eviction scan", "[bucketlist]")
                 // Check that we only evict at most maxEntriesToArchive per
                 // ledger
                 auto newCount = entriesEvictedCounter.count();
-                REQUIRE(newCount + 1 >= prevCount);
+                REQUIRE((newCount == prevCount || newCount == prevCount + 1));
                 prevCount = newCount;
             }