ptupitsyn commented on code in PR #7844:
URL: https://github.com/apache/ignite-3/pull/7844#discussion_r3116461563


##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java:
##########
@@ -94,12 +105,35 @@ public void rollback() throws TransactionException {
     public CompletableFuture<Void> rollbackAsync() {
         var tx0 = tx;
 
+        // TODO: IGNITE-28405 This is really fishy. It will probably let you 
reuse a transaction after calling a rollback :(
         if (tx0 == null) {
             // No operations were performed, nothing to rollback.
             return nullCompletedFuture();
         }
 
-        return tx0.thenCompose(ClientTransaction::rollbackAsync);
+        // If the transaction is not started. Issue the rollback and wait for 
the server response.
+        if (!tx0.isDone() && cancelled.compareAndSet(false, true)) {
+            return requestInfoFuture

Review Comment:
   I think `requestInfoFuture` might never complete: connection fails => 
request is not sent => `onSent` is not called.



##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/DirectTxUtils.java:
##########
@@ -282,6 +284,81 @@ public static CompletableFuture<ClientChannel> 
resolveChannel(
         });
     }
 
+    /**
+     * If the current request is the first request of a direct tx, add a 
listener to the {@link PayloadWriter}.
+     *
+     * @param ctx The {@link WriteContext} that holds transactional context 
information.
+     * @param tx The client transaction associated with the request, or {@code 
null} if none.
+     * @param base Request native {@link PayloadWriter}.
+     * @return The {@link PayloadWriter} that should be used on the request.
+     */
+    public static PayloadWriter payloadWriter(WriteContext ctx, @Nullable 
Transaction tx, PayloadWriter base) {
+        if (ctx.firstReqFut != null && tx instanceof ClientLazyTransaction) {
+            return poc -> {
+                base.accept(poc);
+
+                var clientLazyTx = (ClientLazyTransaction) tx;
+                long requestId = poc.requestId();
+                ClientChannel cc = poc.clientChannel();
+                poc.onSent(() -> clientLazyTx.updateRequestInfo(requestId, 
cc));
+            };
+        } else {
+            return base;
+        }
+    }
+
+    /**
+     * Try to handle error on first request. Returns false if not in the 
context of the first request.
+     * This method essentially populates context with a failed request 
instance.
+     *
+     * @param ctx The {@link WriteContext} that holds transactional context 
information.
+     * @param ch The {@link ReliableChannel} used to resolve the actual 
communication channel.
+     * @param id Client Table Id.

Review Comment:
   Table id?



##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java:
##########
@@ -94,12 +105,35 @@ public void rollback() throws TransactionException {
     public CompletableFuture<Void> rollbackAsync() {
         var tx0 = tx;
 
+        // TODO: IGNITE-28405 This is really fishy. It will probably let you 
reuse a transaction after calling a rollback :(
         if (tx0 == null) {
             // No operations were performed, nothing to rollback.
             return nullCompletedFuture();
         }
 
-        return tx0.thenCompose(ClientTransaction::rollbackAsync);
+        // If the transaction is not started. Issue the rollback and wait for 
the server response.
+        if (!tx0.isDone() && cancelled.compareAndSet(false, true)) {

Review Comment:
   `isDone` can become true concurrently, is that ok? Should we add a comment?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to