You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/08/13 15:16:15 UTC
[GitHub] [ignite] NSAmelchev opened a new pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
NSAmelchev opened a new pull request #8150:
URL: https://github.com/apache/ignite/pull/8150
Add metrics to the new framework:
- TopologyVersion
- TotalNodes
- TotalBaselineNodes
- TotalServerNodes
- TotalClientNodes
- ActiveBaselineNodes
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r470615267
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/ClusterNodeMetricsSelfTest.java
##########
@@ -310,7 +314,7 @@ public void testIoMetrics() throws Exception {
*/
@Test
public void testClusterNodeMetrics() throws Exception {
- final Ignite ignite0 = grid();
+ IgniteEx ignite0 = grid();
Review comment:
Let's return `final` modifier.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r470585340
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ClusterProcessor.java
##########
@@ -607,6 +610,33 @@ private Boolean findLastFlag(Collection<Serializable> vals) {
U.error(log, "Failed to register MBean for cluster: ", e);
}
}
+
+ MetricRegistry reg = ctx.metric().registry(CLUSTER_METRICS);
+
+ reg.register("TopologyVersion",
Review comment:
Can we use this metric and all below inside `ClusterMetricsMXBeanImpl` to keep single method to calculate metric value.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] NSAmelchev commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r470605437
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ClusterProcessor.java
##########
@@ -607,6 +610,33 @@ private Boolean findLastFlag(Collection<Serializable> vals) {
U.error(log, "Failed to register MBean for cluster: ", e);
}
}
+
+ MetricRegistry reg = ctx.metric().registry(CLUSTER_METRICS);
+
+ reg.register("TopologyVersion",
Review comment:
We can use this metrics for the `ClusterMetricsMXBeanImpl` implementation. But not for the `ClusterLocalNodeMetricsMXBeanImpl`.
Moreover metrics registration should be placed between starting `GridMetricManager` and beans registration. For example, in the `IgniteKernal#registerMetrics()`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r470586856
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/ClusterBaselineNodesMetricsSelfTest.java
##########
@@ -82,9 +92,13 @@ public void testBaselineNodes() throws Exception {
log.info(String.format(">>> State #1: topology version = %d", ignite0.cluster().topologyVersion()));
assertEquals(2, mxBeanCluster.getTotalServerNodes());
+ assertEquals(2, ((IntMetric)mreg.findMetric("TotalServerNodes")).value());
Review comment:
Let's use `merg.<IntMetrics>findMetric("XXX").value()` to avoid type casting.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] NSAmelchev commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r471355594
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/ClusterMetricsMXBeanImpl.java
##########
@@ -47,13 +54,37 @@
/** Cluster metrics update mutex. */
private final Object clusterMetricsMux = new Object();
+ /** Topology version metric. */
+ private final LongMetric topVer;
Review comment:
Done.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] NSAmelchev commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r470620736
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/ClusterNodeMetricsSelfTest.java
##########
@@ -310,7 +314,7 @@ public void testIoMetrics() throws Exception {
*/
@Test
public void testClusterNodeMetrics() throws Exception {
- final Ignite ignite0 = grid();
+ IgniteEx ignite0 = grid();
Review comment:
Reverted.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] NSAmelchev commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
NSAmelchev commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r470607894
##########
File path: modules/core/src/test/java/org/apache/ignite/internal/ClusterBaselineNodesMetricsSelfTest.java
##########
@@ -82,9 +92,13 @@ public void testBaselineNodes() throws Exception {
log.info(String.format(">>> State #1: topology version = %d", ignite0.cluster().topologyVersion()));
assertEquals(2, mxBeanCluster.getTotalServerNodes());
+ assertEquals(2, ((IntMetric)mreg.findMetric("TotalServerNodes")).value());
Review comment:
Fixed.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov commented on a change in pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
nizhikov commented on a change in pull request #8150:
URL: https://github.com/apache/ignite/pull/8150#discussion_r471311105
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/ClusterMetricsMXBeanImpl.java
##########
@@ -47,13 +54,37 @@
/** Cluster metrics update mutex. */
private final Object clusterMetricsMux = new Object();
+ /** Topology version metric. */
+ private final LongMetric topVer;
Review comment:
Let's remove this metric from new framework.
"CurrentTopologyVersion" would be added by the #8145 in the "io.discovery" registry.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] nizhikov merged pull request #8150: IGNITE-13354 Add ClusterMetrics to the new framework
Posted by GitBox <gi...@apache.org>.
nizhikov merged pull request #8150:
URL: https://github.com/apache/ignite/pull/8150
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org