diff --git a/docs/src/dev/provider/index.asciidoc b/docs/src/dev/provider/index.asciidoc index 7654807a90f..f1c1bc64466 100644 --- a/docs/src/dev/provider/index.asciidoc +++ b/docs/src/dev/provider/index.asciidoc @@ -1260,8 +1260,32 @@ 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. +==== Implicit Transaction Management + +Implicit transactions (requests without a `transactionId`) are not affected by any of the explicit transaction-specific +behavior described above. All graphs participate in implicit transactions regardless of whether they support explicit +transactions. The difference lies in what the server does at the boundaries: + +When the graph supports explicit transactions, the server auto-commits and rolls back implicit transactions: + +* 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. + +When the graph does not support explicit transactions, writes are immediately durable. If a failure occurs +mid-traversal, the graph may be left in a partially mutated state since rollback is not possible. + +Graph system providers implementing their own server or HTTP endpoint should replicate the auto-commit/rollback behavior +when their graph implementation supports explicit 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. Note that because HTTP responses use chunked transfer encoding, the initial HTTP status is sent as 200 OK +before iteration begins. If the commit itself fails, the error will appear in the response body status field and in +the trailing HTTP headers rather than as a non-200 HTTP status code. Clients should therefore check the response body +status (and trailing headers) to confirm that the transaction was committed successfully. + +IMPORTANT: The auto-commit/rollback logic must only apply to implicit transactions. Requests that carry a +`transactionId` are part of an explicit, 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 7a5925eb0a6..18391c5506c 100644 --- a/docs/src/reference/gremlin-applications.asciidoc +++ b/docs/src/reference/gremlin-applications.asciidoc @@ -2242,14 +2242,20 @@ 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()`. - -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 -implementation. Attempting to begin a transaction on a non-transactional graph will result in an error. +Every traversal executes within a transaction. TinkerPop distinguishes between two modes: + +* *Implicit transactions* are the default. Each traversal submitted through `g` is a self-contained unit of work. All +graphs participate in implicit transactions. When the graph supports explicit transactions, implicit transactions are +auto-committed on success and rolled back on error, providing atomicity. When the graph does not support explicit +transactions, writes are immediately durable and cannot be rolled back if a failure occurs mid-traversal, potentially +leaving the graph in a partially mutated state. In either case, a simple `g.addV('person')` will be persisted on +success without requiring an explicit `commit()`. +* *Explicit transactions* are opened via `g.tx().begin()`, where the client controls the lifecycle through +`g.tx().commit()` and `g.tx().rollback()`. Multiple traversals can participate in the same explicit transaction. + +IMPORTANT: Not all graph implementations support explicit transactions (for example, basic `TinkerGraph` does not). +Use `TinkerTransactionGraph` or another graph implementation that supports explicit transactions. Attempting to begin +an explicit transaction on a graph that does not support them will result in an error. Two settings in the Gremlin Server YAML control transaction resource usage: diff --git a/docs/src/reference/the-traversal.asciidoc b/docs/src/reference/the-traversal.asciidoc index a081e209b30..ab49ae2614c 100644 --- a/docs/src/reference/the-traversal.asciidoc +++ b/docs/src/reference/the-traversal.asciidoc @@ -87,11 +87,11 @@ try { The above example is straightforward and represents a good starting point for discussing the nuances of transactions in relation to the usage convention and graph provider caveats alluded to earlier. -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`: +Focusing on remote contexts first, note that it is still possible to issue traversals from `g`, but those will +operate as implicit transactions (as opposed to the explicit transaction opened by `gtx`) and simply behave as +self-contained units of work (i.e. one traversal is one implicit transaction). Each explicit transaction requires 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 f0cb2b9ea2c..b520464d689 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 @@ -422,19 +422,30 @@ private void iterateScriptEvalResult(final Context context, MessageSerializer } final Bindings mergedBindings = mergeBindingsFromRequest(context, new SimpleBindings(graphManager.getAsBindings())); - final Object result = scriptEngine.eval(message.getGremlin(), mergedBindings); - final String bulkingSetting = context.getChannelHandlerContext().channel().attr(StateKey.REQUEST_HEADERS).get().get(Tokens.BULK_RESULTS); - // bulking only applies if it's gremlin-lang, and per request token setting takes precedence over header setting. - // The serializer check is temporarily needed because GraphSON hasn't been removed yet and doesn't support bulking. - final boolean bulking = language.equals("gremlin-lang") && serializer instanceof GraphBinaryMessageSerializerV4 ? - (args.containsKey(Tokens.BULK_RESULTS) ? - Objects.equals(args.get(Tokens.BULK_RESULTS), "true") : - Objects.equals(bulkingSetting, "true")) : - false; + // 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(); Iterator itty = null; try { + final Object result = scriptEngine.eval(message.getGremlin(), mergedBindings); + + final String bulkingSetting = context.getChannelHandlerContext().channel().attr(StateKey.REQUEST_HEADERS).get().get(Tokens.BULK_RESULTS); + // bulking only applies if it's gremlin-lang, and per request token setting takes precedence over header setting. + // The serializer check is temporarily needed because GraphSON hasn't been removed yet and doesn't support bulking. + final boolean bulking = language.equals("gremlin-lang") && serializer instanceof GraphBinaryMessageSerializerV4 ? + (args.containsKey(Tokens.BULK_RESULTS) ? + Objects.equals(args.get(Tokens.BULK_RESULTS), "true") : + Objects.equals(bulkingSetting, "true")) : + false; + if (bulking) { // optimization for driver requests ((Traversal.Admin) result).applyStrategies(); @@ -444,7 +455,11 @@ private void iterateScriptEvalResult(final Context context, MessageSerializer 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()); @@ -452,7 +467,7 @@ private void iterateScriptEvalResult(final Context context, MessageSerializer 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 3cb37d06a19..af35cf43923 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 void shouldRejectTransactionWithUnsupportedTypeInGValue() throws Exceptio 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 895b09b75c2..7d724975b9b 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 Settings overrideSettings(final Settings settings) { settings.maxRequestContentLength = 31; break; case "should200OnPOSTTransactionalGraph": + case "shouldRollbackOnFailedMutatingTraversal": + case "shouldCommitMutatingTraversalWithEmptyResult": useTinkerTransactionGraph(settings); break; case "should200OnPOSTTransactionalGraphInStrictMode": @@ -489,36 +491,97 @@ public void should200OnPOSTWithGremlinJsonEndcodedBodyForJavaTime() throws Excep } } - /*@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 {