Copilot commented on code in PR #6094:
URL: https://github.com/apache/ignite-3/pull/6094#discussion_r2161209364


##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionCommitRequest.java:
##########
@@ -140,6 +140,10 @@ public static CompletableFuture<ResponseWriter> process(
 
             metrics.transactionsActiveDecrement();
 
+            if (err != null) {
+                throw ExceptionUtils.sneakyThrow(err);

Review Comment:
   Instead of throwing via `ExceptionUtils.sneakyThrow` inside the callback, 
consider returning a failed future (e.g., `return 
CompletableFuture.failedFuture(err);`) to keep the error within the async flow.
   ```suggestion
                   return CompletableFuture.failedFuture(err);
   ```



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientTransactionsTest.java:
##########
@@ -389,6 +392,27 @@ void testReadWriteTxAttributes() {
         tx.rollback();
     }
 
+    @Test
+    void testKillTransaction() {
+        @SuppressWarnings("resource") IgniteClient client = client();
+        KeyValueView<Integer, String> kvView = kvView();
+
+        Transaction tx = client.transactions().begin();
+        kvView.put(tx, 1, "1");
+
+        try (ResultSet<SqlRow> cursor = client.sql().execute(null, "SELECT 
TRANSACTION_ID FROM SYSTEM.TRANSACTIONS")) {
+            cursor.forEachRemaining(r -> {
+                String txId = r.stringValue("TRANSACTION_ID");
+                client.sql().executeScript("KILL TRANSACTION '" + txId + "'");
+            });
+        }
+
+        TransactionException ex = assertThrows(TransactionException.class, 
tx::commit);
+
+        assertThat(ex.getMessage(), startsWith("Transaction is killed"));
+        assertEquals("IGN-TX-13", ex.codeAsString());

Review Comment:
   [nitpick] Consider using try-with-resources for the `Transaction` to ensure 
it’s always closed, e.g., `try (Transaction tx = client.transactions().begin()) 
{ ... }`.
   ```suggestion
           try (Transaction tx = client.transactions().begin()) {
               kvView.put(tx, 1, "1");
   
               try (ResultSet<SqlRow> cursor = client.sql().execute(null, 
"SELECT TRANSACTION_ID FROM SYSTEM.TRANSACTIONS")) {
                   cursor.forEachRemaining(r -> {
                       String txId = r.stringValue("TRANSACTION_ID");
                       client.sql().executeScript("KILL TRANSACTION '" + txId + 
"'");
                   });
               }
   
               TransactionException ex = 
assertThrows(TransactionException.class, tx::commit);
   
               assertThat(ex.getMessage(), startsWith("Transaction is killed"));
               assertEquals("IGN-TX-13", ex.codeAsString());
           }
   ```



-- 
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: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to