rpuch commented on code in PR #7650:
URL: https://github.com/apache/ignite-3/pull/7650#discussion_r2845900624
##########
modules/raft/src/integrationTest/java/org/apache/ignite/raft/ItRaftMetricTest.java:
##########
@@ -18,78 +18,56 @@
package org.apache.ignite.raft;
import static
org.apache.ignite.internal.ClusterPerTestIntegrationTest.aggressiveLowWatermarkIncreaseClusterConfig;
-import static
org.apache.ignite.internal.TestMetricUtils.testMetricChangeAfterOperation;
-import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
import static
org.apache.ignite.internal.metrics.sources.RaftMetricSource.RAFT_GROUP_LEADERS;
import static
org.apache.ignite.internal.metrics.sources.RaftMetricSource.SOURCE_NAME;
import static org.awaitility.Awaitility.await;
-import java.util.List;
import org.apache.ignite.InitParametersBuilder;
import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.TestMetricUtils;
import org.apache.ignite.internal.metrics.sources.RaftMetricSource;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/** Tests for {@link RaftMetricSource}. */
public class ItRaftMetricTest extends ClusterPerClassIntegrationTest {
private static final String ZONE_NAME = "TEST_ZONE";
+ // CMG and Metastore leaders.
+ private static final int SYSTEM_RAFT_LEADER_COUNT = 2;
+
@Override
protected void configureInitParameters(InitParametersBuilder builder) {
// To trigger zone's raft partitions destruction.
builder.clusterConfiguration(aggressiveLowWatermarkIncreaseClusterConfig());
}
- @BeforeEach
- void setUp() {
- dropTableAndZone();
- }
-
@Test
- @Disabled("https://issues.apache.org/jira/browse/IGNITE-27728";)
void testLeaderCountIncreases() {
- testMetricChangeAfterOperation(
- CLUSTER,
- SOURCE_NAME,
- List.of(RAFT_GROUP_LEADERS),
- List.of((long) DEFAULT_PARTITION_COUNT),
- ItRaftMetricTest::createZoneAndTable
- );
+ createZoneIfNotExists();
+
+ awaitExpectedLeaderCount(DEFAULT_PARTITION_COUNT +
SYSTEM_RAFT_LEADER_COUNT);
Review Comment:
It might happen that default partition count starts being calculated
dynamically, this will break this test. Let's just specify partition count
explicitly in the DDL
##########
modules/raft/src/integrationTest/java/org/apache/ignite/raft/ItRaftMetricTest.java:
##########
@@ -18,78 +18,56 @@
package org.apache.ignite.raft;
import static
org.apache.ignite.internal.ClusterPerTestIntegrationTest.aggressiveLowWatermarkIncreaseClusterConfig;
-import static
org.apache.ignite.internal.TestMetricUtils.testMetricChangeAfterOperation;
-import static org.apache.ignite.internal.TestWrappers.unwrapIgniteImpl;
import static
org.apache.ignite.internal.metrics.sources.RaftMetricSource.RAFT_GROUP_LEADERS;
import static
org.apache.ignite.internal.metrics.sources.RaftMetricSource.SOURCE_NAME;
import static org.awaitility.Awaitility.await;
-import java.util.List;
import org.apache.ignite.InitParametersBuilder;
import org.apache.ignite.internal.ClusterPerClassIntegrationTest;
+import org.apache.ignite.internal.TestMetricUtils;
import org.apache.ignite.internal.metrics.sources.RaftMetricSource;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
/** Tests for {@link RaftMetricSource}. */
public class ItRaftMetricTest extends ClusterPerClassIntegrationTest {
private static final String ZONE_NAME = "TEST_ZONE";
+ // CMG and Metastore leaders.
+ private static final int SYSTEM_RAFT_LEADER_COUNT = 2;
+
@Override
protected void configureInitParameters(InitParametersBuilder builder) {
// To trigger zone's raft partitions destruction.
builder.clusterConfiguration(aggressiveLowWatermarkIncreaseClusterConfig());
}
- @BeforeEach
- void setUp() {
- dropTableAndZone();
- }
-
@Test
- @Disabled("https://issues.apache.org/jira/browse/IGNITE-27728";)
void testLeaderCountIncreases() {
- testMetricChangeAfterOperation(
- CLUSTER,
- SOURCE_NAME,
- List.of(RAFT_GROUP_LEADERS),
- List.of((long) DEFAULT_PARTITION_COUNT),
- ItRaftMetricTest::createZoneAndTable
- );
+ createZoneIfNotExists();
+
+ awaitExpectedLeaderCount(DEFAULT_PARTITION_COUNT +
SYSTEM_RAFT_LEADER_COUNT);
}
@Test
- @Disabled("https://issues.apache.org/jira/browse/IGNITE-27728";)
void testLeaderCountDecreases() {
- createZoneAndTable();
+ createZoneIfNotExists();
- testMetricChangeAfterOperation(
- CLUSTER,
- SOURCE_NAME,
- List.of(RAFT_GROUP_LEADERS),
- List.of((long) -DEFAULT_PARTITION_COUNT),
- () -> {
- int initialNodes = getRaftNodesCount();
+ awaitExpectedLeaderCount(DEFAULT_PARTITION_COUNT +
SYSTEM_RAFT_LEADER_COUNT);
- dropTableAndZone();
+ dropZone();
- // Waiting for zone partitions to be destroyed.
- await().until(() -> initialNodes - getRaftNodesCount() >=
DEFAULT_PARTITION_COUNT);
- });
+ awaitExpectedLeaderCount(SYSTEM_RAFT_LEADER_COUNT);
}
- private static void dropTableAndZone() {
- sql("DROP ZONE IF EXISTS " + ZONE_NAME);
+ private static void awaitExpectedLeaderCount(int expected) {
+ await().until(() -> TestMetricUtils.metricValue(CLUSTER, SOURCE_NAME,
RAFT_GROUP_LEADERS) == expected);
Review Comment:
```suggestion
await().until(() -> TestMetricUtils.metricValue(CLUSTER,
SOURCE_NAME, RAFT_GROUP_LEADERS), is(expected));
```
This will make failure information more useful: it will print the actual
value as well
##########
modules/runner/src/testFixtures/java/org/apache/ignite/internal/TestMetricUtils.java:
##########
@@ -70,33 +70,46 @@ public static void testMetricChangeAfterOperation(
* @param metricNames Metric names.
* @return Map of metric names to their values.
*/
- private static Map<String, Long> metricValues(Cluster cluster, String
sourceName, List<String> metricNames) {
+ public static Map<String, Long> metricValues(Cluster cluster, String
sourceName, List<String> metricNames) {
Map<String, Long> values = new HashMap<>(metricNames.size());
+ for (String metricName : metricNames) {
+ values.put(metricName, metricValue(cluster, sourceName,
metricName));
+ }
+
+ return values;
+ }
+
+ /**
+ * Returns the sum of the specified metric on all nodes.
+ *
+ * @param metricName Metric names.
+ */
+ public static long metricValue(Cluster cluster, String sourceName, String
metricName) {
+ long result = 0;
+
for (int i = 0; i < cluster.runningNodes().count(); i++) {
MetricSet metricSet =
unwrapIgniteImpl(cluster.node(i)).metricManager().metricSnapshot().metrics()
.get(sourceName);
assertThat(metricSet, is(notNullValue()));
- for (String metricName : metricNames) {
- Metric metric = metricSet.get(metricName);
+ Metric metric = metricSet.get(metricName);
- assertThat(metric, is(notNullValue()));
+ assertThat(metric, is(notNullValue()));
- if (metric instanceof IntMetric) {
- values.merge(metricName, (long) ((IntMetric)
metric).value(), Long::sum);
- } else {
- assertThat(
- "Not a LongMetric / IntMetric [name=" + metricName
+ ", class=" + metric.getClass().getSimpleName() + ']',
- metric,
- instanceOf(LongMetric.class));
+ if (metric instanceof IntMetric) {
Review Comment:
This code is asymmetric wrt IntMetric/LongMetric, even though it works
symmetrically wrt to them. Let's do it symmetric: if (IntMetric) ... else if
(LongMetric) ... else fail(not IntMetric/LongMetric)
--
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]
