sashapolo commented on code in PR #6072:
URL: https://github.com/apache/ignite-3/pull/6072#discussion_r2159988418
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/PartitionCommandListenerTest.java:
##########
@@ -404,6 +414,57 @@ public void testInsertRowsBatchedAndCheck() {
readAndCheck(false);
}
+ @Test
+ @WithSystemProperty(key = COLOCATION_FEATURE_FLAG, value = "false")
Review Comment:
Why do we need this annotation? How does the colocation flag affect this
test?
##########
modules/table/src/test/java/org/apache/ignite/internal/table/distributed/raft/PartitionCommandListenerTest.java:
##########
@@ -568,6 +629,26 @@ void updatesLastAppliedForUpdateAllCommands() {
verify(mvPartitionStorage).lastApplied(3, 2);
}
+ @Test
+ @WithSystemProperty(key = COLOCATION_FEATURE_FLAG, value = "false")
Review Comment:
Same here
##########
modules/partition-replicator/src/test/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManagerTest.java:
##########
@@ -324,4 +378,55 @@ void producesEventsOnPartitionDestroy() {
assertThat(beforeReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
assertThat(afterReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
}
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenAllComponentsAreStoppedFine() throws
Exception {
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ verify(replicaManager,
times(CatalogUtils.DEFAULT_PARTITION_COUNT)).stopReplica(any());
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenReplicasAreStoppedExceptionally()
throws Exception {
+ when(replicaManager.stopReplica(any())).thenThrow(new
NodeStoppingException());
+
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ // One more because of exceptional mock above counts.
+ verify(replicaManager, times(CatalogUtils.DEFAULT_PARTITION_COUNT +
1)).stopReplica(any());
+
+ // Do reset for correct replica manager stop on tear down.
+ reset(replicaManager);
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenTxStatePartitionStoragesAreStoppedExceptionally()
throws Exception {
+
doReturn(commonZonePartitionResources).when(zoneResourcesManager).getZonePartitionResources(any());
+
+ int defaultZoneId = 0;
Review Comment:
please don't hardcode zone ids, use the catalog
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/ZoneResourcesManager.java:
##########
@@ -67,7 +67,7 @@ public class ZoneResourcesManager implements
ManuallyCloseable {
private final Executor partitionOperationsExecutor;
/** Map from zone IDs to their resource holders. */
- private final Map<Integer, ZoneResources> resourcesByZoneId = new
ConcurrentHashMap<>();
+ private final Map<Integer, ZoneResources> resourcesByZoneId = new
ConcurrentHashMap<>();
Review Comment:
?
##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/storage/state/rocksdb/TxStateRocksDbStorage.java:
##########
@@ -92,7 +92,7 @@ public TxStatePartitionStorage
getOrCreatePartitionStorage(int partitionId) {
TxStateRocksDbPartitionStorage storage = storages.get(partitionId);
if (storage == null) {
- storage = new TxStateRocksDbPartitionStorage(partitionId, this);
+ storage = (TxStateRocksDbPartitionStorage)
createPartitionStorage(partitionId);
Review Comment:
This is a bad design. We need to use this cast only because of tests?
##########
modules/partition-replicator/src/test/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManagerTest.java:
##########
@@ -324,4 +378,55 @@ void producesEventsOnPartitionDestroy() {
assertThat(beforeReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
assertThat(afterReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
}
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenAllComponentsAreStoppedFine() throws
Exception {
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ verify(replicaManager,
times(CatalogUtils.DEFAULT_PARTITION_COUNT)).stopReplica(any());
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenReplicasAreStoppedExceptionally()
throws Exception {
+ when(replicaManager.stopReplica(any())).thenThrow(new
NodeStoppingException());
+
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ // One more because of exceptional mock above counts.
Review Comment:
What does `exceptional mock above counts` mean?
##########
modules/partition-replicator/src/test/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManagerTest.java:
##########
@@ -324,4 +378,55 @@ void producesEventsOnPartitionDestroy() {
assertThat(beforeReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
assertThat(afterReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
}
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenAllComponentsAreStoppedFine() throws
Exception {
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ verify(replicaManager,
times(CatalogUtils.DEFAULT_PARTITION_COUNT)).stopReplica(any());
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenReplicasAreStoppedExceptionally()
throws Exception {
+ when(replicaManager.stopReplica(any())).thenThrow(new
NodeStoppingException());
+
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ // One more because of exceptional mock above counts.
+ verify(replicaManager, times(CatalogUtils.DEFAULT_PARTITION_COUNT +
1)).stopReplica(any());
+
+ // Do reset for correct replica manager stop on tear down.
+ reset(replicaManager);
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenTxStatePartitionStoragesAreStoppedExceptionally()
throws Exception {
Review Comment:
Can you please explain what is the test scenario here?
##########
modules/partition-replicator/src/test/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManagerTest.java:
##########
@@ -324,4 +378,55 @@ void producesEventsOnPartitionDestroy() {
assertThat(beforeReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
assertThat(afterReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
}
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenAllComponentsAreStoppedFine() throws
Exception {
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ verify(replicaManager,
times(CatalogUtils.DEFAULT_PARTITION_COUNT)).stopReplica(any());
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenReplicasAreStoppedExceptionally()
throws Exception {
+ when(replicaManager.stopReplica(any())).thenThrow(new
NodeStoppingException());
+
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ // One more because of exceptional mock above counts.
+ verify(replicaManager, times(CatalogUtils.DEFAULT_PARTITION_COUNT +
1)).stopReplica(any());
+
+ // Do reset for correct replica manager stop on tear down.
+ reset(replicaManager);
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenTxStatePartitionStoragesAreStoppedExceptionally()
throws Exception {
+
doReturn(commonZonePartitionResources).when(zoneResourcesManager).getZonePartitionResources(any());
+
+ int defaultZoneId = 0;
+ List<ZonePartitionResources> defaultZoneResources = IntStream.range(0,
CatalogUtils.DEFAULT_PARTITION_COUNT)
+ .mapToObj(partId -> new ZonePartitionId(defaultZoneId, partId))
+ .map(partitionReplicaLifecycleManager::zonePartitionResources)
+ .collect(Collectors.toList());
+
+ defaultZoneResources.forEach(resources -> {
+ TxStatePartitionStorage txStorage =
resources.txStatePartitionStorage();
+
+ doThrow(new RuntimeException()).when(txStorage).close();
+ });
+
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willThrow(RuntimeException.class));
+
+ verify(replicaManager,
times(CatalogUtils.DEFAULT_PARTITION_COUNT)).stopReplica(any());
+
+ defaultZoneResources.forEach(resources ->
verify(resources.txStatePartitionStorage(), atLeastOnce()).close());
+
+ defaultZoneResources.forEach(resources ->
reset(resources.txStatePartitionStorage()));
Review Comment:
What is this about?
##########
modules/partition-replicator/src/test/java/org/apache/ignite/internal/partition/replicator/PartitionReplicaLifecycleManagerTest.java:
##########
@@ -324,4 +378,55 @@ void producesEventsOnPartitionDestroy() {
assertThat(beforeReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
assertThat(afterReplicaDestroyedFuture.thenApply(LocalPartitionReplicaEventParameters::zonePartitionId),
willBe(zonePartitionId));
}
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenAllComponentsAreStoppedFine() throws
Exception {
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ verify(replicaManager,
times(CatalogUtils.DEFAULT_PARTITION_COUNT)).stopReplica(any());
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenReplicasAreStoppedExceptionally()
throws Exception {
+ when(replicaManager.stopReplica(any())).thenThrow(new
NodeStoppingException());
+
+ assertDoesNotThrow(() ->
partitionReplicaLifecycleManager.beforeNodeStop());
+
+ assertThat(partitionReplicaLifecycleManager.stopAsync(),
willCompleteSuccessfully());
+
+ // One more because of exceptional mock above counts.
+ verify(replicaManager, times(CatalogUtils.DEFAULT_PARTITION_COUNT +
1)).stopReplica(any());
+
+ // Do reset for correct replica manager stop on tear down.
+ reset(replicaManager);
+ }
+
+ @Test
+ public void
partitionLifecycleManagerStopsCorrectWhenTxStatePartitionStoragesAreStoppedExceptionally()
throws Exception {
+
doReturn(commonZonePartitionResources).when(zoneResourcesManager).getZonePartitionResources(any());
+
+ int defaultZoneId = 0;
+ List<ZonePartitionResources> defaultZoneResources = IntStream.range(0,
CatalogUtils.DEFAULT_PARTITION_COUNT)
+ .mapToObj(partId -> new ZonePartitionId(defaultZoneId, partId))
+ .map(partitionReplicaLifecycleManager::zonePartitionResources)
+ .collect(Collectors.toList());
+
+ defaultZoneResources.forEach(resources -> {
+ TxStatePartitionStorage txStorage =
resources.txStatePartitionStorage();
+
+ doThrow(new RuntimeException()).when(txStorage).close();
Review Comment:
So, you need all these machinations with the TX storages only to throw an
exception on close? `ZoneResourcesManager` is already a spy here, can you
simply throw an exception from its `close` method?
--
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
