This is an automated email from the ASF dual-hosted git repository.
colegreer pushed a commit to branch 3.7-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/3.7-dev by this push:
new f9245f1c43 TINKERPOP-3141 (#3161)
f9245f1c43 is described below
commit f9245f1c43f71f1abdc0a72ed64c6009ff1a2cbe
Author: andreachild <[email protected]>
AuthorDate: Mon Jul 21 17:40:31 2025 -0700
TINKERPOP-3141 (#3161)
Account for the scenario of dropping and re-adding a vertex within the same
transaction by adding a new unmarkDeleted method to the TinkerElementContainer
and calling unmarkDeleted from addVertex if it is detected that the vertex had
been previously deleted. Added logic to reference the previously deleted vertex
version when adding the vertex back to avoid transaction conflict error.
---
CHANGELOG.asciidoc | 1 +
.../structure/TinkerElementContainer.java | 14 ++
.../structure/TinkerTransactionGraph.java | 10 +-
.../structure/TinkerTransactionGraphTest.java | 245 +++++++++++++++++++++
4 files changed, 269 insertions(+), 1 deletion(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 708e17cc72..6244b2a3a1 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -54,6 +54,7 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Support hot reloading of SSL certificates.
* Increase default `max_content_length`/`max_msg_size` in `gremlin-python`
from 4MB to 10MB.
* Added the `PopContaining` interface designed to get label and `Pop`
combinations held in a `PopInstruction` object.
+* Fixed bug preventing a vertex from being dropped and then re-added in the
same `TinkerTransaction`
[[release-3-7-3]]
=== TinkerPop 3.7.3 (October 23, 2024)
diff --git
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerElementContainer.java
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerElementContainer.java
index 4d7216d21b..afd875aff1 100644
---
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerElementContainer.java
+++
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerElementContainer.java
@@ -153,6 +153,20 @@ final class TinkerElementContainer<T extends
TinkerElement> {
}
}
+ /**
+ * Unmark element as deleted in the current transaction. Does nothing if
the element was not previously marked as deleted.
+ *
+ * @see #markDeleted(TinkerTransaction)
+ */
+ public void unmarkDeleted(final TinkerTransaction tx) {
+ // only unmark as deleted if it was previously marked as deleted
+ if (isDeletedInTx.get()) {
+ usesInTransactions.decrementAndGet();
+ isDeletedInTx.set(false);
+ tx.markChanged(this);
+ }
+ }
+
/**
* Mark element as changed in the current transaction.
* A copy of the element is made and set as a value in the transaction.
diff --git
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraph.java
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraph.java
index 286688cd3d..0874f66e22 100644
---
a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraph.java
+++
b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraph.java
@@ -152,11 +152,19 @@ public final class TinkerTransactionGraph extends
AbstractTinkerGraph {
if (container != null && container.get() != null)
throw Exceptions.vertexWithIdAlreadyExists(idValue);
+ long version = txNumber;
+ if (container != null && container.isDeleted() &&
container.getModified() != null) {
+ // vertex being added was previously deleted
+ // we need to reference the version from the deleted state when
adding the vertex back
+ version = container.getModified().version();
+ container.unmarkDeleted((TinkerTransaction) tx());
+ }
+
// no existing container, let's use new one
if (container == null)
container = newContainer;
- final TinkerVertex vertex = new TinkerVertex(idValue, label, this,
txNumber);
+ final TinkerVertex vertex = new TinkerVertex(idValue, label, this,
version);
ElementHelper.attachProperties(vertex,
VertexProperty.Cardinality.list, keyValues);
container.setDraft(vertex, (TinkerTransaction) tx());
diff --git
a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraphTest.java
b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraphTest.java
index 88b52413b6..ce3f73f756 100644
---
a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraphTest.java
+++
b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerTransactionGraphTest.java
@@ -18,10 +18,12 @@
*/
package org.apache.tinkerpop.gremlin.tinkergraph.structure;
+import java.util.List;
import
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.structure.VertexProperty;
import org.apache.tinkerpop.gremlin.structure.util.TransactionException;
import org.junit.Test;
@@ -33,6 +35,7 @@ import java.util.concurrent.atomic.AtomicLong;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
@@ -40,6 +43,7 @@ import static org.junit.Assert.fail;
public class TinkerTransactionGraphTest {
+ private static final String PROPERTY_NAME = "name";
final Object vid = 100;
///// vertex tests
@@ -1503,4 +1507,245 @@ public class TinkerTransactionGraphTest {
assertFalse(vertex.inUse());
}
+
+ @Test
+ public void shouldAllowDropAddVertexInSameTransaction() {
+ final TinkerTransactionGraph g = TinkerTransactionGraph.open();
+ final TinkerTransaction tx = (TinkerTransaction) g.tx();
+
+ // create initial vertex with a property
+ final GraphTraversalSource gtx = tx.begin();
+ Vertex vertex = gtx.addV().property(T.id, vid).property(PROPERTY_NAME,
"test").next();
+ // add self-referencing edge
+ gtx.addE("tests").from(vertex).to(vertex).next();
+ gtx.tx().commit();
+
+ verifyCommittedSingleVertex(g, "test");
+ assertEquals(1, g.getEdgesCount());
+ final long originalVersion =
g.getVertices().get(vid).getUnmodified().version();
+
+ // drop the vertex and re-create it without any properties or edges in
the same transaction
+ final GraphTraversalSource gtx2 = tx.begin();
+ gtx2.V().drop().iterate();
+ gtx2.addV().property(T.id, vid).next();
+ gtx2.tx().commit();
+
+ verifyCommittedSingleVertex(g);
+ final TinkerVertex updatedVertex =
g.getVertices().get(vid).getUnmodified();
+ assertEquals(0, g.getEdgesCount());
+ // version should have been updated
+ assertNotEquals(originalVersion, updatedVertex.version());
+ }
+
+ @Test
+ public void shouldAllowAddDropAddVertexInSameTransaction() {
+ final TinkerTransactionGraph g = TinkerTransactionGraph.open();
+ final TinkerTransaction tx = (TinkerTransaction) g.tx();
+
+ final GraphTraversalSource gtx = tx.begin();
+ // create vertex with a property
+ final Vertex vertex = gtx.addV().property(T.id,
vid).property(PROPERTY_NAME, "test").next();
+ // add self-referencing edge
+ gtx.addE("tests").from(vertex).to(vertex).next();
+ // drop vertex
+ gtx.V(vid).drop().iterate();
+ // add vertex again with different properties but no edges
+ gtx.addV().property(T.id, vid).property(PROPERTY_NAME, "updated
name").next();
+ gtx.tx().commit();
+
+ verifyCommittedSingleVertex(g, "updated name");
+ assertEquals(0, g.getEdgesCount());
+ }
+
+ @Test
+ public void shouldAllowAddDropAddDropVertexInSameTransaction() {
+ final TinkerTransactionGraph g = TinkerTransactionGraph.open();
+ final TinkerTransaction tx = (TinkerTransaction) g.tx();
+
+ final GraphTraversalSource gtx = tx.begin();
+ // create vertex with a property
+ final Vertex vertex = gtx.addV().property(T.id,
vid).property(PROPERTY_NAME, "test").next();
+ // add self-referencing edge
+ gtx.addE("tests").from(vertex).to(vertex).next();
+ // drop vertex
+ gtx.V(vid).drop().iterate();
+ // add vertex again with different properties but no edges
+ gtx.addV().property(T.id, vid).property(PROPERTY_NAME, "updated
name").next();
+ // drop the vertex again
+ gtx.V(vid).drop().iterate();
+ gtx.tx().commit();
+
+ assertEquals(0, g.getVerticesCount());
+ assertEquals(0, g.getEdgesCount());
+ }
+
+ @Test
+ public void shouldAllowAddDropAddDropVertexInMultipleTransactions() {
+ final TinkerTransactionGraph g = TinkerTransactionGraph.open();
+ final TinkerTransaction tx = (TinkerTransaction) g.tx();
+
+ // first transaction
+ final GraphTraversalSource gtx1 = tx.begin();
+ // create vertex with a property
+ final Vertex vertex = gtx1.addV().property(T.id,
vid).property(PROPERTY_NAME, "test").next();
+ // add self-referencing edge
+ gtx1.addE("tests").from(vertex).to(vertex).next();
+ // drop vertex
+ gtx1.V(vid).drop().iterate();
+ gtx1.tx().commit();
+
+ assertEquals(0, g.getVerticesCount());
+ assertEquals(0, g.getEdgesCount());
+
+ // second transaction
+ final GraphTraversalSource gtx2 = tx.begin();
+ // add vertex again with different properties but no edges
+ gtx2.addV().property(T.id, vid).property(PROPERTY_NAME, "updated
name").next();
+ // drop the vertex again
+ gtx2.V(vid).drop().iterate();
+ gtx2.tx().commit();
+
+ assertEquals(0, g.getVerticesCount());
+ assertEquals(0, g.getEdgesCount());
+ }
+
+ @Test
+ public void shouldAllowDropAddUniqueVertexInConcurrentTransactions() {
+ final TinkerTransactionGraph g = TinkerTransactionGraph.open();
+ final TinkerTransaction tx = (TinkerTransaction) g.tx();
+
+ // create first vertex and commit
+ final GraphTraversalSource gtx = tx.begin();
+ gtx.addV().property(T.id, vid).next();
+ gtx.tx().commit();
+ verifyCommittedSingleVertex(g);
+
+ // drop and add but don't commit
+ gtx.V(vid).drop().iterate();
+ gtx.addV().property(T.id, vid).property(PROPERTY_NAME, "main
thread").next();
+
+ // in another thread add, drop, add a different vertex and commit
+ int vid2 = (int) vid + 1;
+ runInNewThread(() -> {
+ final GraphTraversalSource gtx2 = g.tx().begin();
+ gtx2.addV().property(T.id, vid2).next();
+ gtx2.V(vid2).drop().iterate();
+ gtx2.addV().property(T.id, vid2).property(PROPERTY_NAME, "another
thread").next();
+ gtx2.tx().commit();
+ });
+
+ // commit the drop add
+ gtx.tx().commit();
+ // both vertices should exist
+ assertEquals(2, g.getVerticesCount());
+ assertEquals("main thread",
g.getVertices().get(vid).getUnmodified().properties.get(PROPERTY_NAME).get(0).value());
+ assertEquals("another thread",
g.getVertices().get(vid2).getUnmodified().properties.get(PROPERTY_NAME).get(0).value());
+ }
+
+ @Test
+ public void shouldPreventDropAddVertexInConcurrentTransactions() {
+ final TinkerTransactionGraph g = TinkerTransactionGraph.open();
+ final TinkerTransaction tx = (TinkerTransaction) g.tx();
+
+ // create initial vertex and commit transaction
+ final GraphTraversalSource gtx = tx.begin();
+ gtx.addV().property(T.id, vid).property(PROPERTY_NAME,
"original").next();
+ gtx.tx().commit();
+ verifyCommittedSingleVertex(g, "original");
+
+ // drop the vertex and add it back but don't commit
+ gtx.V(vid).drop().iterate();
+ gtx.addV().property(T.id, vid).property(PROPERTY_NAME, "should
fail!").next();
+
+ // drop the vertex and add it back in another thread and commit
+ runInNewThread(() -> {
+ final GraphTraversalSource gtx2 = g.tx().begin();
+ gtx2.V(vid).drop().iterate();
+ gtx2.addV().property(T.id, vid).property(PROPERTY_NAME,
"updated").next();
+ gtx2.tx().commit();
+ });
+
+ // try to commit the drop and add in main thread - should fail
+ try {
+ gtx.tx().commit();
+ fail("should throw TransactionException");
+ } catch (TransactionException e) {
+ assertEquals("Conflict: element modified in another transaction",
e.getMessage());
+ verifyCommittedSingleVertex(g, "updated");
+ }
+ }
+
+ @Test
+ public void shouldPreventDropUpdateVertexInConcurrentTransactions() {
+ final TinkerTransactionGraph g = TinkerTransactionGraph.open();
+ final TinkerTransaction tx = (TinkerTransaction) g.tx();
+
+ // create initial vertex and commit transaction
+ final GraphTraversalSource gtx = tx.begin();
+ gtx.addV().property(T.id, vid).property(PROPERTY_NAME,
"original").next();
+ gtx.tx().commit();
+ verifyCommittedSingleVertex(g, "original");
+
+ // drop but don't commit
+ gtx.V(vid).drop().iterate();
+
+ // modify the vertex in another thread and commit
+ runInNewThread(() -> {
+ final GraphTraversalSource gtx2 = g.tx().begin();
+ gtx2.V(vid).property(PROPERTY_NAME, "updated").next();
+ gtx2.tx().commit();
+ });
+
+ // try to commit the drop vertex - should fail
+ try {
+ gtx.tx().commit();
+ fail("should throw TransactionException");
+ } catch (TransactionException e) {
+ assertEquals("Conflict: element modified in another transaction",
e.getMessage());
+ verifyCommittedSingleVertex(g, "updated");
+ }
+ }
+
+ private void runInNewThread(final Runnable runnable) {
+ final Thread thread = new Thread(runnable);
+ thread.start();
+ try {
+ thread.join();
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private void verifyCommittedSingleVertex(final TinkerTransactionGraph g) {
+ verifyCommittedSingleVertex(g, null);
+ }
+
+ private void verifyCommittedSingleVertex(final TinkerTransactionGraph g,
final String namePropertyValue) {
+ // graph should only have a single vertex
+ assertEquals(1, g.getVertices().size());
+
+ // the single vertex should have the expected id
+ final TinkerElementContainer<TinkerVertex> vertexContainer =
g.getVertices().get(vid);
+
+ // container should be committed and not have an update in progress
+ assertFalse(vertexContainer.isChanged());
+ assertFalse(vertexContainer.inUse());
+
+ // the element in the container should have the same id
+ // there are multiple ways to obtain the container element's id and
they should be consistent
+ assertEquals(vid, vertexContainer.getElementId());
+ assertEquals(vid, vertexContainer.get().id());
+ final TinkerVertex unmodified = vertexContainer.getUnmodified();
+ assertEquals(vid, unmodified.id());
+
+ if (namePropertyValue != null) {
+ final Map<String, List<VertexProperty>> properties =
g.getVertices().get(vid).getUnmodified().properties;
+ assertEquals(1, properties.size());
+ final List<VertexProperty> nameProperties =
properties.get(PROPERTY_NAME);
+ assertEquals(1, nameProperties.size());
+ assertEquals(namePropertyValue, nameProperties.get(0).value());
+ } else {
+ assertNull(vertexContainer.getUnmodified().properties);
+ }
+ }
}
