Fix three broken/flaky Tsavorite and Garnet tests (#1711)

1. LargeObjectMultiFlushedPages (Tsavorite ObjectTests):
   Remove [Explicit] and add a no-op ShiftReadOnlyAddress(currentROA,
   wait: true) sync point so the FlushedUntilAddress / ReadOnlyAddress
   assertions see a quiesced state after the async flush pipeline
   catches up. MonotonicUpdate is a no-op when passed the current ROA;
   the built-in wait loop in ShiftReadOnlyAddressWithWait blocks until
   FUA >= ROA, which is exactly the observation the test wants.

2. LargeObjectLinearizeFlushedPages (Tsavorite ObjectTests):
   Drop the Is.True assertion on the second ShiftReadOnlyAddress in
   DoFlush. That call intentionally races with the main thread's
   ShiftReadOnlyToTail; if the latter wins, MonotonicUpdate correctly
   refuses to move ROA backwards and returns false. The post-
   Task.WhenAll invariants (ROA == FUA == TailAddress) still fully
   verify correctness, and the test continues to exercise the
   overlapping-shift linearization path.

3. ConfigSetHeapMemorySizeUtilizationTest (Garnet RespConfigTests):
   Rewrite test body. The old test inserted 4 tiny lists (i++ inside
   the loop meant only 4 unique keys) against a 3MB budget, so the
   LogSizeTracker never saw over-budget and PostMemoryTrim was never
   invoked; trimCompleteEvent.Wait timed out at 90s. The post-wait
   assertion also expected AllocatedPageCount < MinLogAllocatedPageCount
   (== 2), which is logically impossible. The fix inserts 128 list
   objects with meaningful heap (16 * 256-byte items each, ~512 KB
   total), then issues CONFIG SET memory 8192 to trigger the shrink
   branch of UpdateTargetSize. TotalSize > highTargetSize causes the
   resizer to compute an eviction range, shift addresses with
   waitForEviction, and invoke PostMemoryTrim. The test now waits at
   most ~30s for the callback and asserts that APC or heap actually
   decreased. Remove [Explicit].

No product code changes. Validated with many consecutive green runs
of each affected test, plus a full pass of the LargeObject and
RespConfig* suites.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: badrishc

libs/storage/Tsavorite/cs/test/ObjectTests.cs

@@ -273,7 +273,6 @@ void DoRead(bool onDisk)
 
         [Test, Category(TsavoriteKVTestCategory), Category(LogRecordCategory), Category(SmokeTestCategory), Category(ObjectIdMapCategory)]
         //[Repeat(300)]
-        [Explicit("Temporary while I figure out why FlushedUntilAddress is not as expected")]
         public void LargeObjectMultiFlushedPages([Values(SerializeKeyValueSize.Thirty, SerializeKeyValueSize.OneK)] SerializeKeyValueSize serializeValueSize)
         {
             // Ensure our size calculations are correct by validating the test parameters are what we expect
@@ -304,12 +303,20 @@ public void LargeObjectMultiFlushedPages([Values(SerializeKeyValueSize.Thirty, S
                 _ = bContext.Upsert(key, ref input, value, ref output);
             }
 
+            // The upsert loop implicitly advanced ReadOnlyAddress as pages were turned, which kicks off asynchronous page flushes.
+            // ROA is already at its final value, but the flush completions that drive FlushedUntilAddress arrive on background
+            // I/O threads and may not all have landed by the time we reach here. Synchronize by re-invoking ShiftReadOnlyAddress
+            // with wait:true; the shift itself is a no-op (MonotonicUpdate fails for the same address) but the wait loop blocks
+            // until FlushedUntilAddress catches up to the current ReadOnlyAddress.
+            store.Log.ShiftReadOnlyAddress(store.hlogBase.ReadOnlyAddress, wait: true);
+
             // Test that the expected number of pages were flushed and that we get the right results back. We've flushed in a multiple of full pages,
-            // so ReadOnlyAddress will be aligned to page size. We have 0.1 mutable fraction, so we should have closed 2 pages and have 3 mutable,
-            // leaving 27 immutable, totaling FUA 29 * PageSize. (That's why we verified 32 above.. easier to verify numbers here).
+            // so ReadOnlyAddress will be aligned to page size. With 0.1 mutable fraction, tail at 34 * PageSize, and head shifted to page 2 during
+            // the last two page-turns (each of which moves head by one page once MaxAllocatedPageCount is reached), CalculateReadOnlyAddress snaps
+            // ROA to page 29. That is: 2 closed pages (0, 1), 27 immutable (2..28) and 5 mutable (29..33), with FUA == ROA == 29 * PageSize.
             // Note: This test does NOT use SizeTracker; therefore it does not evict except when page count is exceeded.
             Assert.That(IsAligned(store.hlogBase.ReadOnlyAddress, store.hlogBase.PageSize), $"ReadOnlyAddress ({store.hlogBase.ReadOnlyAddress}) should be page-aligned");
-            Assert.That(store.hlogBase.FlushedUntilAddress, Is.EqualTo(store.hlogBase.PageSize * 29), $"FUA ({store.hlogBase.FlushedUntilAddress}) != PageSize * 29");  // <============== Fix this AF
+            Assert.That(store.hlogBase.FlushedUntilAddress, Is.EqualTo(store.hlogBase.PageSize * 29), $"FUA ({store.hlogBase.FlushedUntilAddress}) != PageSize * 29");
             Assert.That(store.hlogBase.FlushedUntilAddress, Is.EqualTo(store.hlogBase.ReadOnlyAddress), $"FUA ({store.hlogBase.FlushedUntilAddress}) == ROA");
             DoRead(0, 2 * ObjectsPerPage - 1, onDisk: true);
             DoRead(2 * ObjectsPerPage, lastKey, onDisk: false);
@@ -463,9 +470,16 @@ void DoFlush(long newReadOnlyAddress, ManualResetEventSlim gate)
                 try
                 {
                     // Do this in two pieces so we signal the gate in between to give the second call above a chance to launch.
+                    // The first shift must succeed because no concurrent shift has happened yet.
                     Assert.That(store.hlogBase.ShiftReadOnlyAddress(newReadOnlyAddress - store.hlogBase.PageSize * 10), Is.True);
                     gate?.Set();
-                    Assert.That(store.hlogBase.ShiftReadOnlyAddress(newReadOnlyAddress), Is.True);
+
+                    // The second shift races with the main thread's ShiftReadOnlyToTail. Either ordering is valid and exercises
+                    // the linearization path: if this shift lands first, ShiftReadOnlyToTail extends ROA from newReadOnlyAddress
+                    // to the tail; if ShiftReadOnlyToTail lands first, ROA is already past newReadOnlyAddress and MonotonicUpdate
+                    // (correctly) refuses to move ROA backwards, so this call returns false. Both outcomes converge to the same
+                    // post-condition (ROA == FUA == TailAddress), which is what the caller asserts after awaiting both tasks.
+                    _ = store.hlogBase.ShiftReadOnlyAddress(newReadOnlyAddress);
                 }
                 finally
                 {

test/Garnet.test/RespConfigTests.cs

@@ -682,59 +682,64 @@ public void TearDown()
         }
 
         /// <summary>
-        /// This test verifies that dynamically changing the object store heap size configuration using CONFIG SET
-        /// incurs a reduction in the used memory of the store.
+        /// This test verifies that dynamically shrinking the main-log memory target via CONFIG SET memory
+        /// drives the log size tracker to evict pages / release heap so that total memory usage is reduced.
         /// </summary>
-        /// <param name="largerSize">Heap size larger than configured size</param>
+        /// <param name="smallerSize">Memory size smaller than the current total log+heap usage</param>
         [Test]
         [TestCase("8192")]
-        [Explicit("Currently hangs due to waiting for eviction callback.")]
-        public void ConfigSetHeapMemorySizeUtilizationTest(string largerSize)
+        public void ConfigSetHeapMemorySizeUtilizationTest(string smallerSize)
         {
             using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig(allowAdmin: true));
             var db = redis.GetDatabase(0);
             var option = "memory";
 
-            // Verify that initial allocated page count is the minimum (before we've added anything)
             var store = server.Provider.StoreWrapper.store;
             var tracker = store.Log.LogSizeTracker;
 
+            // Verify initial state: no data yet, allocated page count at the minimum.
+            ClassicAssert.AreEqual(RespConfigTests.MinLogAllocatedPageCount, store.Log.AllocatedPageCount);
+
             using var trimCompleteEvent = new ManualResetEventSlim(false);
             tracker.PostMemoryTrim = (allocatedPageCount, headAddress) => { trimCompleteEvent.Set(); };
 
-            var initialApc = RespConfigTests.MinLogAllocatedPageCount;
-            ClassicAssert.AreEqual(initialApc, store.Log.AllocatedPageCount);
-
-            // TODO make this larger. Assert that HA has advanced before reaching MaxAllocatedPageCount
-
-
-            // Add objects to store to fill up heap
+            // Insert enough list objects so that (a) the inline log grows beyond MinResizeTargetPageCount pages
+            // (required before DetermineEvictionRange can evict anything), and (b) the tracked heap size is
+            // well above the new target we're about to configure. Each list contributes ~4KB of heap
+            // (16 items * 256 bytes), and each record adds a small inline entry to the log.
+            const int numKeys = 128;
             var values = new RedisValue[16];
+            var valPayload = new string('x', 256);
             for (var i = 0; i < values.Length; i++)
-                values[i] = "x";
-
-            for (var i = 0; i < 8; i++)
-                _ = db.ListRightPush($"key{i++:00000}", values);
-
-            // Wait for the logSizeTracker to stabilize.
-            Assert.That(trimCompleteEvent.Wait(TimeSpan.FromSeconds(3 * 3 * LogSizeTracker.ResizeTaskDelaySeconds)),
-                "Timeout occurred. Resizing did not happen within the specified time.");
-
-            // Verify that allocated page count has decreased
-            ClassicAssert.Less(store.Log.AllocatedPageCount, initialApc);
-            var prevApc = store.Log.AllocatedPageCount;
-
-            // TODO verify that the HeadAddress has moved
-
-            //////////////////////////////////////////////////////
-            // Try to set heap size to a larger value than current
-            var result = db.Execute("CONFIG", "SET", option, largerSize);
+                values[i] = valPayload;
+            for (var i = 0; i < numKeys; i++)
+                _ = db.ListRightPush($"key{i:00000}", values);
+
+            // Sanity-check the preconditions for the shrink/eviction we are about to trigger.
+            var apcBefore = store.Log.AllocatedPageCount;
+            var heapBefore = tracker.LogHeapSizeBytes;
+            Assert.That(apcBefore, Is.GreaterThan(LogSizeTracker.MinResizeTargetPageCount),
+                "Test precondition: need more than MinResizeTargetPageCount pages for eviction to be possible.");
+            Assert.That(heapBefore, Is.GreaterThan(0), "Test precondition: heap should be non-empty after inserts.");
+
+            // Shrink the memory target. The 'shrink' branch of LogSizeTracker.UpdateTargetSize signals the
+            // resizer task; because TotalSize is now well above highTargetSize, the task calls
+            // DetermineEvictionRange + ShiftAddresses and then invokes PostMemoryTrim.
+            var result = db.Execute("CONFIG", "SET", option, smallerSize);
             ClassicAssert.AreEqual("OK", result.ToString());
-
-            // There is no need to wait for the logSizeTracker to stabilize, as it will simply increase the page count.
-
-            // Verify that MaxAllocatedPageCount has increased.
-            ClassicAssert.Less(store.Log.AllocatedPageCount, prevApc);
+            var smallerTarget = ServerOptions.ParseSize(smallerSize, out _);
+            Assert.That(tracker.TargetSize, Is.EqualTo(smallerTarget));
+
+            // Wait for the trim callback.
+            Assert.That(trimCompleteEvent.Wait(TimeSpan.FromSeconds(3 * LogSizeTracker.ResizeTaskDelaySeconds)),
+                "Timeout occurred. PostMemoryTrim was not invoked after CONFIG SET memory shrink.");
+
+            // Verify that memory usage actually dropped: we expect at least one of the allocated page count
+            // or the tracked heap size to have decreased as a result of the eviction scan.
+            var apcAfter = store.Log.AllocatedPageCount;
+            var heapAfter = tracker.LogHeapSizeBytes;
+            Assert.That(apcAfter < apcBefore || heapAfter < heapBefore, Is.True,
+                $"Expected APC ({apcBefore}->{apcAfter}) or heap ({heapBefore}->{heapAfter}) to decrease after trim.");
         }
     }
 }