IGNITE-27542 : Use MessageSerializer for TcpDiscoveryMetricsUpdateMessage#12620
Conversation
…Message # Conflicts: # modules/core/src/main/java/org/apache/ignite/internal/managers/discovery/DiscoveryMessageFactory.java # modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryNodeMetricsMessage.java
|
...in/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryNodesMetricsMapMessage.java
Outdated
Show resolved
Hide resolved
...main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryMetricsUpdateMessage.java
Outdated
Show resolved
Hide resolved
| assert metrics != null; | ||
| assert !this.metrics.containsKey(nodeId); | ||
| public void addServerMetrics(UUID srvrId, ClusterMetrics newMetrics) { | ||
| assert srvrId != null; |
There was a problem hiding this 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.
There was a problem hiding this comment.
We still can drop a node by the metrics. At least a client. Look for 'Failing client node due to not receiving metrics updates '. It traverses a map. If key not found, node is considered as failed. Unfortunatelly. I would not touch this logic and what related to it. Instead, we might move these metrics into Communication. And/or remove them at all.




No description provided.