sanpwc commented on code in PR #3436:
URL: https://github.com/apache/ignite-3/pull/3436#discussion_r1531615487
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TransactionInflights.java:
##########
@@ -99,6 +99,15 @@ public void removeInflight(UUID txId) {
tuple.onInflightsRemoved();
}
+ /**
+ * Cleanup tx inflights on tx finish.
+ *
+ * @param txId Transaction id.
+ */
+ void clearInflights(UUID txId) {
Review Comment:
First of all let's be consistent with naming, few lines below there is a
`void removeTxContexts(Collection<UUID> txIds)` - let's rename it to
removeInflight, please also rename newly proposed methods to removeInflights.
Generally, naming is confusing. TransactionInflights holds the map of
TxContext which is semantically wider than inflights.
And even more, we treat RO and RW inflights differently. They have different
lifecycle, ones closed on tx finish, others on broadcasting
FinishedTransactionsBatchMessage. Within last option we actually do not use the
context only the txId itself.
All in all that means that design is inconsistent and should be refactored.
Let's discuss this on tx sync.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -1746,7 +1745,7 @@ private CompletableFuture<FuturesCleanupResult>
awaitCleanupReadyFutures(UUID tx
txOps.futures.clear();
- return txOps;
+ return null;
Review Comment:
Do we use cleanupReadyFutures in order to handle the race between cleanup
and adding new operations? Meaning that:
- we need to await all pending operations prior to cleanup processing - all
good here.
- we need to discard new operation after cleanup was done - do we use
cleanupReadyFutures to handle such race?
--
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]
