[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1739: IGNITE-18882 Don't store tombstones if there is no other version of row

["tkalkirill (via GitHub)" <gi...@apache.org>]

tkalkirill commented on code in PR #1739:
URL: https://github.com/apache/ignite-3/pull/1739#discussion_r1123022879


##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/AddWriteCommittedInvokeClosure.java:
##########
@@ -81,6 +81,14 @@ public void call(@Nullable VersionChain oldRow) throws IgniteInternalCheckedExce
             throw new StorageException("Write intent exists: [rowId={}, {}]", oldRow.rowId(), storage.createStorageInfo());
         }
 
+        if (row == null && oldRow == null) {
+            // If previous version doesn't exist and current version is a tombstone,

Review Comment:
   I would paraphrase in:
   `If only one version, and it is a tombstone, then we will not save such a chain.`



##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/mv/CommitWriteInvokeClosure.java:
##########
@@ -80,6 +80,13 @@ public void call(@Nullable VersionChain oldRow) throws IgniteInternalCheckedExce
         RowVersion current = storage.readRowVersion(oldRow.headLink(), DONT_LOAD_VALUE);
         RowVersion next = oldRow.hasNextLink() ? storage.readRowVersion(oldRow.nextLink(), DONT_LOAD_VALUE) : null;
 
+        if (next == null && current.isTombstone()) {
+            // Previous version doesn't exist and current version is a tombstone, delete current version.

Review Comment:
   I would paraphrase in:
   `If only one version, and it is a tombstone, then we will remove such a chain.`



##########
modules/storage-api/src/testFixtures/java/org/apache/ignite/internal/storage/impl/TestMvPartitionStorage.java:
##########
@@ -271,6 +271,12 @@ private VersionChain resolveCommittedVersionChain(VersionChain committedVersionC
             // Calling it from the compute is fine. Concurrent writes of the same row are impossible, and if we call the compute closure
             // several times, the same tuple will be inserted into the GC queue (timestamp and rowId don't change in this case).
             gcQueue.add(committedVersionChain);
+        } else {
+            if (committedVersionChain.row == null) {
+                // If previous version doesn't exist and current version is a tombstone,

Review Comment:
   I would paraphrase in: 
   `if only one version, and it is a tombstone, then we will remove such a chain.`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org