rpuch commented on code in PR #5588:
URL: https://github.com/apache/ignite-3/pull/5588#discussion_r2044212878
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItZoneDataReplicationTest.java:
##########
@@ -314,56 +335,61 @@ void
testTableDropInTheMiddleOfRebalanceOnIncomingSide(@InjectExecutorService Ex
truncateLogOnEveryNode(partitionId);
+ dropTable(node.catalogManager, DEFAULT_SCHEMA_NAME, TEST_TABLE_NAME1);
+
// Block outgoing snapshot so that we have time to register mocks on
the receiving side.
startBlockOutgoingSnapshot(node, partitionId);
- Node newNode = addNodeToCluster();
+ Node receiverNode = addNodeToCluster();
+
+ // Wait for the table to start on the receiving side.
+ assertTrue(waitForCondition(() -> tableStorage(receiverNode, tableId)
!= null, 10_000));
- assertTrue(waitForCondition(() -> tableStorage(newNode, tableId) !=
null, 10_000));
+ MvTableStorage receivingTableStorage = tableStorage(receiverNode,
tableId);
- MvTableStorage tableStorage = tableStorage(newNode, tableId);
+ // Future that gets completed as soon as
"receivingTableStorage.startRebalancePartition" has been called.
+ var starRebalanceInvokedFuture = new CompletableFuture<Void>();
Review Comment:
'star' or 'start'?
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItZoneDataReplicationTest.java:
##########
@@ -314,56 +335,61 @@ void
testTableDropInTheMiddleOfRebalanceOnIncomingSide(@InjectExecutorService Ex
truncateLogOnEveryNode(partitionId);
+ dropTable(node.catalogManager, DEFAULT_SCHEMA_NAME, TEST_TABLE_NAME1);
+
// Block outgoing snapshot so that we have time to register mocks on
the receiving side.
startBlockOutgoingSnapshot(node, partitionId);
- Node newNode = addNodeToCluster();
+ Node receiverNode = addNodeToCluster();
+
+ // Wait for the table to start on the receiving side.
+ assertTrue(waitForCondition(() -> tableStorage(receiverNode, tableId)
!= null, 10_000));
- assertTrue(waitForCondition(() -> tableStorage(newNode, tableId) !=
null, 10_000));
+ MvTableStorage receivingTableStorage = tableStorage(receiverNode,
tableId);
- MvTableStorage tableStorage = tableStorage(newNode, tableId);
+ // Future that gets completed as soon as
"receivingTableStorage.startRebalancePartition" has been called.
+ var starRebalanceInvokedFuture = new CompletableFuture<Void>();
- var firstConditionReachedFuture = new CompletableFuture<Void>();
- var secondConditionReachedFuture = new CompletableFuture<Void>();
+ // Future that is actually block the execution of
"receivingTableStorage.startRebalancePartition".
Review Comment:
'that is actually block' - is everything ok here?
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItZoneDataReplicationTest.java:
##########
@@ -245,7 +244,13 @@ void testLocalRaftLogReapplication() throws Exception {
* </ol>
*/
@Test
- void
testTableDropInTheMiddleOfRebalanceOnOutgoingSide(@InjectExecutorService
ExecutorService executorService) throws Exception {
+ void
testTableDropInTheMiddleOfRebalanceOnSendingSide(@InjectExecutorService
ExecutorService executorService) throws Exception {
+ // Disable automatic low watermark updates.
+ CompletableFuture<Void> cfgChangeFuture =
gcConfiguration.lowWatermark()
+ .change(lowWatermarkChange ->
lowWatermarkChange.changeUpdateIntervalMillis(Long.MAX_VALUE));
Review Comment:
Would it make sense to extract this to a method? I think it should be used
at least twice. Having a method, the comment would become obsolete
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/snapshot/incoming/IncomingSnapshotCopier.java:
##########
@@ -133,12 +137,13 @@ public class IncomingSnapshotCopier extends
SnapshotCopier {
public IncomingSnapshotCopier(
PartitionSnapshotStorage partitionSnapshotStorage,
SnapshotUri snapshotUri,
- Executor executor,
+ ExecutorService executor,
Review Comment:
You changed `Executor` to `ExecutorService` because
`IgniteThrottledLoggerImpl` wants the latter, but in reality `Executor` is
enough in the logger. I suggest to change `ExecutorService` to `Executor` in
the logger and revert this part of the changes.
--
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: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
