vldpyatkov commented on code in PR #2585:
URL: https://github.com/apache/ignite-3/pull/2585#discussion_r1326216219
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -480,7 +480,15 @@ public TableManager(
clusterNodeResolver = topologyService::getByConsistentId;
- transactionStateResolver = new TransactionStateResolver(replicaSvc,
clusterNodeResolver);
+ transactionStateResolver = new TransactionStateResolver(
+ replicaSvc,
+ txManager,
+ clock,
+ clusterNodeResolver,
+ topologyService::getById,
+ () -> localNode().id(),
Review Comment:
Are you sure this closure is required here? Use a constant if possible.
##########
modules/network/src/main/java/org/apache/ignite/network/scalecube/ScaleCubeTopologyService.java:
##########
@@ -59,6 +59,9 @@ final class ScaleCubeTopologyService extends
AbstractTopologyService {
/** Topology members map from the consistent id to the cluster node. */
private final ConcurrentMap<String, ClusterNode> consistentIdToMemberMap =
new ConcurrentHashMap<>();
+ /** Topology members map from the non-consistent id to the cluster node. */
+ private final ConcurrentMap<String, ClusterNode> idToMemberMap = new
ConcurrentHashMap<>();
Review Comment:
Why don't you scan consistentIdToMemberMap instead of creating the new
collection?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -1229,22 +1226,22 @@ private CompletableFuture<Void> finishAndCleanup(
* @param aggregatedGroupIds Partition identifies which are enlisted in
the transaction.
* @param txId Transaction id.
* @param commit True is the transaction is committed, false otherwise.
+ * @param commitTimestamp Commit timestamp, if applicable.
* @param txCoordinatorId Transaction coordinator id.
* @return Future to wait of the finish.
*/
private CompletableFuture<Object> finishTransaction(
List<TablePartitionId> aggregatedGroupIds,
UUID txId,
boolean commit,
+ @Nullable HybridTimestamp commitTimestamp,
Review Comment:
Why can the timestamp be null?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -2527,34 +2524,33 @@ private CompletableFuture<BinaryRow>
resolveWriteIntentAsync(
/**
* Resolve the actual tx state.
*
- * @param commitGrpId Commit partition id.
* @param txId Transaction id.
+ * @param commitGrpId Commit partition id.
* @param timestamp Timestamp.
* @return The future completes with true when the transaction is not
completed yet and false otherwise.
*/
private CompletableFuture<Boolean> resolveTxState(
- TablePartitionId commitGrpId,
UUID txId,
+ TablePartitionId commitGrpId,
HybridTimestamp timestamp
) {
- boolean readLatest = timestamp == null;
-
- return transactionStateResolver.sendMetaRequest(commitGrpId,
FACTORY.txStateReplicaRequest()
- .groupId(commitGrpId)
- .readTimestampLong((readLatest ?
HybridTimestamp.MIN_VALUE : timestamp).longValue())
- .txId(txId)
- .build())
- .thenApply(txMeta -> {
- if (txMeta == null) {
- return true;
- } else if (txMeta.txState() == COMMITED) {
- return !readLatest &&
txMeta.commitTimestamp().compareTo(timestamp) > 0;
- } else {
- assert txMeta.txState() == ABORTED : "Unexpected
transaction state [state=" + txMeta.txState() + ']';
+ return transactionStateResolver.resolveTxState(txId, commitGrpId,
timestamp).thenApply(txMeta -> {
+ if (txMeta == null) {
+ return true;
Review Comment:
I don't think we can answer so easily if the transaction state has not been
resolved.
--
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]
