You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "xtern (via GitHub)" <gi...@apache.org> on 2025/02/10 16:03:27 UTC

[PR] IGNITE-23488 Integrate cancellation of transaction with sql kill handler [ignite-3]

xtern opened a new pull request, #5207:
URL: https://github.com/apache/ignite-3/pull/5207

   https://issues.apache.org/jira/browse/IGNITE-23488


-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-23488 Integrate cancellation of transaction with sql kill handler [ignite-3]

Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern merged PR #5207:
URL: https://github.com/apache/ignite-3/pull/5207


-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-23488 Integrate cancellation of transaction with sql kill handler [ignite-3]

Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #5207:
URL: https://github.com/apache/ignite-3/pull/5207#discussion_r1950304142


##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -124,19 +110,15 @@ static class TxInfo {
         private final String type;
         private final String priority;
 
-        static TxInfo readOnly(UUID id) {
-            return new TxInfo(id, PENDING, true);
-        }
+        TxInfo(UUID id, TxStateMeta txStateMeta) {
+            InternalTransaction tx = txStateMeta.tx();
 
-        static TxInfo readWrite(UUID id, TxState txState) {
-            return new TxInfo(id, txState, false);
-        }
+            assert tx != null;
 
-        private TxInfo(UUID id, @Nullable TxState state, boolean readOnly) {
             this.id = id.toString();
-            this.state = state == null ? null : state.name();
+            this.state = txStateMeta.txState().name();

Review Comment:
   let's remove `@Nullable` from `state`



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -93,27 +86,20 @@ public SystemView<?> get() {
     static class TxInfoDataSource implements Iterable<TxInfo> {
         private final UUID localNodeId;
 
-        private final Iterable<UUID> roTxIds;
-
         private final Map<UUID, TxStateMeta> rwTxStates;
 
-        TxInfoDataSource(UUID localNodeId, Iterable<UUID> roTxIds, Map<UUID, TxStateMeta> rwTxStates) {
+        TxInfoDataSource(UUID localNodeId, Map<UUID, TxStateMeta> rwTxStates) {
             this.localNodeId = localNodeId;
-            this.roTxIds = roTxIds;
             this.rwTxStates = rwTxStates;
         }
 
         @Override
         public Iterator<TxInfo> iterator() {
-            return CollectionUtils.concat(
-                    new TransformingIterator<>(roTxIds.iterator(), TxInfo::readOnly),
-                    rwTxStates.entrySet().stream()
+            return rwTxStates.entrySet().stream()
                             .filter(e -> localNodeId.equals(e.getValue().txCoordinatorId())
-                                    && e.getValue().tx() != null && !e.getValue().tx().isReadOnly()
-                                    && !isFinalState(e.getValue().txState()))
-                            .map(e -> TxInfo.readWrite(e.getKey(), e.getValue().txState()))
-                            .iterator()
-            );
+                                    && e.getValue().tx() != null && !e.getValue().tx().isFinishingOrFinished())

Review Comment:
   I think, previous condition `!isFinalState(e.getValue().txState())` was more correct than new one `!e.getValue().tx().isFinishingOrFinished()`. The latter excludes transactions in FINISHING state, which is quite important.



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -93,27 +86,20 @@ public SystemView<?> get() {
     static class TxInfoDataSource implements Iterable<TxInfo> {
         private final UUID localNodeId;
 
-        private final Iterable<UUID> roTxIds;
-
         private final Map<UUID, TxStateMeta> rwTxStates;

Review Comment:
   if `rwTxStates` now keeps states for both RO and RW transactions, it's better to rename it accordingly 



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -124,19 +110,15 @@ static class TxInfo {
         private final String type;
         private final String priority;
 
-        static TxInfo readOnly(UUID id) {
-            return new TxInfo(id, PENDING, true);
-        }
+        TxInfo(UUID id, TxStateMeta txStateMeta) {

Review Comment:
   you don't need to pass `id` explicitly, this can be derived from InternalTransaction held by state



-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-23488 Integrate cancellation of transaction with sql kill handler [ignite-3]

Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #5207:
URL: https://github.com/apache/ignite-3/pull/5207#discussion_r1950586835


##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -124,19 +110,15 @@ static class TxInfo {
         private final String type;
         private final String priority;
 
-        static TxInfo readOnly(UUID id) {
-            return new TxInfo(id, PENDING, true);
-        }
+        TxInfo(UUID id, TxStateMeta txStateMeta) {

Review Comment:
   Done, thanks



-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-23488 Integrate cancellation of transaction with sql kill handler [ignite-3]

Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #5207:
URL: https://github.com/apache/ignite-3/pull/5207#discussion_r1950581691


##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -124,19 +110,15 @@ static class TxInfo {
         private final String type;
         private final String priority;
 
-        static TxInfo readOnly(UUID id) {
-            return new TxInfo(id, PENDING, true);
-        }
+        TxInfo(UUID id, TxStateMeta txStateMeta) {
+            InternalTransaction tx = txStateMeta.tx();
 
-        static TxInfo readWrite(UUID id, TxState txState) {
-            return new TxInfo(id, txState, false);
-        }
+            assert tx != null;
 
-        private TxInfo(UUID id, @Nullable TxState state, boolean readOnly) {
             this.id = id.toString();
-            this.state = state == null ? null : state.name();
+            this.state = txStateMeta.txState().name();

Review Comment:
   Done, thanks



-- 
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-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-23488 Integrate cancellation of transaction with sql kill handler [ignite-3]

Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #5207:
URL: https://github.com/apache/ignite-3/pull/5207#discussion_r1950586449


##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -93,27 +86,20 @@ public SystemView<?> get() {
     static class TxInfoDataSource implements Iterable<TxInfo> {
         private final UUID localNodeId;
 
-        private final Iterable<UUID> roTxIds;
-
         private final Map<UUID, TxStateMeta> rwTxStates;
 
-        TxInfoDataSource(UUID localNodeId, Iterable<UUID> roTxIds, Map<UUID, TxStateMeta> rwTxStates) {
+        TxInfoDataSource(UUID localNodeId, Map<UUID, TxStateMeta> rwTxStates) {
             this.localNodeId = localNodeId;
-            this.roTxIds = roTxIds;
             this.rwTxStates = rwTxStates;
         }
 
         @Override
         public Iterator<TxInfo> iterator() {
-            return CollectionUtils.concat(
-                    new TransformingIterator<>(roTxIds.iterator(), TxInfo::readOnly),
-                    rwTxStates.entrySet().stream()
+            return rwTxStates.entrySet().stream()
                             .filter(e -> localNodeId.equals(e.getValue().txCoordinatorId())
-                                    && e.getValue().tx() != null && !e.getValue().tx().isReadOnly()
-                                    && !isFinalState(e.getValue().txState()))
-                            .map(e -> TxInfo.readWrite(e.getKey(), e.getValue().txState()))
-                            .iterator()
-            );
+                                    && e.getValue().tx() != null && !e.getValue().tx().isFinishingOrFinished())

Review Comment:
   Thanks, this check has been changed because RO tx currently does not change state after completion in the state map (it always has state PENDING).
   
   I reworked this code a bit to do separate checks for RW and RO tx, and added a comment about the RO state.



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/views/TransactionsViewProvider.java:
##########
@@ -93,27 +86,20 @@ public SystemView<?> get() {
     static class TxInfoDataSource implements Iterable<TxInfo> {
         private final UUID localNodeId;
 
-        private final Iterable<UUID> roTxIds;
-
         private final Map<UUID, TxStateMeta> rwTxStates;

Review Comment:
   Done, thanks



-- 
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-unsubscribe@ignite.apache.org

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