sanpwc commented on code in PR #5213:
URL: https://github.com/apache/ignite-3/pull/5213#discussion_r1952469406
##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/table/ItTableScanTest.java:
##########
@@ -1044,6 +1044,7 @@ private InternalTransaction
startTxWithEnlistedPartition(int partId, boolean rea
tx.enlist(
tblPartId,
+ tblPartId.tableId(),
Review Comment:
We may miss that.
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/TxFinishReplicaRequestHandler.java:
##########
@@ -146,18 +145,18 @@ public CompletableFuture<TransactionResult>
handle(TxFinishReplicaRequest reques
}
}
- private static Map<TablePartitionId, String>
asTablePartitionIdStringMap(Map<TablePartitionIdMessage, String> messages) {
- var result = new HashMap<TablePartitionId,
String>(IgniteUtils.capacity(messages.size()));
+ private static Map<ReplicationGroupId, String>
asReplicationGroupIdToStringMap(Map<ReplicationGroupIdMessage, String>
messages) {
+ var result = new HashMap<ReplicationGroupId,
String>(IgniteUtils.capacity(messages.size()));
- for (Entry<TablePartitionIdMessage, String> e : messages.entrySet()) {
- result.put(e.getKey().asTablePartitionId(), e.getValue());
+ for (Entry<ReplicationGroupIdMessage, String> e : messages.entrySet())
{
+ result.put(e.getKey().asReplicationGroupId(), e.getValue());
}
return result;
}
private CompletableFuture<TransactionResult> finishAndCleanup(
- Map<TablePartitionId, String> enlistedPartitions,
+ Map<ReplicationGroupId, String> enlistedPartitions,
Review Comment:
Are you going to add tableIds here while implementing [IGNITE-24384]? I mean
that in order to switch writeIntents you will need to know which tables to
touch.
##########
modules/partition-replicator/src/test/java/org/apache/ignite/internal/partition/replicator/schemacompat/SchemaCompatibilityValidatorTest.java:
##########
@@ -120,7 +120,11 @@ private void
assertForwardCompatibleChangeAllowsCommitting(SchemaChangeSource ch
when(schemasSource.tableSchemaVersionsBetween(TABLE_ID,
beginTimestamp, commitTimestamp))
.thenReturn(changeSource.schemaVersions());
- CompletableFuture<CompatValidationResult> resultFuture =
validator.validateCommit(txId, List.of(tablePartitionId), commitTimestamp);
+ CompletableFuture<CompatValidationResult> resultFuture =
validator.validateCommit(
Review Comment:
We may miss that.
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -1878,6 +1880,7 @@ private CompletableFuture<?> beginAndCommitTx() {
.commitPartitionId(tablePartitionIdMessage(grpId))
.txId(txId)
.groups(Map.of(tablePartitionIdMessage(grpId),
localNode.name()))
Review Comment:
And that.
##########
modules/table/src/integrationTest/java/org/apache/ignite/distributed/ItInternalTableReadWriteScanTest.java:
##########
@@ -82,7 +82,7 @@ protected InternalTransaction startTx() {
ClusterNode primaryReplicaNode = getPrimaryReplica(tblPartId);
- tx.enlist(tblPartId, new IgniteBiTuple<>(primaryReplicaNode, term));
+ tx.enlist(tblPartId, internalTbl.tableId(), new
IgniteBiTuple<>(primaryReplicaNode, term));
Review Comment:
We may miss that.
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -1813,6 +1814,7 @@ private CompletableFuture<?> beginAndAbortTx() {
.commitPartitionId(tablePartitionIdMessage(grpId))
.txId(txId)
.groups(Map.of(tablePartitionIdMessage(grpId),
localNode.name()))
Review Comment:
We may miss that.
##########
modules/table/src/integrationTest/java/org/apache/ignite/internal/table/ItColocationTest.java:
##########
@@ -220,7 +220,8 @@ public CompletableFuture<Void> finish(
HybridTimestampTracker observableTimestampTracker,
TablePartitionId commitPartition,
boolean commitIntent,
- Map<TablePartitionId, IgniteBiTuple<ClusterNode, Long>>
enlistedGroups,
+ Map<ReplicationGroupId, IgniteBiTuple<ClusterNode, Long>>
enlistedGroups,
Review Comment:
We may miss that. Meaning that all(?) finish users currently propagate
TablePartitionId's.
##########
modules/transactions/src/test/java/org/apache/ignite/internal/tx/impl/ReadWriteTransactionImplTest.java:
##########
@@ -120,8 +120,8 @@ private void startTxAndTryToEnlist(boolean commit) {
tx.assignCommitPartition(TX_COMMIT_PART);
- tx.enlist(new TablePartitionId(TABLE_ID, 0), NODE_AND_TOKEN);
- tx.enlist(new TablePartitionId(TABLE_ID, 2), NODE_AND_TOKEN);
+ tx.enlist(new TablePartitionId(TABLE_ID, 0), TABLE_ID, NODE_AND_TOKEN);
Review Comment:
We may miss that.
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/TxManager.java:
##########
@@ -182,7 +185,7 @@ CompletableFuture<Void> finish(
*/
CompletableFuture<Void> cleanup(
ReplicationGroupId commitPartitionId,
- Map<TablePartitionId, String> enlistedPartitions,
+ Map<ReplicationGroupId, String> enlistedPartitions,
Review Comment:
As mentioned above, we will also need tableIds here, because of
WriteIntentSwitch logic. I guess that you are going to add it in [IGNITE-24384].
##########
modules/transactions/src/test/java/org/apache/ignite/internal/tx/TxManagerTest.java:
##########
@@ -230,7 +230,7 @@ public void testEnlist() {
TablePartitionId tablePartitionId = new TablePartitionId(1, 0);
- tx.enlist(tablePartitionId, new IgniteBiTuple<>(REMOTE_NODE, 1L));
+ tx.enlist(tablePartitionId, tablePartitionId.tableId(), new
IgniteBiTuple<>(REMOTE_NODE, 1L));
Review Comment:
We may miss that.
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/replication/PartitionReplicaListenerTest.java:
##########
@@ -2651,6 +2654,7 @@ private void testCommitRequestIfTableWasDropped(
.groupId(tablePartitionIdMessage(commitPartitionId))
.commitPartitionId(tablePartitionIdMessage(commitPartitionId))
.groups(groups.entrySet().stream().collect(toMap(e ->
tablePartitionIdMessage(e.getKey()), Map.Entry::getValue)))
Review Comment:
And that.
--
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]
