vldpyatkov commented on code in PR #6611:
URL: https://github.com/apache/ignite-3/pull/6611#discussion_r2386857419


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/InflightTransactionalOperationTracker.java:
##########
@@ -37,21 +37,13 @@ class InflightTransactionalOperationTracker implements 
TransactionalOperationTra
 
     @Override
     public void registerOperationStart(InternalTransaction tx) {
-        if (shouldBeTracked(tx)) {
-            if (!delegate.addInflight(tx.id(), tx.isReadOnly())) {
-                throw new TransactionException(TX_ALREADY_FINISHED_ERR, 
format("Transaction is already finished [tx={}]", tx));
-            }
+        if (!tx.isReadOnly() && !delegate.track(tx.id())) {
+            throw new TransactionException(TX_ALREADY_FINISHED_ERR, 
format("Transaction is already finished [tx={}]", tx));
         }
     }
 
     @Override
     public void registerOperationFinish(InternalTransaction tx) {

Review Comment:
   This method has no implementation anywhere. Does really need it?



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/client/ItThinClientTransactionsTest.java:
##########
@@ -931,6 +931,44 @@ void testBatchScenarioWithNoopEnlistmentExplicit() {
         tx.commit();
     }
 
+    @Test
+    void testExplicitReadWriteTransaction() {
+        ClientTable table = (ClientTable) table();
+
+        KeyValueView<Tuple, Tuple> kvView = table.keyValueView();
+
+        // Load partition map to ensure all entries are directly mapped.
+        Map<Partition, ClusterNode> map = 
table.partitionManager().primaryReplicasAsync().join();
+
+        IgniteImpl server0 = TestWrappers.unwrapIgniteImpl(server(0));
+        IgniteImpl server1 = TestWrappers.unwrapIgniteImpl(server(1));
+
+        List<Tuple> tuples0 = generateKeysForNode(600, 20, map, 
server0.clusterService().topologyService().localMember(), table);
+        List<Tuple> tuples1 = generateKeysForNode(600, 20, map, 
server1.clusterService().topologyService().localMember(), table);

Review Comment:
   It is better to replace it with a public API:
   ```
   server0.cluster().localNode();
   List<Tuple> tuples0 = generateKeysForNode(600, 20, map, 
server0.cluster().localNode(), table);
   ```
   I also looked at generateKeysForNode method. A public cluster node is 
possible to use there.
   



-- 
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