denis-chudov commented on code in PR #7482:
URL: https://github.com/apache/ignite-3/pull/7482#discussion_r2789402607
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/InternalTransaction.java:
##########
@@ -164,6 +164,15 @@ default boolean remote() {
*/
CompletableFuture<Void> rollbackTimeoutExceededAsync();
+ /**
+ * Rolls back the transaction due to a non-user exception and records the
abort reason.
+ * A rollback of a completed or ending transaction has no effect and
always succeeds when the transaction is completed.
+ *
+ * @param throwable Abort reason.
+ * @return The future.
+ */
+ CompletableFuture<Void> rollbackWithExceptionAsync(Throwable throwable);
Review Comment:
Can we get rid of `rollbackTimeoutExceededAsync()` right in this patch? It
is internal API and we shouldn't care about compatibility.
But, if it's complicated - lets better create a ticket and leave TODO.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxStateMeta.java:
##########
@@ -350,6 +450,22 @@ public TxStateMetaBuilder tx(@Nullable InternalTransaction
tx) {
return this;
}
+ /**
+ * Appends exception info to the list, preserving any previously
recorded entries.
+ *
+ * @param exceptionInfo Exception info to add.
+ * @return Builder.
+ */
+ public TxStateMetaBuilder exceptionInfo(@Nullable
TxStateMetaExceptionInfo exceptionInfo) {
+ if (exceptionInfo != null) {
+ if (exceptionInfos == null) {
+ exceptionInfos = new CopyOnWriteArrayList<>();
Review Comment:
I'm not sure you need copy-on-write array list, we update txn metas inside
critical section in `chm.compute()`
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxStateMeta.java:
##########
@@ -191,6 +239,56 @@ public TxState txState() {
return txLabel;
}
+ public @Nullable List<TxStateMetaExceptionInfo> exceptionInfos() {
+ return exceptionInfos;
+ }
+
+ /**
+ * Records exceptional information by mutating tx state or by creating a
new one. This method should be called after tx is finished.
+ *
+ * @param old previous TxStateMeta.
+ * @param throwable to record
+ */
+ public static TxStateMeta recordExceptionInfo(@Nullable TxStateMeta old,
Throwable throwable) {
+ TxStateMetaExceptionInfo exceptionInfo = fromThrowable(throwable);
+ return old == null
+ ? builder(old, ABORTED).exceptionInfo(exceptionInfo).build()
+ : old.mutate().exceptionInfo(exceptionInfo).build();
+ }
+
+ /**
+ * Aggregates exception infos into a single throwable, using the last
exception as the primary one and attaching previous exceptions as
+ * suppressed.
+ *
+ * @param exceptionInfos Exception infos list.
+ * @return Aggregated throwable or {@code null} if nothing usable is
present.
+ */
+ public static @Nullable Throwable aggregateExceptionInfos(@Nullable
List<TxStateMetaExceptionInfo> exceptionInfos) {
+ if (exceptionInfos == null || exceptionInfos.isEmpty()) {
+ return null;
+ }
+
+ Throwable primary = exceptionInfos.get(exceptionInfos.size() -
1).throwable();
Review Comment:
Why the last one is primary?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxStateMeta.java:
##########
@@ -258,6 +356,7 @@ public static class TxStateMetaBuilder {
protected @Nullable Boolean isFinishedDueToTimeout;
protected @Nullable String txLabel;
protected @Nullable InternalTransaction tx;
+ protected @Nullable List<TxStateMetaExceptionInfo> exceptionInfos;
Review Comment:
you can store single exception field here and add other exceptions as
suppressed
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/IgniteAbstractTransactionImpl.java:
##########
@@ -164,4 +169,20 @@ public long getTimeout() {
public boolean isRolledBackWithTimeoutExceeded() {
return timeoutExceeded;
}
+
+ @Override
+ public CompletableFuture<Void> rollbackWithExceptionAsync(Throwable
throwable) {
+ return rollbackAsync().whenComplete((v, t) -> {
+ if (t != null) {
+ // If rollback fails we consider that there is coordinator
problem, but we keep original exception.
+ txManager.updateTxMeta(id,
+ old -> ofNullable(old)
+ .map(txStateMeta ->
txStateMeta.mutate().txState(TxState.ABANDONED)
+
.exceptionInfo(fromThrowable(throwable)).build())
+
.orElse(builder(TxState.ABANDONED).exceptionInfo(fromThrowable(throwable)).build()));
Review Comment:
ABANDONED state means that txn coordinator is absent. Txn rollback shouldn't
set this state to transaction. And this method is called during rollback on
coordinator, which brings even more mess :)
also, duplicating
`(TxState.ABANDONED).exceptionInfo(fromThrowable(throwable)).build()` looks
weird
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxStateMetaExceptionInfo.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.tx;
+
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.ignite.internal.util.ExceptionUtils;
+import org.apache.ignite.lang.TraceableException;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Transaction abort exception information stored in {@link TxStateMeta}.
+ *
+ * <p>This information is stored for cases when a transaction is aborted
exceptionally and the original response to the user
+ * is lost (for example, due to disconnections). The stored information can
later be used to understand the abort reason.
+ */
+public class TxStateMetaExceptionInfo {
+ /** Exception class name. */
+ private final String exceptionClassName;
+
+ /** Full Ignite error code (group + code), see {@link
TraceableException#code()}. */
+ private final int code;
+
+ /** Trace id of the exception. If exception is not traceable will be null.
*/
+ private final @Nullable UUID traceId;
+
+ /** Exception message. */
+ private final @Nullable String message;
+
+ /** Original exception object. */
+ private final @Nullable Throwable throwable;
Review Comment:
This information is already in IgniteException, why do you need one more
wrapper?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxStateMeta.java:
##########
@@ -191,6 +239,56 @@ public TxState txState() {
return txLabel;
}
+ public @Nullable List<TxStateMetaExceptionInfo> exceptionInfos() {
+ return exceptionInfos;
+ }
+
+ /**
+ * Records exceptional information by mutating tx state or by creating a
new one. This method should be called after tx is finished.
+ *
+ * @param old previous TxStateMeta.
+ * @param throwable to record
+ */
+ public static TxStateMeta recordExceptionInfo(@Nullable TxStateMeta old,
Throwable throwable) {
+ TxStateMetaExceptionInfo exceptionInfo = fromThrowable(throwable);
+ return old == null
+ ? builder(old, ABORTED).exceptionInfo(exceptionInfo).build()
+ : old.mutate().exceptionInfo(exceptionInfo).build();
+ }
+
+ /**
+ * Aggregates exception infos into a single throwable, using the last
exception as the primary one and attaching previous exceptions as
+ * suppressed.
+ *
+ * @param exceptionInfos Exception infos list.
+ * @return Aggregated throwable or {@code null} if nothing usable is
present.
+ */
+ public static @Nullable Throwable aggregateExceptionInfos(@Nullable
List<TxStateMetaExceptionInfo> exceptionInfos) {
+ if (exceptionInfos == null || exceptionInfos.isEmpty()) {
+ return null;
+ }
+
+ Throwable primary = exceptionInfos.get(exceptionInfos.size() -
1).throwable();
+ if (primary == null) {
+ return null;
+ }
+
+ if (exceptionInfos.size() == 1) {
+ return primary;
+ }
+
+ RuntimeException aggregate = new RuntimeException("Multiple
transaction abort reasons", primary);
Review Comment:
We only use descendants of IgniteException/IgniteInternalException. And you
can just add other exceptions as suppressed to primary
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/IgniteAbstractTransactionImpl.java:
##########
@@ -164,4 +169,20 @@ public long getTimeout() {
public boolean isRolledBackWithTimeoutExceeded() {
return timeoutExceeded;
}
+
+ @Override
+ public CompletableFuture<Void> rollbackWithExceptionAsync(Throwable
throwable) {
+ return rollbackAsync().whenComplete((v, t) -> {
Review Comment:
It would be more correct to do it using `ReadWriteTransactionImpl#finish`
method where `boolean timeoutExceeded` should be replaced with `exception`.
This flag is used only for updating txn meta, so it can be replaced with
exception safely. After that, you can rework `rollbackTimeoutExceededAsync` to
call `rollbackWithExceptionAsync(new TimeoutException())` or smth like that (or
get rid of `rollbackTimeoutExceededAsync ` at all).
And when you do that, please remove `updateTxMeta` from
`TxManagerImpl#finishFull` - there is no sense in storing txn metas for full
txns.
Also, it should be wrapped using
`TransactionsExceptionMapperUtil.convertToPublicFuture` like it is done in
`rollbackTimeoutExceededAsync`.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxStateMetaExceptionInfo.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.tx;
+
+import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR;
+
+import java.util.Objects;
+import java.util.UUID;
+import org.apache.ignite.internal.util.ExceptionUtils;
+import org.apache.ignite.lang.TraceableException;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Transaction abort exception information stored in {@link TxStateMeta}.
+ *
+ * <p>This information is stored for cases when a transaction is aborted
exceptionally and the original response to the user
+ * is lost (for example, due to disconnections). The stored information can
later be used to understand the abort reason.
+ */
+public class TxStateMetaExceptionInfo {
+ /** Exception class name. */
+ private final String exceptionClassName;
+
+ /** Full Ignite error code (group + code), see {@link
TraceableException#code()}. */
+ private final int code;
+
+ /** Trace id of the exception. If exception is not traceable will be null.
*/
+ private final @Nullable UUID traceId;
+
+ /** Exception message. */
+ private final @Nullable String message;
+
+ /** Original exception object. */
+ private final @Nullable Throwable throwable;
+
+ /**
+ * Constructor.
+ *
+ * @param exceptionClassName Exception class name.
+ * @param code Full error code (group + code).
+ * @param traceId Trace id.
+ * @param message Exception message.
+ */
+ public TxStateMetaExceptionInfo(String exceptionClassName, int code,
@Nullable UUID traceId, @Nullable String message) {
Review Comment:
This one is never used
--
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]
