RavenDB-23094 When freeing scratch pages allocated in current write t…

…ransaction then we should mark them as immediately available for allocation
arekpalinski · Nov 15, 2024 · 94f9a11 · 94f9a11
1 parent 4f1235f
commit 94f9a11
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 6 deletions.
diff --git a/src/Voron/Impl/LowLevelTransaction.cs b/src/Voron/Impl/LowLevelTransaction.cs
@@ -846,7 +846,7 @@ internal void DiscardScratchModificationOn(long pageNumber)
             {
                 _transactionPages.Remove(scratchPage);
 
-                _env.ScratchBufferPool.Free(this, scratchPage.File.Number, scratchPage.PositionInScratchBuffer);
+                _env.ScratchBufferPool.FreeImmediately(this, scratchPage.File.Number, scratchPage.PositionInScratchBuffer);
 
                 if (_env.Options.Encryption.IsEnabled)
                 {
@@ -1215,7 +1215,7 @@ public void Rollback()
                 if(maybeRollBack.AllocatedInTransaction != Id)
                     continue; // from a committed transaction, can keep
 
-                _env.ScratchBufferPool.Free(this, maybeRollBack.File.Number, maybeRollBack.PositionInScratchBuffer);
+                _env.ScratchBufferPool.FreeImmediately(this, maybeRollBack.File.Number, maybeRollBack.PositionInScratchBuffer);
             }
 
             RolledBack = true;

diff --git a/src/Voron/Impl/Scratch/ScratchBufferPool.cs b/src/Voron/Impl/Scratch/ScratchBufferPool.cs
@@ -218,6 +218,15 @@ public void Free(LowLevelTransaction tx, int scratchNumber, long page)
             }
         }
 
+        public void FreeImmediately(LowLevelTransaction tx, int scratchNumber, long page)
+        {
+            var scratch = _scratchBuffers[scratchNumber];
+            if (scratch.File.Free(tx, asOfTxId: -1, page))
+            {
+                MaybeRecycleFile(tx, scratch);
+            }
+        }
+
         private void MaybeRecycleFile(LowLevelTransaction tx, ScratchBufferItem scratch)
         {
             List<ScratchBufferFile> recycledScratchesToDispose = null;

diff --git a/test/SlowTests/Voron/Issues/RavenDB_16023.cs b/test/SlowTests/Voron/Issues/RavenDB_16023.cs
@@ -217,8 +217,9 @@ public unsafe void FreeingPageShouldAlsoMarkItsRelatedEncryptionBufferAsNotValid
 
                 var state = tx.LowLevelTransaction.PagerTransactionState.ForCrypto![scratchFile.Pager];
 
-                Assert.True(state[66].SkipOnTxCommit); // page 66 is PositionInScratchBuffer of the age #21 that was freed at the beginning of this transaction
-                Assert.True(state[83].SkipOnTxCommit); // page 83 is PositionInScratchBuffer of the age #20 that was freed in this transaction
+                // page 66 is PositionInScratchBuffer of the page #21 that was freed at the beginning of this transaction,
+                // and it was reused by page #20 that was freed in this transaction later on
+                Assert.True(state[66].SkipOnTxCommit);
 
                 tx.Commit();
             }
@@ -278,8 +279,9 @@ public unsafe void FreeingPageShouldAlsoMarkItsRelatedEncryptionBufferAsNotValid
 
                 var state = tx.LowLevelTransaction.PagerTransactionState.ForCrypto![scratchFile.Pager];
 
-                Assert.True(state[66].SkipOnTxCommit); // page 66 is PositionInScratchBuffer of the age #21 that was freed at the beginning of this transaction
-                Assert.True(state[83].SkipOnTxCommit); // page 83 is PositionInScratchBuffer of the age #20 that was freed in this transaction
+                // page 66 is PositionInScratchBuffer of the page #21 that was freed at the beginning of this transaction,
+                // and it was reused by page #20 that was freed in this transaction later on
+                Assert.True(state[66].SkipOnTxCommit);
 
                 tx.Commit();
             }