tkalkirill commented on code in PR #6033:
URL: https://github.com/apache/ignite-3/pull/6033#discussion_r2149568986
##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceTest.java:
##########
@@ -146,6 +149,34 @@ void assignmentsChangingOnNodeLeaveNodeJoin() throws
Exception {
);
}
+ @Test
+ @WithSystemProperty(key = THREAD_ASSERTIONS_ENABLED, value = "false")
+ public void compactionAfterAbortedRebalance() {
+ createZone("TEST_ZONE", 1, 1);
+ createTestTable("TEST_TABLE", "TEST_ZONE");
+
+ IgniteImpl igniteImpl = igniteImpl(0);
+
+ TableViewInternal table =
unwrapTableViewInternal(igniteImpl.tables().table("TEST_TABLE"));
+
+ MvTableStorage storage = table.internalTable().storage();
+
+ table.keyValueView().put(
+ null,
+ Tuple.create().set("id", 1),
+ Tuple.create().set("val", "value1")
+ );
+
+ CompletableFuture<CompletableFuture<Void>> abortRebalance =
storage.startRebalancePartition(0)
Review Comment:
According to the documentation, if the rebalance is not in progress, then
nothing will happen. Let's say after the rebalance starts, some internal
component will finish it for some reason. What do you expect after that?
I suggest at least writing a comment about it.
##########
modules/storage-page-memory/src/main/java/org/apache/ignite/internal/storage/pagememory/PersistentPageMemoryTableStorage.java:
##########
@@ -353,13 +356,40 @@ CompletableFuture<Void>
clearStorageAndUpdateDataStructures(AbstractPageMemoryMv
}
private CompletableFuture<Void>
destroyPartitionPhysically(GroupPartitionId groupPartitionId) {
-
dataRegion.filePageStoreManager().getStore(groupPartitionId).markToDestroy();
+ FilePageStore store =
dataRegion.filePageStoreManager().getStore(groupPartitionId);
- dataRegion.pageMemory().invalidate(groupPartitionId.getGroupId(),
groupPartitionId.getPartitionId());
+ assert store != null : groupPartitionId;
- return
dataRegion.checkpointManager().onPartitionDestruction(groupPartitionId)
- .thenAccept(unused ->
dataRegion.partitionMetaManager().removeMeta(groupPartitionId))
- .thenCompose(unused ->
dataRegion.filePageStoreManager().destroyPartition(groupPartitionId));
+ store.markToDestroy();
+
+ CompletableFuture<Void> future = new CompletableFuture<>();
+
+ CheckpointManager checkpointManager = dataRegion.checkpointManager();
+
+ CheckpointListener listener = new CheckpointListener() {
+ @Override
+ public void afterCheckpointEnd(CheckpointProgress progress) {
+ checkpointManager.removeCheckpointListener(this);
+
+ try {
+
dataRegion.pageMemory().invalidate(groupPartitionId.getGroupId(),
groupPartitionId.getPartitionId());
+
+
dataRegion.partitionMetaManager().removeMeta(groupPartitionId);
+
+ future.complete(null);
+ } catch (Exception e) {
+ future.completeExceptionally(new
StorageException("Couldn't destroy partition: " + groupPartitionId, e));
+ }
+ }
+ };
+
+ checkpointManager.addCheckpointListener(listener, dataRegion);
+
+ CheckpointProgress checkpoint =
dataRegion.checkpointManager().forceCheckpoint("Partition destruction");
Review Comment:
Will there be problems here, for example, when rebalancing multiple
partitions? as a result of which we will initiate many checkpoints, which will
be unnecessary and can cause node degradation?
##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceTest.java:
##########
@@ -146,6 +149,34 @@ void assignmentsChangingOnNodeLeaveNodeJoin() throws
Exception {
);
}
+ @Test
+ @WithSystemProperty(key = THREAD_ASSERTIONS_ENABLED, value = "false")
+ public void compactionAfterAbortedRebalance() {
+ createZone("TEST_ZONE", 1, 1);
+ createTestTable("TEST_TABLE", "TEST_ZONE");
+
+ IgniteImpl igniteImpl = igniteImpl(0);
+
+ TableViewInternal table =
unwrapTableViewInternal(igniteImpl.tables().table("TEST_TABLE"));
+
+ MvTableStorage storage = table.internalTable().storage();
+
+ table.keyValueView().put(
+ null,
+ Tuple.create().set("id", 1),
+ Tuple.create().set("val", "value1")
+ );
+
+ CompletableFuture<CompletableFuture<Void>> abortRebalance =
storage.startRebalancePartition(0)
+ .thenApply(v -> storage.abortRebalancePartition(0));
+
+ assertThat(abortRebalance, willCompleteSuccessfully());
+
+ CompletableFuture<Void> flush = storage.getMvPartition(0).flush(true);
+
+ assertThat(flush, willCompleteSuccessfully());
Review Comment:
I think that inline of future are enough here.
According to the name of the test, you want to check compaction, but you
only check flash, I think something in the test is not finished yet.
##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/rebalance/ItRebalanceTest.java:
##########
@@ -146,6 +149,34 @@ void assignmentsChangingOnNodeLeaveNodeJoin() throws
Exception {
);
}
+ @Test
+ @WithSystemProperty(key = THREAD_ASSERTIONS_ENABLED, value = "false")
+ public void compactionAfterAbortedRebalance() {
+ createZone("TEST_ZONE", 1, 1);
Review Comment:
Maybe we can move the names of zones and tables to constants?
--
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]
