RavenDB-22983 - rename, and do the clone with dispose and forget abou…

…t before returning from MoveNext
garayx · Nov 11, 2024 · b061301 · b061301
1 parent 9bacf53
commit b061301
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 68 deletions.
diff --git a/src/Raven.Server/Documents/DocumentsTransaction.cs b/src/Raven.Server/Documents/DocumentsTransaction.cs
@@ -163,11 +163,11 @@ public void ForgetAbout(Document doc)
             InnerTransaction.ForgetAbout(doc.StorageId);
         }
 
-        public bool CanForgetAbout(Document doc)
+        public bool IsCompressed(Document doc)
         {
             if (doc == null)
                 return false;
-            return InnerTransaction.CanForgetAbout(doc.StorageId);
+            return InnerTransaction.IsCompressed(doc.StorageId);
         }
 
         internal void CheckIfShouldDeleteAttachmentStream(Slice hash)

diff --git a/src/Raven.Server/Utils/Enumerators/TransactionForgetAboutAbstractEnumerator.cs b/src/Raven.Server/Utils/Enumerators/TransactionForgetAboutAbstractEnumerator.cs
@@ -1,6 +1,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using JetBrains.Annotations;
+using Raven.Server.Documents;
 using Raven.Server.ServerWide.Context;
 
 namespace Raven.Server.Utils.Enumerators;
@@ -16,27 +17,16 @@ protected TransactionForgetAboutAbstractEnumerator([NotNull] IEnumerator<T> inne
         DocsContext = docsContext;
     }
 
-    protected abstract void ForgetAbout(ForgetAboutItem item);
-    protected abstract ForgetAboutItem CloneCurrent(T item);
-
-    protected struct ForgetAboutItem
-    {
-        public T Item;
-        public T CompressedItem;
-        public bool IsCloned;
-    }
+    protected abstract T CloneCurrent(T item);
 
     public bool MoveNext()
     {
-        ForgetAbout(_current);
-
         if (_innerEnumerator.MoveNext() == false)
             return false;
 
-        // the clone is needed because we are going to forget about the _current document
-        // Current should be disposed by the caller
-        _current = CloneCurrent(_innerEnumerator.Current);
-        Current = _current.Item;
+        // the clone is needed because we are going to dispose and ForgetAbout the _innerEnumerator.Current document
+        // Current object should be disposed by the caller
+        Current = CloneCurrent(_innerEnumerator.Current);
 
         return true;
     }
@@ -46,7 +36,6 @@ public void Reset()
         throw new System.NotImplementedException();
     }
 
-    private ForgetAboutItem _current = new ForgetAboutItem();
     public T Current { get; private set; }
 
     object IEnumerator.Current => Current;
@@ -55,4 +44,17 @@ public void Dispose()
     {
         _innerEnumerator.Dispose();
     }
+
+    protected Document CloneCompressedDocumentAndForgetAbout(Document item)
+    {
+        if (DocsContext.Transaction.IsCompressed(item) == false)
+            return item;
+
+        using (item)
+        {
+            var cloned = item.Clone(DocsContext);
+            DocsContext.Transaction.ForgetAbout(item);
+            return cloned;
+        }
+    }
 }
diff --git a/...Raven.Server/Utils/Enumerators/TransactionForgetAboutCurrentPreviousRevisionEnumerator.cs b/...Raven.Server/Utils/Enumerators/TransactionForgetAboutCurrentPreviousRevisionEnumerator.cs
@@ -11,35 +11,23 @@ public TransactionForgetAboutCurrentPreviousRevisionEnumerator([NotNull] IEnumer
     {
     }
 
-    protected override void ForgetAbout(ForgetAboutItem item)
+    protected override (Document Previous, Document Current) CloneCurrent((Document Previous, Document Current) item)
     {
-        if (item.IsCloned)
+        if (DocsContext.Transaction.IsCompressed(item.Previous) == false && DocsContext.Transaction.IsCompressed(item.Current) == false)
+            return item;
+
+        if (DocsContext.Transaction.IsCompressed(item.Previous) == false)
         {
-            using (item.CompressedItem.Current)
-            using (item.CompressedItem.Previous)
-            {
-                DocsContext.Transaction.ForgetAbout(item.CompressedItem.Current);
-                DocsContext.Transaction.ForgetAbout(item.CompressedItem.Previous);
-            }
+            var cloned = (item.Previous, CloneCompressedDocumentAndForgetAbout(item.Current));
+            return cloned;
         }
-    }
 
-    protected override ForgetAboutItem CloneCurrent((Document Previous, Document Current) item)
-    {
-        if (DocsContext.Transaction.CanForgetAbout(item.Previous) && DocsContext.Transaction.CanForgetAbout(item.Current))
+        if (DocsContext.Transaction.IsCompressed(item.Current) == false)
         {
-            // this is revision so both items should be compressed
-            return new ForgetAboutItem()
-            {
-                Item = (item.Previous.Clone(DocsContext), item.Current.Clone(DocsContext)),
-                CompressedItem = item,
-                IsCloned = true
-            };
+            var cloned = (CloneCompressedDocumentAndForgetAbout(item.Previous), item.Current);
+            return cloned;
         }
 
-        return new ForgetAboutItem()
-        {
-            Item = item
-        };
+        return (CloneCompressedDocumentAndForgetAbout(item.Previous), CloneCompressedDocumentAndForgetAbout(item.Current));
     }
 }
diff --git a/src/Raven.Server/Utils/Enumerators/TransactionForgetAboutDocumentEnumerator.cs b/src/Raven.Server/Utils/Enumerators/TransactionForgetAboutDocumentEnumerator.cs
@@ -11,32 +11,8 @@ public TransactionForgetAboutDocumentEnumerator([NotNull] IEnumerator<Document>
     {
     }
 
-    protected override void ForgetAbout(ForgetAboutItem item)
+    protected override Document CloneCurrent(Document item)
     {
-        if (item.IsCloned)
-        {
-            using (item.CompressedItem)
-            {
-                DocsContext.Transaction.ForgetAbout(item.CompressedItem);
-            }
-        }
-    }
-
-    protected override ForgetAboutItem CloneCurrent(Document item)
-    {
-        if (DocsContext.Transaction.CanForgetAbout(item))
-        {
-            return new ForgetAboutItem()
-            {
-                Item = item.Clone(DocsContext),
-                CompressedItem = item,
-                IsCloned = true
-            };
-        }
-
-        return new ForgetAboutItem()
-        {
-            Item = item
-        };
+        return CloneCompressedDocumentAndForgetAbout(item);
     }
 }
diff --git a/src/Voron/Impl/Transaction.cs b/src/Voron/Impl/Transaction.cs
@@ -719,7 +719,7 @@ public void ForgetAbout(in long storageId)
             }
         }
 
-        public bool CanForgetAbout(in long storageId)
+        public bool IsCompressed(in long storageId)
         {
             if (_cachedDecompressedBuffersByStorageId == null)
                 return false;