Skip to content

Conversation

@Galoretka
Copy link
Contributor

AllTransactions metrics were updated on inserts but could remain stale after removals and evictions. Several removal paths invoked only TxPool::update_size_metrics, which updates sub-pool gauges but not AllTransactions gauges, and discard_worst did not update metrics at all. This change makes TxPool::update_size_metrics also refresh AllTransactions metrics, invokes update_size_metrics at the end of discard_worst, and updates AllTransactions metrics inside its removal methods. The result is consistent metrics for both sub-pools and the AllTransactions set across insertion, removal, and eviction flows without relying on incidental future inserts to refresh gauges. Tests cover remove-by-hash and eviction flows.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense in general, but I don't think we need all the additional calls then?

]
);

self.update_size_metrics();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't do this here because there's no need to update the metrics here

self.metrics.blob_pool_transactions.set(stats.blob as f64);
self.metrics.blob_pool_size_bytes.set(stats.blob_size as f64);
self.metrics.total_transactions.set(stats.total as f64);
self.all_transactions.update_size_metrics();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we add this here, why do we need to also call this in the other locations

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

2 participants