sergey-chugunov-1985 commented on code in PR #12620: URL: https://github.com/apache/ignite/pull/12620#discussion_r2698386638
########## modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryNodesMetricsMapMessage.java: ########## @@ -0,0 +1,57 @@ +/* + * 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.spi.discovery.tcp.messages; + +import java.util.Map; +import java.util.UUID; +import org.apache.ignite.internal.Order; +import org.apache.ignite.internal.managers.discovery.DiscoveryMessageFactory; +import org.apache.ignite.internal.util.typedef.internal.S; +import org.apache.ignite.plugin.extensions.communication.Message; + +/** Holds map of nodes metrics messages per node id. */ +public class TcpDiscoveryNodesMetricsMapMessage implements Message { Review Comment: From usages of this class I can see that it is used solely for metrics from client nodes. I think we should capture this information right in the name of the class to make it easier for a developer to understand its purpose. What do you think about a name like `TcpDiscoveryClientNodesMetricsMessage`? In that case it may make sense to rewrite javadoc as well. ########## modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryMetricsUpdateMessage.java: ########## @@ -79,130 +83,136 @@ public TcpDiscoveryMetricsUpdateMessage(UUID creatorNodeId) { /** * Sets metrics for particular node. * - * @param nodeId Node ID. - * @param metrics Node metrics. + * @param srvrId Server ID. + * @param newMetrics New server metrics to add. */ - public void setMetrics(UUID nodeId, ClusterMetrics metrics) { - assert nodeId != null; - assert metrics != null; - assert !this.metrics.containsKey(nodeId); + public void addServerMetrics(UUID srvrId, ClusterMetrics newMetrics) { + assert srvrId != null; Review Comment: Do we really need to protect something in metric message with asserts? I believe that metrics are not that important, and even if some parameters are null, we could skip any actions with metrics without big harm to functionality of the cluster. At the same time triggered assertion could cause a node or ever a cluster to shut down. I don't think we should tie stability of the cluster to possible issues with metrics gathering. Importance of metrics is lower than an overall stability of the cluster. ########## modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryMetricsUpdateMessage.java: ########## @@ -79,130 +83,136 @@ public TcpDiscoveryMetricsUpdateMessage(UUID creatorNodeId) { /** * Sets metrics for particular node. * - * @param nodeId Node ID. - * @param metrics Node metrics. + * @param srvrId Server ID. + * @param newMetrics New server metrics to add. */ - public void setMetrics(UUID nodeId, ClusterMetrics metrics) { - assert nodeId != null; - assert metrics != null; - assert !this.metrics.containsKey(nodeId); + public void addServerMetrics(UUID srvrId, ClusterMetrics newMetrics) { + assert srvrId != null; + assert newMetrics != null; + + if (serversFullMetricsMsgs == null) + serversFullMetricsMsgs = new HashMap<>(); + + assert !serversFullMetricsMsgs.containsKey(srvrId); + + serversFullMetricsMsgs.compute(srvrId, (srvrId0, srvrFullMetrics) -> { + if (srvrFullMetrics == null) + srvrFullMetrics = new TcpDiscoveryNodeFullMetricsMessage(); + + srvrFullMetrics.nodeMetricsMessage(new TcpDiscoveryNodeMetricsMessage(newMetrics)); - this.metrics.put(nodeId, new MetricsSet(metrics)); + return srvrFullMetrics; + }); } /** * Sets cache metrics for particular node. * - * @param nodeId Node ID. - * @param metrics Node cache metrics. + * @param srvrId Server ID. + * @param newCachesMetrics News server's caches metrics to add. */ - public void setCacheMetrics(UUID nodeId, Map<Integer, CacheMetrics> metrics) { - assert nodeId != null; - assert metrics != null; - assert !this.cacheMetrics.containsKey(nodeId); + public void addServerCacheMetrics(UUID srvrId, Map<Integer, CacheMetrics> newCachesMetrics) { + assert srvrId != null; + assert newCachesMetrics != null; - if (!F.isEmpty(metrics)) - this.cacheMetrics.put(nodeId, metrics); + if (serversFullMetricsMsgs == null) + serversFullMetricsMsgs = new HashMap<>(); + + assert serversFullMetricsMsgs.containsKey(srvrId) && serversFullMetricsMsgs.get(srvrId).cachesMetricsMessages() == null; + + serversFullMetricsMsgs.compute(srvrId, (srvrId0, srvrFullMetrics) -> { + if (srvrFullMetrics == null) + srvrFullMetrics = new TcpDiscoveryNodeFullMetricsMessage(); + + Map<Integer, CacheMetricsMessage> newCachesMsgsMap = U.newHashMap(newCachesMetrics.size()); + + newCachesMetrics.forEach((cacheId, cacheMetrics) -> + newCachesMsgsMap.put(cacheId, new TcpDiscoveryCacheMetricsMessage(cacheMetrics))); + + srvrFullMetrics.cachesMetricsMessages(newCachesMsgsMap); + + return srvrFullMetrics; + }); } /** - * Sets metrics for a client node. + * Sets metrics for a connected client node. * - * @param nodeId Server node ID. - * @param clientNodeId Client node ID. - * @param metrics Node metrics. + * @param srvrId Server node ID. + * @param clientNodeId Connected client node ID. + * @param clientMetrics Client metrics. */ - public void setClientMetrics(UUID nodeId, UUID clientNodeId, ClusterMetrics metrics) { - assert nodeId != null; + public void setClientMetrics(UUID srvrId, UUID clientNodeId, ClusterMetrics clientMetrics) { Review Comment: ```suggestion public void addClientMetrics(UUID srvrId, UUID clientNodeId, ClusterMetrics clientMetrics) { ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
