Skip to content
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

fix(buffer): Optimistically initialize stacks as ready #4046

Merged
merged 3 commits into from
Sep 19, 2024

Conversation

jjbayer
Copy link
Member

@jjbayer jjbayer commented Sep 19, 2024

When a new stack is created, the buffer does not know the corresponding project state(s). It previously assumed "not ready", which means it sent a UpdateState message to the project state on the first envelope.

In practice, the vast majority is created when a project state is ready and the stack was previously removed from the priority queue because it was empty. This PR flips the default to "ready", which means that on startup the project cache will return NotReady once.

#skip-changelog

relay_statsd::metric!(counter(RelayCounters::BufferEnvelopesReturned) += 1);
self.push(buffer, envelope).await;
buffer.mark_ready(&project_key, false);
Copy link
Member Author

Choose a reason for hiding this comment

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

This was a pre-existing bug: Calling mark_ready on an empty stack has no effect, so we should call it after putting back the envelope.

@jjbayer jjbayer marked this pull request as ready for review September 19, 2024 11:54
@jjbayer jjbayer requested a review from a team as a code owner September 19, 2024 11:54
Copy link
Member

@iambriccardo iambriccardo left a comment

Choose a reason for hiding this comment

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

LGTM

@jjbayer jjbayer merged commit 3533a47 into master Sep 19, 2024
24 checks passed
@jjbayer jjbayer deleted the fix/spans-optimistic-readiness branch September 19, 2024 12:06
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