This is an automated email from the ASF dual-hosted git repository. kenhuuu pushed a commit to branch implicit-tx in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 188a399e010cbe2b90629b860b3f5f1180bcb547 Author: Ken Hu <[email protected]> AuthorDate: Tue May 12 16:51:15 2026 -0700 Enable autocommit in GremlinServer This behaves the same as the TraversalOpProcessor would have in the 3.x line. All traversals are now transactional if the underlying Graph supports transactions. Traversals that aren't explicitly in a transaction are now wrapped into their own implicit transaction and the server will autocommit on sucess and rollback on failure. Assisted-by: Kiro:claude-opus-4-6 --- docs/src/dev/provider/index.asciidoc | 21 ++++- docs/src/reference/gremlin-applications.asciidoc | 12 ++- docs/src/reference/the-traversal.asciidoc | 8 +- .../server/handler/HttpGremlinEndpointHandler.java | 19 ++++- .../GremlinDriverTransactionIntegrateTest.java | 27 +++++++ .../server/GremlinServerHttpIntegrateTest.java | 91 ++++++++++++++++++---- 6 files changed, 153 insertions(+), 25 deletions(-) diff --git a/docs/src/dev/provider/index.asciidoc b/docs/src/dev/provider/index.asciidoc index 7654807a90..f6d93aaa66 100644 --- a/docs/src/dev/provider/index.asciidoc +++ b/docs/src/dev/provider/index.asciidoc @@ -1260,8 +1260,25 @@ Graph transactions are typically `ThreadLocal`-bound. The server maintains a sin to ensure all operations execute on the same thread. This is an important implementation detail for graph system providers whose `Transaction` implementation relies on thread-local state. -NOTE: Non-transactional requests (those without a `transactionId`) are not affected by any of the transaction-specific -behavior described above. They continue to operate exactly as before. +==== Auto-Commit for Non-Transactional Requests + +Non-transactional requests (those without a `transactionId`) are not affected by any of the transaction-specific +behavior described above. However, if the underlying graph supports transactions, the server must automatically manage +transactions for these requests: + +* Before processing: roll back any stale open transaction to prevent state leakage between requests. +* On success: commit the transaction after the traversal has been fully iterated and serialized. +* On error: roll back the transaction so that partial mutations are not persisted. + +This auto-commit behavior ensures that users who do not use explicit transactions still get durable writes on success +and clean rollback on failure. Graph system providers implementing their own server or HTTP endpoint must replicate +this behavior if their graph implementation supports transactions. The commit occurs after serialization is complete +but before the final response is flushed to the client, so the client only receives a success response after the +data is committed. + +IMPORTANT: This auto-commit/rollback logic must only apply to non-transactional requests. Requests that carry a +`transactionId` are part of a client-managed transaction and must not be auto-committed or auto-rolled-back by the +server. === HTTP Request Interceptor diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc index 7a5925eb0a..f4684863e6 100644 --- a/docs/src/reference/gremlin-applications.asciidoc +++ b/docs/src/reference/gremlin-applications.asciidoc @@ -2242,10 +2242,14 @@ above with the use of the `maximumSize`. [[considering-transactions]] ==== Considering Transactions -Non-transactional requests (those without a `transactionId`) behave as self-contained units of work where the graph's -own transaction semantics apply. Each traversal executes within its own transaction as managed by the graph -implementation itself. Transactional requests participate in a transaction opened via `g.tx().begin()`, where the -client explicitly controls the lifecycle through `g.tx().commit()` and `g.tx().rollback()`. +Non-transactional requests (those without a `transactionId`) behave as self-contained units of work. If the underlying +graph supports transactions, the server automatically commits after successfully iterating the traversal and +automatically rolls back on any error. This means that a simple `g.addV('person')` will be persisted immediately upon +success without requiring an explicit `commit()`. Before processing each non-transactional request, the server also +rolls back any stale open transaction to prevent state leakage between requests. + +Transactional requests participate in a transaction opened via `g.tx().begin()`, where the client explicitly controls +the lifecycle through `g.tx().commit()` and `g.tx().rollback()`. IMPORTANT: Understand the transactional capabilities of the graph configured in Gremlin Server. For example, a basic `TinkerGraph` does not support transactions. Use `TinkerTransactionGraph` or another transaction-capable graph diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index a081e209b3..bfea6f5265 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -89,9 +89,11 @@ in relation to the usage convention and graph provider caveats alluded to earlie Focusing on remote contexts first, note that it is still possible to issue traversals from `g`, but those will have a transaction scope outside of `gtx` and will simply behave as self-contained units of work where the graph's own -transaction semantics apply (i.e. one traversal is one transaction). Each isolated transaction will require its own -`Transaction` object. Multiple `begin()` calls on the same `Transaction` object are not permitted and will throw an -`IllegalStateException`: +transaction semantics apply (i.e. one traversal is one transaction). For graphs that support transactions, the server +automatically commits on successful completion of the traversal and rolls back on any error, so a simple +`g.addV('person')` will be persisted immediately upon success without requiring an explicit `commit()`. Each isolated +transaction will require its own `Transaction` object. Multiple `begin()` calls on the same `Transaction` object are +not permitted and will throw an `IllegalStateException`: [source,java] ---- diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java index 7c474d6ba0..7631563ba7 100644 --- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java +++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java @@ -418,6 +418,17 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ } final Bindings mergedBindings = mergeBindingsFromRequest(context, new SimpleBindings(graphManager.getAsBindings())); + + // resolve the graph for auto-transaction management on non-transactional requests + final String g = message.getField(Tokens.ARGS_G); + final TraversalSource ts = g != null ? graphManager.getTraversalSource(g) : null; + final Graph graph = ts != null ? ts.getGraph() : null; + final boolean autoCommit = (context.getTransactionId() == null) && (graph != null) && + graph.features().graph().supportsTransactions(); + + // rollback any stale open transaction before processing + if (autoCommit && graph.tx().isOpen()) graph.tx().rollback(); + final Object result = scriptEngine.eval(message.getGremlin(), mergedBindings); final String bulkingSetting = context.getChannelHandlerContext().channel().attr(StateKey.REQUEST_HEADERS).get().get(Tokens.BULK_RESULTS); @@ -440,7 +451,11 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ itty = IteratorUtils.asIterator(result); handleIterator(context, itty, serializer, false); } - } catch (Exception ex) { + + if (autoCommit && graph.tx().isOpen()) graph.tx().commit(); + } catch (Throwable t) { + if (autoCommit && graph.tx().isOpen()) graph.tx().rollback(); + // TINKERPOP-3144 ensure Traversals are closed when exception thrown. if (itty instanceof TraverserIterator) { CloseableIterator.closeIterator(((TraverserIterator) itty).getTraversal()); @@ -448,7 +463,7 @@ public class HttpGremlinEndpointHandler extends SimpleChannelInboundHandler<Requ CloseableIterator.closeIterator(itty); } - throw ex; + throw t; } } diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java index 3cb37d06a1..af35cf4392 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverTransactionIntegrateTest.java @@ -647,4 +647,31 @@ public class GremlinDriverTransactionIntegrateTest extends AbstractGremlinServer gtx.tx().rollback(); } } + + @Test + public void shouldAutoCommitNonTransactionalMutatingTraversal() throws Exception { + final Client client = cluster.connect().alias(GTX); + + // add a vertex without explicit transaction management - should be auto-committed + client.submit("g.addV('person').property('name','alice')").all().get(); + + // verify persisted on a subsequent request + assertEquals(1L, client.submit("g.V().hasLabel('person').count()").all().get().get(0).getLong()); + } + + @Test + public void shouldAutoRollbackOnFailedNonTransactionalMutatingTraversal() throws Exception { + final Client client = cluster.connect().alias(GTX); + + // submit a traversal that mutates then fails - should be rolled back + try { + client.submit("g.addV('person').fail()").all().get(); + fail("Expected an exception"); + } catch (Exception ex) { + // expected + } + + // verify nothing was persisted + assertEquals(0L, client.submit("g.V().hasLabel('person').count()").all().get().get(0).getLong()); + } } diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java index 895b09b75c..7d724975b9 100644 --- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java +++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java @@ -111,6 +111,8 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra settings.maxRequestContentLength = 31; break; case "should200OnPOSTTransactionalGraph": + case "shouldRollbackOnFailedMutatingTraversal": + case "shouldCommitMutatingTraversalWithEmptyResult": useTinkerTransactionGraph(settings); break; case "should200OnPOSTTransactionalGraphInStrictMode": @@ -489,36 +491,97 @@ public class GremlinServerHttpIntegrateTest extends AbstractGremlinServerIntegra } } - /*@Test disabled for now as current implementation doesn't support implicit transactions. + @Test public void should200OnPOSTTransactionalGraph() throws Exception { final CloseableHttpClient httpclient = HttpClients.createDefault(); + + // add a vertex without an explicit transaction - should be auto-committed final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); httppost.addHeader("Content-Type", "application/json"); - httppost.setEntity(new StringEntity("{\"gremlin\":\"graph.addVertex('name','stephen');g.V().count()\"}", Consts.UTF_8)); + httppost.setEntity(new StringEntity( + "{\"gremlin\":\"g.addV('person').property('name','stephen')\",\"g\":\"g\"}", Consts.UTF_8)); try (final CloseableHttpResponse response = httpclient.execute(httppost)) { assertEquals(200, response.getStatusLine().getStatusCode()); - assertEquals("application/json", response.getEntity().getContentType().getValue()); - final String json = EntityUtils.toString(response.getEntity()); - final JsonNode node = mapper.readTree(json); - assertEquals(1, node.get("result").get(TOKEN_DATA).get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).intValue()); + EntityUtils.consume(response.getEntity()); } - final HttpGet httpget = new HttpGet(TestClientFactory.createURLString("?gremlin=g.V().count()")); - httpget.addHeader("Accept", "application/json"); + // verify the vertex is visible on subsequent requests from potentially different threads + final HttpPost countPost = new HttpPost(TestClientFactory.createURLString()); + countPost.addHeader("Content-Type", "application/json"); + countPost.setEntity(new StringEntity("{\"gremlin\":\"g.V().count()\",\"g\":\"g\"}", Consts.UTF_8)); - // execute this a bunch of times so that there's a good chance a different thread on the server processes - // the request for (int ix = 0; ix < 100; ix++) { - try (final CloseableHttpResponse response = httpclient.execute(httpget)) { + try (final CloseableHttpResponse response = httpclient.execute(countPost)) { assertEquals(200, response.getStatusLine().getStatusCode()); - assertEquals("application/json", response.getEntity().getContentType().getValue()); final String json = EntityUtils.toString(response.getEntity()); final JsonNode node = mapper.readTree(json); - assertEquals(1, node.get("result").get(TOKEN_DATA).get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).intValue()); + assertEquals(1, node.get("result").get(TOKEN_DATA) + .get(GraphSONTokens.VALUEPROP).get(0) + .get(GraphSONTokens.VALUEPROP).intValue()); } } - } */ + } + + @Test + public void shouldRollbackOnFailedMutatingTraversal() throws Exception { + final CloseableHttpClient httpclient = HttpClients.createDefault(); + + // submit a traversal that adds a vertex then fails - should be rolled back + final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); + httppost.addHeader("Content-Type", "application/json"); + httppost.setEntity(new StringEntity("{\"gremlin\":\"g.addV('person').fail()\",\"g\":\"g\"}", Consts.UTF_8)); + + try (final CloseableHttpResponse response = httpclient.execute(httppost)) { + // the fail() error appears in the response body, not the HTTP status + } + + // verify the vertex was not persisted + final HttpPost countPost = new HttpPost(TestClientFactory.createURLString()); + countPost.addHeader("Content-Type", "application/json"); + countPost.setEntity(new StringEntity( + "{\"gremlin\":\"g.V().hasLabel('person').count()\",\"g\":\"g\"}", Consts.UTF_8)); + + try (final CloseableHttpResponse response = httpclient.execute(countPost)) { + assertEquals(200, response.getStatusLine().getStatusCode()); + final String json = EntityUtils.toString(response.getEntity()); + final JsonNode node = mapper.readTree(json); + assertEquals(0, node.get("result").get(TOKEN_DATA) + .get(GraphSONTokens.VALUEPROP).get(0) + .get(GraphSONTokens.VALUEPROP).intValue()); + } + } + + @Test + public void shouldCommitMutatingTraversalWithEmptyResult() throws Exception { + final CloseableHttpClient httpclient = HttpClients.createDefault(); + + // g.addV() followed by iterate-style consumption returns no results but should still commit + final HttpPost httppost = new HttpPost(TestClientFactory.createURLString()); + httppost.addHeader("Content-Type", "application/json"); + httppost.setEntity(new StringEntity( + "{\"gremlin\":\"g.addV('person').property('name','p').iterate()\",\"g\":\"g\"}", Consts.UTF_8)); + + try (final CloseableHttpResponse response = httpclient.execute(httppost)) { + assertEquals(200, response.getStatusLine().getStatusCode()); + EntityUtils.consume(response.getEntity()); + } + + // verify the vertex was committed despite the empty result set + final HttpPost countPost2 = new HttpPost(TestClientFactory.createURLString()); + countPost2.addHeader("Content-Type", "application/json"); + countPost2.setEntity(new StringEntity( + "{\"gremlin\":\"g.V().hasLabel('person').count()\",\"g\":\"g\"}", Consts.UTF_8)); + + try (final CloseableHttpResponse response = httpclient.execute(countPost2)) { + assertEquals(200, response.getStatusLine().getStatusCode()); + final String json = EntityUtils.toString(response.getEntity()); + final JsonNode node = mapper.readTree(json); + assertEquals(1, node.get("result").get(TOKEN_DATA) + .get(GraphSONTokens.VALUEPROP).get(0) + .get(GraphSONTokens.VALUEPROP).intValue()); + } + } @Test public void should200OnPOSTTransactionalGraphInStrictMode() throws Exception {
