alievmirza commented on code in PR #6902:
URL: https://github.com/apache/ignite-3/pull/6902#discussion_r2498844314
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/exception/DistributionZonesNotFoundException.java:
##########
@@ -15,17 +15,18 @@
* limitations under the License.
*/
-package org.apache.ignite.internal.table.distributed.disaster.exceptions;
+package org.apache.ignite.internal.distributionzones.exception;
import static
org.apache.ignite.lang.ErrorGroups.DistributionZones.ZONE_NOT_FOUND_ERR;
-import java.util.Set;
+import java.util.Collection;
+import org.apache.ignite.internal.lang.IgniteInternalException;
/** Exception is thrown when appropriate zones can`t be found. */
-public class ZonesNotFoundException extends DisasterRecoveryException {
+public class DistributionZonesNotFoundException extends
IgniteInternalException {
private static final long serialVersionUID = -8475588176132321568L;
- public ZonesNotFoundException(Set<String> missingZoneNames) {
+ public DistributionZonesNotFoundException(Collection<String>
missingZoneNames) {
super(ZONE_NOT_FOUND_ERR, "Some distribution zones are missing: " +
missingZoneNames);
Review Comment:
Now we have `DistributionZoneNotFoundException` and
`DistributionZonesNotFoundException`, but they both use `ZONE_NOT_FOUND_ERR`,
looks inconsistent
##########
modules/rest/src/integrationTest/java/org/apache/ignite/internal/rest/cluster/ItDataNodesControllerTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.rest.cluster;
+
+import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
+import static
org.apache.ignite.internal.distributionzones.DataNodesTestUtil.createZoneWithInfiniteTimers;
+import static
org.apache.ignite.internal.distributionzones.DataNodesTestUtil.waitForDataNodes;
+import static
org.apache.ignite.internal.testframework.IgniteTestUtils.waitForCondition;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.micronaut.http.HttpRequest;
+import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpStatus;
+import io.micronaut.http.client.HttpClient;
+import io.micronaut.http.client.annotation.Client;
+import io.micronaut.http.client.exceptions.HttpClientResponseException;
+import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
+import jakarta.inject.Inject;
+import java.util.Set;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.internal.ClusterConfiguration;
+import org.apache.ignite.internal.ClusterPerTestIntegrationTest;
+import org.apache.ignite.internal.app.IgniteImpl;
+import
org.apache.ignite.internal.rest.api.cluster.zone.datanodes.DataNodesRecalculationRequest;
+import org.apache.ignite.internal.rest.constants.HttpCode;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/** Test for distributed zones data nodes controller. */
+@MicronautTest
+public class ItDataNodesControllerTest extends ClusterPerTestIntegrationTest {
Review Comment:
We need to add similar test as
`org.apache.ignite.internal.distributionzones.DataNodesManagerTest#filterChange`
to check new recalculation method, we need to check that timers are
discarded
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DataNodesManager.java:
##########
Review Comment:
not entirely true now
##########
modules/distribution-zones/src/integrationTest/java/org/apache/ignite/internal/distributionzones/ItDataNodesManagerTest.java:
##########
@@ -60,7 +52,7 @@ public void manualDataNodeRecalculationIdempotencyTest()
throws InterruptedExcep
public void manualDataNodeRecalculationAfterNewNodeAddedTest() throws
InterruptedException {
IgniteImpl node = unwrapIgniteImpl(node(0));
- createZoneWithInfiniteTimers(node);
+ createZoneWithInfiniteTimers(node, ZONE_NAME);
Review Comment:
I would expect that we also test that removed node is reflected in the data
nodes after immediate recalculation.
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DataNodesManager.java:
##########
@@ -916,10 +916,10 @@ public CompletableFuture<Set<String>> dataNodes(int
zoneId, HybridTimestamp time
}
/**
- * Unlike {@link #dataNodes} this method recalculates the data nodes,
writes it to metastorage and history, and returns them.
+ * Unlike {@link #dataNodes} this method data nodes for given zone and
writes them to metastorage.
Review Comment:
missed some word
--
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]
