Skip to content

Conversation

@Vladsz83
Copy link
Contributor

No description provided.

@Vladsz83 Vladsz83 changed the title Test : Serialization of DiscoveryMetricsMsg IGNITE-27542 : Use MessageSerializer for TcpDiscoveryMetricsUpdateMessage Jan 16, 2026
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
14 New Code Smells (required ≤ 1)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

assert metrics != null;
assert !this.metrics.containsKey(nodeId);
public void addServerMetrics(UUID srvrId, ClusterMetrics newMetrics) {
assert srvrId != null;
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@sergey-chugunov-1985 sergey-chugunov-1985 merged commit 38188bb into apache:master Jan 21, 2026
5 of 6 checks passed
@Vladsz83 Vladsz83 deleted the Serialization-of-TcpDiscoveryMetricsUpdateMessage branch January 22, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants