denis-chudov commented on code in PR #7052:
URL: https://github.com/apache/ignite-3/pull/7052#discussion_r2577041930
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -1209,6 +1210,19 @@ public void onReceived(NetworkMessage message,
InternalClusterNode sender, @Null
// Ignore error responses here. A transaction will be rolled back in
other place.
Review Comment:
```suggestion
// Only removing the failed inflight here. A transaction will be
rolled back in other place.
```
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionInflights.java:
##########
@@ -190,6 +208,22 @@ public boolean hasActiveInflights() {
return false;
}
+ /**
+ * Return true if at least on inflight is failed.
+ *
+ * @return {@code True} if has failed inflights.
+ */
+ @TestOnly
+ public boolean hasFailedInflight() {
Review Comment:
It's never used
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionInflights.java:
##########
@@ -170,7 +170,25 @@ public void removeInflight(UUID txId) {
// Avoid completion under lock.
if (tuple != null) {
- tuple.onInflightsRemoved();
+ tuple.onInflightsRemoved(tuple.err);
+ }
+ }
+
+ void removeInflight(UUID txId, Throwable cause) {
+ // Can be null if tx was aborted and inflights were removed from the
collection.
+ TxContext tuple = txCtxMap.computeIfPresent(txId, (uuid, ctx) -> {
+ ctx.removeInflight(txId);
+
+ if (cause != null && ctx.err == null) {
+ ctx.err = cause; // Retain only first exception.
Review Comment:
Why not unify with overloaded method? You check the cause for `null` anyway
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionInflights.java:
##########
@@ -418,9 +475,13 @@ void cancelWaitingInflights(ReplicationGroupId groupId,
long enlistmentConsisten
}
@Override
- public void onInflightsRemoved() {
+ void onInflightsRemoved(@Nullable Throwable t) {
Review Comment:
Let's rename to `onInflightRemoved`
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -1209,6 +1210,19 @@ public void onReceived(NetworkMessage message,
InternalClusterNode sender, @Null
// Ignore error responses here. A transaction will be rolled back in
other place.
if (message instanceof ErrorReplicaResponse) {
+ ErrorReplicaResponse response = (ErrorReplicaResponse) message;
+
+ Throwable err = response.throwable();
+
+ Throwable cause = ExceptionUtils.unwrapCause(err);
+
+ if (cause instanceof DelayedAckException) {
Review Comment:
Shouldn't we throw MismatchingTransactionOutcomeInternalException in case of
this exception?
If so, you can make corresponding changes in
`TransactionInflights#completeFinishInProgressFuture`
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionInflights.java:
##########
@@ -251,6 +285,7 @@ ReadWriteTxContext lockTxForNewUpdates(UUID txId,
Map<ReplicationGroupId, Pendin
abstract static class TxContext {
volatile long inflights = 0; // Updated under lock.
+ Throwable err;
Review Comment:
```suggestion
volatile Throwable err;
```
--
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]
