(tinkerpop) branch tinkergraph-storage updated: Publish TinkerStorageGraph commits under the storage lock

[sp...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch tinkergraph-storage
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/tinkergraph-storage by this push:
     new 8ae8a29551 Publish TinkerStorageGraph commits under the storage lock
8ae8a29551 is described below

commit 8ae8a29551cf0d4f271fe12de4ed5d011517b5b7
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Wed Sep 9 11:09:40 2026 -0400

    Publish TinkerStorageGraph commits under the storage lock
    
    Compaction builds its snapshot by reading the graph and then discards the log,
    which is only sound while the graph reflects everything the log holds. That was
    false for as long as a changeset sat persisted but not yet applied to memory, so
    a compaction landing in that window snapshotted without the transaction and then
    deleted the record that held it, losing an acknowledged commit with no error
    reported. The in-memory apply now happens inside the same lock as the write to
    the log, and auto-compaction runs after it rather than before. Commits also
    become visible in the order they were recorded.
    
    Assisted-by: Claude Code:claude-opus-5
    Claude-Session: https://claude.ai/code/session_01KgH2VCpRw57sbFg5GoAiVV
---
 .../tinkergraph/structure/TinkerTransaction.java   | 34 ++++++++++++++--------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransaction.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransaction.java
index 36d5167c13..12f585cff3 100644
--- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransaction.java
+++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransaction.java
@@ -181,22 +181,32 @@ final class TinkerTransaction extends AbstractThreadLocalTransaction {
             // aborts the commit (via the catch below) and leaves memory and disk consistent. Skipped while the graph
             // is replaying its storage log on open. Serialized by storageCommitLock because commits of disjoint
             // elements otherwise reach the engine's single append log concurrently and interleave its records.
-            if (graph.storage != null && !graph.loading) {
-                graph.storageCommitLock.lock();
-                try {
+            //
+            // The in-memory apply is held inside that same lock. Compaction builds its snapshot by reading the graph
+            // and then discards the log, which is only sound while the graph reflects everything the log holds. That
+            // is false for exactly as long as a changeset sits persisted but not yet applied, so a compaction landing
+            // in that window snapshots without the transaction and then deletes the record that held it, losing an
+            // acknowledged commit. Publishing under the lock closes the window, and also makes the order in which
+            // transactions become visible match the order they were recorded in.
+            final boolean durable = graph.storage != null && !graph.loading;
+            if (durable) graph.storageCommitLock.lock();
+            try {
+                if (durable) {
                     graph.storage.persist(txVersion, toVertexMutations(changedVertices), toEdgeMutations(changedEdges));
                     graph.storage.flush();
-                    // bound log growth for a long-running graph that is never explicitly closed; no-op unless the
-                    // engine's accumulated log has crossed its threshold
-                    graph.storage.maybeCompact(graph);
-                } finally {
-                    graph.storageCommitLock.unlock();
                 }
-            }
 
-            // commit all changes
-            changedVertices.forEach(v -> v.commit(txVersion));
-            changedEdges.forEach(e -> e.commit(txVersion));
+                // commit all changes
+                changedVertices.forEach(v -> v.commit(txVersion));
+                changedEdges.forEach(e -> e.commit(txVersion));
+
+                // bound log growth for a long-running graph that is never explicitly closed; no-op unless the
+                // engine's accumulated log has crossed its threshold. Runs after the apply so the snapshot it may
+                // write includes this transaction rather than omitting it and then truncating the log that held it.
+                if (durable) graph.storage.maybeCompact(graph);
+            } finally {
+                if (durable) graph.storageCommitLock.unlock();
+            }
         } catch (TransactionException ex) {
             // rollback on error
             changedVertices.forEach(v -> v.rollback());