You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "tkalkirill (via GitHub)" <gi...@apache.org> on 2024/05/29 13:52:17 UTC
[PR] IGNITE-21466 Add metrics for partition states [ignite-3]
tkalkirill opened a new pull request, #3835:
URL: https://github.com/apache/ignite-3/pull/3835
https://issues.apache.org/jira/browse/IGNITE-21466
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- There is a single JIRA ticket related to the pull request.
\- The web-link to the pull request is attached to the JIRA ticket.
\- The JIRA ticket has the Patch Available state.
\- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
\- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625654416
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -575,10 +595,8 @@ private static <T> boolean containsOrEmpty(T item, Collection<T> collection) {
return collection.isEmpty() || collection.contains(item);
}
- /**
- * Converts internal raft node state into public local partition state.
- */
- private static LocalPartitionStateEnum convertState(State nodeState) {
+ /** Converts internal raft node state into public local partition state. */
Review Comment:
> In this question you mention about yourself, I cannot answer it.
That was a question directed to you, maybe I phrased it poorly. But you already replied that you don't know the answer, so that's fine
> For me, if a multi-line comment can fit into one, then it looks more expressive, but not 100%, it depends on the case and environment.
This is what we discussed. That you won't be doing such changes, because it's all too subjective. It you have a desire to fix formatting of comments in the project - then do it in the entire project. Otherwise I'm against these changes in your PRs, and you know that.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1624633692
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
+ if (raftNodeId.groupId() instanceof TablePartitionId) {
+ var tablePartitionId = (TablePartitionId) raftNodeId.groupId();
+
+ if (tablePartitionId.tableId() == tableId) {
+ LocalPartitionStateEnum partitionState = localPartitionState(raftGroupService);
+
+ if (partitionState == state) {
+ count[0]++;
+ }
+ }
+ }
+ });
+
+ return count[0];
+ }
+
+ private static LocalPartitionStateEnum localPartitionState(RaftGroupService raftGroupService) {
+ Node raftNode = raftGroupService.getRaftNode();
+
+ LocalPartitionStateEnum localState = convertState(raftNode.getNodeState());
+ long lastLogIndex = raftNode.lastLogIndex();
+
+ if (localState == HEALTHY) {
Review Comment:
I agree that it looks like a copy-paste, I’ll try to look at it again or comment.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625643679
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
> what do you think?
Same problem will be there for large numbers of zones, but I think that this situation would be rare. Originally I said _I don't know if I should ask you to fix it now_ for this exact reason by the way
> for each metric source of the total will be O(5*N), where it will be N^2 I don’t understand.
What exactly was confusing in my explanation? I thought it was as detailed as possible. If you have `O(N)` metrics, and all of them require `O(N)` operations to be computed, then computing all of them would be `O(N*N)`. What else should I add?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625602920
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -575,10 +595,8 @@ private static <T> boolean containsOrEmpty(T item, Collection<T> collection) {
return collection.isEmpty() || collection.contains(item);
}
- /**
- * Converts internal raft node state into public local partition state.
- */
- private static LocalPartitionStateEnum convertState(State nodeState) {
+ /** Converts internal raft node state into public local partition state. */
Review Comment:
> purely for beauty.
We discussed this many times, and I thought that we had an agreement. Apparently we don't. And I feel stupid, because I have to repeat the same comment over and over again, without any success.
> I changed this particular method a little for the task and therefore formatted the documentation
Did you change the documentation?
Let me rephrase the question. Let's assume that you changed line number 15 in the code. What's the range of lines in the code that allow for changes with _very_ subjective value, the ones that I would call _useless_? Lines 14-16, or maybe 12-18? How do you decide that?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625627865
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -575,10 +595,8 @@ private static <T> boolean containsOrEmpty(T item, Collection<T> collection) {
return collection.isEmpty() || collection.contains(item);
}
- /**
- * Converts internal raft node state into public local partition state.
- */
- private static LocalPartitionStateEnum convertState(State nodeState) {
+ /** Converts internal raft node state into public local partition state. */
Review Comment:
Yes, we discussed this more than once, for my part I feel violence and intolerance for a different view of things.
> Did you change the documentation?
No, I just changed the number of lines.
> What's the range of lines in the code that allow for changes with very subjective value, the ones that I would call useless?
In this question you mention about yourself, I cannot answer it.
> Lines 14-16, or maybe 12-18?
Don't know.
> How do you decide that?
For me, if a multi-line comment can fit into one, then it looks more expressive, but not 100%, it depends on the case and environment.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625643679
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
> what do you think?
Same problem will be there for large numbers of zones, but I think that this situation would be rare. Originally I said I don't know if I should ask you to fix it now_ for this exact reason by the way
> for each metric source of the total will be O(5*N), where it will be N^2 I don’t understand.
What exactly was confusing in my explanation? I thought it was as details as possible. If you have `O(N)` metrics, and all of them require `O(N)` operations to be computed, then computing all of them would be `O(N*N)`. What else should I add?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625590711
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
> I don’t understand where the O(n^2), can you clearly indicate?
Never thought that I would have to do this, but here we go. Suppose you have `N` tables, each of them has an associated metrics source. For each source you have `5` metrics that show partition states. For each state you scan all `N` tables while looking for the one that you need (let's ignore partitions for simplicity). That's called a linear search and it's `O(N)`. If you multiply it by the number of metrics sources and the number of metrics, you will get `5*N*N` read operations per metrics accumulation, which is `O(N^2)`. This is what I meant.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625644873
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -556,35 +551,19 @@ private void handleLocalPartitionStatesRequest(LocalPartitionStatesRequest reque
CatalogTableDescriptor tableDescriptor = catalogManager.table(tablePartitionId.tableId(), catalogVersion);
// Only tables that belong to a specific catalog version will be returned.
- if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())
- ) {
+ if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())) {
return;
}
- Node raftNode = raftGroupService.getRaftNode();
- State nodeState = raftNode.getNodeState();
-
- LocalPartitionStateEnum localState = convertState(nodeState);
- long lastLogIndex = raftNode.lastLogIndex();
-
- if (localState == HEALTHY) {
- // Node without log didn't process anything yet, it's not really "healthy" before it accepts leader's configuration.
- if (lastLogIndex == 0) {
- localState = INITIALIZING;
- }
-
- if (raftNode.isInstallingSnapshot()) {
- localState = INSTALLING_SNAPSHOT;
- }
- }
+ LocalPartitionStateEnumWithLogIndex localPartitionStateWithLogIndex = of(raftGroupService.getRaftNode());
Review Comment:
I think we can avoid using static import here and that would be fine. We don't use it for `List.of` and `Set.of`, and nobody is upset about it, right?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625666687
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -556,35 +551,19 @@ private void handleLocalPartitionStatesRequest(LocalPartitionStatesRequest reque
CatalogTableDescriptor tableDescriptor = catalogManager.table(tablePartitionId.tableId(), catalogVersion);
// Only tables that belong to a specific catalog version will be returned.
- if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())
- ) {
+ if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())) {
return;
}
- Node raftNode = raftGroupService.getRaftNode();
- State nodeState = raftNode.getNodeState();
-
- LocalPartitionStateEnum localState = convertState(nodeState);
- long lastLogIndex = raftNode.lastLogIndex();
-
- if (localState == HEALTHY) {
- // Node without log didn't process anything yet, it's not really "healthy" before it accepts leader's configuration.
- if (lastLogIndex == 0) {
- localState = INITIALIZING;
- }
-
- if (raftNode.isInstallingSnapshot()) {
- localState = INSTALLING_SNAPSHOT;
- }
- }
+ LocalPartitionStateEnumWithLogIndex localPartitionStateWithLogIndex = of(raftGroupService.getRaftNode());
Review Comment:
Reasonable, I agree.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1624464476
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
+ if (raftNodeId.groupId() instanceof TablePartitionId) {
+ var tablePartitionId = (TablePartitionId) raftNodeId.groupId();
+
+ if (tablePartitionId.tableId() == tableId) {
+ LocalPartitionStateEnum partitionState = localPartitionState(raftGroupService);
+
+ if (partitionState == state) {
+ count[0]++;
+ }
+ }
+ }
+ });
+
+ return count[0];
+ }
+
+ private static LocalPartitionStateEnum localPartitionState(RaftGroupService raftGroupService) {
+ Node raftNode = raftGroupService.getRaftNode();
+
+ LocalPartitionStateEnum localState = convertState(raftNode.getNodeState());
+ long lastLogIndex = raftNode.lastLogIndex();
+
+ if (localState == HEALTHY) {
Review Comment:
Looks like a copy-paste, maybe you should re-use existing code instead. If not, then please explain you reasoning, thank you
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -575,10 +595,8 @@ private static <T> boolean containsOrEmpty(T item, Collection<T> collection) {
return collection.isEmpty() || collection.contains(item);
}
- /**
- * Converts internal raft node state into public local partition state.
- */
- private static LocalPartitionStateEnum convertState(State nodeState) {
+ /** Converts internal raft node state into public local partition state. */
Review Comment:
Why do you keep fixing formatting for comments that you have not changed? Text is the same, literally every letter is the same, I don't get it. We discussed it, like, 10 times already. Is this the impulse that you can't control? In that case I already offered you fixing the entire project, you rejected.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
I think that this algorithm will be extremely inefficient. Loading metrics for all tables will be `O(n^2)`, which is pretty bad in general. I don't know if I should ask you to fix it now, but you should keep in mind that ideally we should do better than this. If you have better ideas about improving the approach, please reply here, we can discuss them.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625643679
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
> what do you think?
Same problem will be there for large numbers of zones, but I think that this situation would be rare. Originally I said I don't know if I should ask you to fix it now_ for this exact reason by the way
> for each metric source of the total will be O(5*N), where it will be N^2 I don’t understand.
What exactly was confusing in my explanation? I thought it was as detailed as possible. If you have `O(N)` metrics, and all of them require `O(N)` operations to be computed, then computing all of them would be `O(N*N)`. What else should I add?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625606124
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -556,35 +551,19 @@ private void handleLocalPartitionStatesRequest(LocalPartitionStatesRequest reque
CatalogTableDescriptor tableDescriptor = catalogManager.table(tablePartitionId.tableId(), catalogVersion);
// Only tables that belong to a specific catalog version will be returned.
- if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())
- ) {
+ if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())) {
return;
}
- Node raftNode = raftGroupService.getRaftNode();
- State nodeState = raftNode.getNodeState();
-
- LocalPartitionStateEnum localState = convertState(nodeState);
- long lastLogIndex = raftNode.lastLogIndex();
-
- if (localState == HEALTHY) {
- // Node without log didn't process anything yet, it's not really "healthy" before it accepts leader's configuration.
- if (lastLogIndex == 0) {
- localState = INITIALIZING;
- }
-
- if (raftNode.isInstallingSnapshot()) {
- localState = INSTALLING_SNAPSHOT;
- }
- }
+ LocalPartitionStateEnumWithLogIndex localPartitionStateWithLogIndex = of(raftGroupService.getRaftNode());
Review Comment:
I'm not a fan of static imports for methods called `of`, or `getInstance`, or something else non-descriptive. Please chose another name or don't use static import in this particular case, thank you
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill merged PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1629785993
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -556,35 +551,19 @@ private void handleLocalPartitionStatesRequest(LocalPartitionStatesRequest reque
CatalogTableDescriptor tableDescriptor = catalogManager.table(tablePartitionId.tableId(), catalogVersion);
// Only tables that belong to a specific catalog version will be returned.
- if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())
- ) {
+ if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())) {
return;
}
- Node raftNode = raftGroupService.getRaftNode();
- State nodeState = raftNode.getNodeState();
-
- LocalPartitionStateEnum localState = convertState(nodeState);
- long lastLogIndex = raftNode.lastLogIndex();
-
- if (localState == HEALTHY) {
- // Node without log didn't process anything yet, it's not really "healthy" before it accepts leader's configuration.
- if (lastLogIndex == 0) {
- localState = INITIALIZING;
- }
-
- if (raftNode.isInstallingSnapshot()) {
- localState = INSTALLING_SNAPSHOT;
- }
- }
+ LocalPartitionStateEnumWithLogIndex localPartitionStateWithLogIndex = of(raftGroupService.getRaftNode());
Review Comment:
fix it
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625423488
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
Today I thought that it would be great to have some kind of listener that would trigger a callback when the status of the node changes (To do this you will need to update the code to jraft), but then we would need to somehow manage this and keep in memory some amount of data that we don’t really need.
I looked more closely at making some kind of `Loza#raftGroupService` to make a loop through partitions, but we will either need to have 2 maps, or it will be more difficult to create `org.apache.ignite.internal.raft.RaftNodeId`.
On the other hand, we need such a metric only for disaster recovery and we may not worry about its performance for now, since we will go through cycles through the entities in memory, what do you think?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625614279
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
> what do you think?
I think that eventually it will bite us in the rear. Avoiding listeners, the only good solution would be to ask a `TableManager` for all replicas of a given table, and to have it there already grouped by `tableId`. Then we should be able to access the raft group from the replica, that would be optimal and relatively simple.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625711501
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
> What exactly was confusing in my explanation?
I think that it is more correct to estimate the complexity of O(MxN), where N is the number of tables and M is the number of metrics?
O(N*N) is obtained if we load all metrics for all tables at once.
If we load metrics only for a specific one or for the entire table, then this will not happen.
I looked in the `TableManager` and did not find a suitable API for getting the state of the raft node; in `org.apache.ignite.internal.table.TableImpl` we store `org.apache.ignite.internal.raft.service.RaftGroupService`, which is different from the `org.apache.ignite.raft.jraft.RaftGroupService` that is currently in use.
To avoid O(N*N) as I described above, we can make metrics common to all tables (in the future zones) and then there will always be O(M*N). I don’t really like adding `org.apache.ignite.raft.jraft.RaftGroupService` to `org.apache.ignite.internal.table.TableImpl`, we can add `org.apache.ignite.internal.raft.RaftNodeId` to `TableImpl`.
Well, or leave it as is, what do you think?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1624632167
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -575,10 +595,8 @@ private static <T> boolean containsOrEmpty(T item, Collection<T> collection) {
return collection.isEmpty() || collection.contains(item);
}
- /**
- * Converts internal raft node state into public local partition state.
- */
- private static LocalPartitionStateEnum convertState(State nodeState) {
+ /** Converts internal raft node state into public local partition state. */
Review Comment:
I ask you to be more polite, and to be less emotional and more professional.
I said that I would stop formatting the text and making changes that did not affect my changes.
I changed this particular method a little for the task and therefore formatted the documentation, purely for beauty.
I’ll try to answer your question again in advance, no, I won’t make any rules or big changes for the entire project.
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
I have not found any simple and suitable mechanisms for obtaining node statuses, there are no listeners or something like that. Maybe you know?
I thought that to begin with, this approach would be sufficient and cover the needs; if there is a strong drawdown in performance, then we will do it somehow differently.
I don’t understand where the O(n^2), can you clearly indicate?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "ibessonov (via GitHub)" <gi...@apache.org>.
ibessonov commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625720574
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
> O(N*N) is obtained if we load all metrics for all tables at once.
Yes, that's precisely what I said. Quote: _"Loading metrics for all tables will be O(n^2)"_
> I looked in the TableManager and did not find a suitable API for getting the state of the raft node
That's right, it doesn't exist right now, but it could if we really needed it.
> Well, or leave it as is, what do you think?
We can leave it as is. My main point was that you should be aware of the complexity of your algorithms, and be sure that there's a good reason and it's safe to have O(N*N) code in some places. Ideally, such a code should be commented with explanation why it's OK. Otherwise readers will have same questions as I do
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625464185
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
+ if (raftNodeId.groupId() instanceof TablePartitionId) {
+ var tablePartitionId = (TablePartitionId) raftNodeId.groupId();
+
+ if (tablePartitionId.tableId() == tableId) {
+ LocalPartitionStateEnum partitionState = localPartitionState(raftGroupService);
+
+ if (partitionState == state) {
+ count[0]++;
+ }
+ }
+ }
+ });
+
+ return count[0];
+ }
+
+ private static LocalPartitionStateEnum localPartitionState(RaftGroupService raftGroupService) {
+ Node raftNode = raftGroupService.getRaftNode();
+
+ LocalPartitionStateEnum localState = convertState(raftNode.getNodeState());
+ long lastLogIndex = raftNode.lastLogIndex();
+
+ if (localState == HEALTHY) {
Review Comment:
try fix it
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625611015
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
Let's figure it out.
N - the number of tables, which is equal to the number of metrics source.
I suggest adding 5 metrics.
For each metric, O(N) will be performed.
Those. for each metric source of the total will be O(5*N), where it will be N^2 I don’t understand.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625616460
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -556,35 +551,19 @@ private void handleLocalPartitionStatesRequest(LocalPartitionStatesRequest reque
CatalogTableDescriptor tableDescriptor = catalogManager.table(tablePartitionId.tableId(), catalogVersion);
// Only tables that belong to a specific catalog version will be returned.
- if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())
- ) {
+ if (tableDescriptor == null || !containsOrEmpty(tableDescriptor.zoneId(), request.zoneIds())) {
return;
}
- Node raftNode = raftGroupService.getRaftNode();
- State nodeState = raftNode.getNodeState();
-
- LocalPartitionStateEnum localState = convertState(nodeState);
- long lastLogIndex = raftNode.lastLogIndex();
-
- if (localState == HEALTHY) {
- // Node without log didn't process anything yet, it's not really "healthy" before it accepts leader's configuration.
- if (lastLogIndex == 0) {
- localState = INITIALIZING;
- }
-
- if (raftNode.isInstallingSnapshot()) {
- localState = INSTALLING_SNAPSHOT;
- }
- }
+ LocalPartitionStateEnumWithLogIndex localPartitionStateWithLogIndex = of(raftGroupService.getRaftNode());
Review Comment:
I was wondering which name would be more appropriate, `convert` doesn’t seem to fit, maybe `toLocalPartitionStateWithLogIndex` or `getLocalPartitionStateWithLogIndex`, what do you think?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-21466 Add metrics for partition states [ignite-3]
Posted by "tkalkirill (via GitHub)" <gi...@apache.org>.
tkalkirill commented on code in PR #3835:
URL: https://github.com/apache/ignite-3/pull/3835#discussion_r1625632478
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/PartitionStatesMetricSource.java:
##########
@@ -0,0 +1,152 @@
+/*
+ * 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.table.distributed.disaster;
+
+import static org.apache.ignite.internal.table.distributed.disaster.DisasterRecoveryManager.convertState;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.BROKEN;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.HEALTHY;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INITIALIZING;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.INSTALLING_SNAPSHOT;
+import static org.apache.ignite.internal.table.distributed.disaster.LocalPartitionStateEnum.UNAVAILABLE;
+
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.metrics.MetricSet;
+import org.apache.ignite.internal.metrics.MetricSetBuilder;
+import org.apache.ignite.internal.metrics.MetricSource;
+import org.apache.ignite.internal.replicator.TablePartitionId;
+import org.apache.ignite.raft.jraft.Node;
+import org.apache.ignite.raft.jraft.RaftGroupService;
+import org.jetbrains.annotations.Nullable;
+
+/** Source of metrics for table partition statuses. */
+class PartitionStatesMetricSource implements MetricSource {
+ private final String metricSourceName;
+
+ private final int tableId;
+
+ private final DisasterRecoveryManager disasterRecoveryManager;
+
+ /** Enablement status. Accessed from different threads under synchronization on this object. */
+ private boolean enabled;
+
+ PartitionStatesMetricSource(
+ CatalogTableDescriptor tableDescriptor,
+ DisasterRecoveryManager disasterRecoveryManager
+ ) {
+ this.tableId = tableDescriptor.id();
+ this.disasterRecoveryManager = disasterRecoveryManager;
+
+ metricSourceName = String.format("partition.states.zone.%s.table.%s", tableDescriptor.zoneId(), tableDescriptor.id());
+ }
+
+ @Override
+ public String name() {
+ return metricSourceName;
+ }
+
+ @Override
+ public synchronized @Nullable MetricSet enable() {
+ if (enabled) {
+ return null;
+ }
+
+ var builder = new MetricSetBuilder(metricSourceName);
+
+ builder.longGauge(
+ "UnavailablePartitionCount",
+ "Count of partitions not yet started.",
+ () -> calculatePartitionCountByLocalState(UNAVAILABLE)
+ );
+
+ builder.longGauge(
+ "HealthyPartitionCount",
+ "Count of living partitions with a healthy state machine.",
+ () -> calculatePartitionCountByLocalState(HEALTHY)
+ );
+
+ builder.longGauge(
+ "InitializingPartitionCount",
+ "Count of partitions that are starting right now.",
+ () -> calculatePartitionCountByLocalState(INITIALIZING)
+ );
+
+ builder.longGauge(
+ "InstallingSnapshotPartitionCount",
+ "Count of partitions that installing Raft snapshots from the leader.",
+ () -> calculatePartitionCountByLocalState(INSTALLING_SNAPSHOT)
+ );
+
+ builder.longGauge(
+ "BrokenPartitionCount",
+ "Count of broken partitions.",
+ () -> calculatePartitionCountByLocalState(BROKEN)
+ );
+
+ return builder.build();
+ }
+
+ @Override
+ public synchronized void disable() {
+ enabled = false;
+ }
+
+ @Override
+ public synchronized boolean enabled() {
+ return enabled;
+ }
+
+ private long calculatePartitionCountByLocalState(LocalPartitionStateEnum state) {
+ long[] count = {0};
+
+ disasterRecoveryManager.raftManager.forEach((raftNodeId, raftGroupService) -> {
Review Comment:
Yes, but on the other hand, in some future we will switch to partition colocation, and then it will be possible to optimize this, what do you think?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org