From fa76828d0452f51d81e6666c72eb4435f4d9903c Mon Sep 17 00:00:00 2001
From: Stepan Usatiuk <stepan@usatiuk.com>
Date: Sat, 22 Feb 2025 21:28:47 +0100
Subject: [PATCH] more fixes

---
 .../java/com/usatiuk/dhfs/objects/SnapshotManager.java | 10 +++++++++-
 .../persistence/CachingObjectPersistentStore.java      |  2 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/SnapshotManager.java b/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/SnapshotManager.java
index ec24b4b2..0f503b78 100644
--- a/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/SnapshotManager.java
+++ b/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/SnapshotManager.java
@@ -254,7 +254,8 @@ public class SnapshotManager {
                         next = _backing.next();
                         nextNextKey = _backing.hasNext() ? _backing.peekNextKey() : null;
                     }
-                    if (next.getKey().version() <= _id && next.getValue().whenToRemove() > _id) {
+                    // next.getValue().whenToRemove() >=_id, read tx might have same snapshot id as some write tx
+                    if (next.getKey().version() <= _id && next.getValue().whenToRemove() >= _id) {
                         _next = switch (next.getValue()) {
                             case SnapshotEntryObject(JDataVersionedWrapper data, long whenToRemove) ->
                                     Pair.of(next.getKey().key(), new TombstoneMergingKvIterator.Data<>(data));
@@ -310,6 +311,8 @@ public class SnapshotManager {
                     long curVersion = _snapshotVersions.get(_id);
                     _backing = new TombstoneMergingKvIterator<>(new SnapshotKvIterator(start, key), delegateStore.getIterator(start, key));
                     _next = _backing.hasNext() ? _backing.next() : null;
+                    if (_next != null)
+                        assert _next.getValue().version() <= _id;
                     _lastRefreshed = curVersion;
                 }
             }
@@ -347,6 +350,7 @@ public class SnapshotManager {
                 doRefresh();
                 if (_backing.hasNext()) {
                     _next = _backing.next();
+                    assert _next.getValue().version() <= _id;
                 } else {
                     _next = null;
                 }
@@ -354,6 +358,9 @@ public class SnapshotManager {
 
             @Override
             public JObjectKey peekNextKey() {
+                if (_next == null) {
+                    throw new NoSuchElementException();
+                }
                 return _next.getKey();
             }
 
@@ -373,6 +380,7 @@ public class SnapshotManager {
                     throw new NoSuchElementException("No more elements");
                 }
                 var ret = _next;
+                assert ret.getValue().version() <= _id;
                 prepareNext();
                 Log.tracev("Read: {0}, next: {1}", ret, _next);
                 return ret;
diff --git a/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/persistence/CachingObjectPersistentStore.java b/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/persistence/CachingObjectPersistentStore.java
index 1a208be9..4126f743 100644
--- a/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/persistence/CachingObjectPersistentStore.java
+++ b/dhfs-parent/objects/src/main/java/com/usatiuk/dhfs/objects/persistence/CachingObjectPersistentStore.java
@@ -143,6 +143,8 @@ public class CachingObjectPersistentStore {
 
     // Returns an iterator with a view of all commited objects
     // Does not have to guarantee consistent view, snapshots are handled by upper layers
+    // Warning: it has a nasty side effect of global caching, so in this case don't even call next on it,
+    // if some objects are still in writeback
     public CloseableKvIterator<JObjectKey, JDataVersionedWrapper> getIterator(IteratorStart start, JObjectKey key) {
         return new MergingKvIterator<>(
                 new PredicateKvIterator<>(