-
Notifications
You must be signed in to change notification settings - Fork 1.9k
IGNITE-27542 : Use MessageSerializer for TcpDiscoveryMetricsUpdateMessage #12620
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.