Skip to content

Conversation

@toidiu
Copy link
Contributor

@toidiu toidiu commented Jan 30, 2026

@toidiu toidiu requested a review from a team as a code owner January 30, 2026 20:11
…ure being cancelled.

This PR adds a reusable Token to track when an operation is complete or
report the failure via a metric. The flush operation is part of a
`select!` call and can therefore be cancelled if another future
completes first. We want to track how often this happens.
@toidiu toidiu force-pushed the akothari/track_flush_complete branch from 8e53bd4 to 5815d5b Compare January 30, 2026 20:14
/// WouldBlock and are retried in a loop.
fn send_to_wouldblock_duration_s(&self) -> TimeHistogram;

/// Number of flush operations that were incomplete due to the future being
Copy link
Contributor

@evanrittenhouse evanrittenhouse Jan 30, 2026

Choose a reason for hiding this comment

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

Suggested change
/// Number of flush operations that were incomplete due to the future being
/// Number of mid-handshake flush operations that were skipped due to future cancellation.

Same for below


/// Number of flush operations that were incomplete due to the future being
/// canceled.
fn incomplete_flush_operation_count(&self) -> Counter;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn incomplete_flush_operation_count(&self) -> Counter;
fn skipped_mid_handshake_flush_count(&self) -> Counter;

impl<M: Metrics, F: Fn(&M)> TrackCompleteToken<M, F> {
fn new(metrics: M, f: F) -> Self {
Self {
complete: AtomicBool::from(false),
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be atomic?

Also, this is currently only used for incrementing one counter. It may be clearer to just pass in the counter and increment it on drop rather than register a callback.

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