denis-chudov commented on code in PR #7899:
URL: https://github.com/apache/ignite-3/pull/7899#discussion_r3010526491
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxRecoveryEngine.java:
##########
@@ -127,10 +129,24 @@ public CompletableFuture<TransactionMeta>
triggerTxRecovery(
})
.thenCompose(Function.identity())
.whenComplete((v, ex) -> {
- runCleanupOnNode(commitPartitionId, txId,
commitPartitionNode);
+ // v contains the actual resolved transaction state
(COMMITTED or ABORTED).
+ // We must pass it to cleanup so the correct commit flag
is used.
+ // Previously, runCleanupOnNode always used commit=false
(hardcoded in the 3-param
+ // TxCleanupRequestSender.cleanup), which caused data
corruption when the transaction
+ // was actually COMMITTED: the abort cleanup would race
with the legitimate commit
+ // cleanup, and whichever arrived first at each partition
would win, producing a mix
+ // of committed and aborted rows within the same
transaction.
Review Comment:
I would reword this without "Previously", let's just leave an explanation
"why it looks like this right now"
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -660,7 +660,7 @@ public CompletableFuture<Void> finish(
Map<ZonePartitionId, PendingTxPartitionEnlistment> enlistedGroups,
UUID txId
) {
- LOG.debug("Finish [commit={}, {}, groups={}, commitPartId={}].",
commitIntent,
+ LOG.info("Finish [commit={}, {}, groups={}, commitPartId={}].",
commitIntent,
Review Comment:
please change back to `debug`
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/tx/ClientTransactionCommitRequest.java:
##########
@@ -172,8 +172,11 @@ public static boolean merge(InternalTable table, int
partId, String consistentId
if (existing == null) {
tx.enlist(replicationGroupId, table.tableId(), consistentId,
token);
} else {
+ boolean tokenMatch = existing.consistencyToken() == token;
+ existing.addTableId(table.tableId());
+
// Enlistment tokens should be equal on commit.
- return !commit || existing.consistencyToken() == token;
+ return !commit || tokenMatch;
Review Comment:
on commit, this will always return false and lead to exception, does it
really work?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxManagerImpl.java:
##########
@@ -906,7 +906,7 @@ private CompletableFuture<Void> sendFinishRequest(
HybridTimestamp commitTimestamp,
CompletableFuture<TransactionMeta> txFinishFuture
) {
- LOG.debug("Finish [partition={}, node={},
enlistmentConsistencyToken={}, commit={}, {}, groups={}",
+ LOG.info("Finish [partition={}, node={},
enlistmentConsistencyToken={}, commit={}, {}, groups={}",
Review Comment:
please change back to `debug`
--
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]
