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

ref(spool): Make EnvelopeBuffer a Service #3965

Merged
merged 19 commits into from
Aug 29, 2024
Merged

Conversation

jjbayer
Copy link
Member

@jjbayer jjbayer commented Aug 28, 2024

From the linked ticket:

We decided not to write EnvelopeBuffer as a service because we did not want to put an additional queue in front of it (every service has an inbound queue).

However, since we have multiple request handler threads, we need some kind of synchronization when pushing to the buffer, so in the end we always end up with contention i.e. queueing, whether we queue threads, tokio tasks, or messages. The current implementation spawns a tokio task for every push and synchronizes the buffer with a tokio mutex).

The current solution is in no way better than having a service, so we might as well wrap the envelope buffer in a service and remove the tokio::spawn and mutex.

New data flow:

flowchart TD
    RequestHandler --> |Envelope| BufferService
    BufferService --> |Envelope| ProjectCache 
    ProjectCache --> |"Ready(ProjectKey)"| BufferService
    ProjectCache --> |"NotReady(ProjectKey, Envelope)"| BufferService
Loading

Fixes https://github.com/getsentry/team-ingest/issues/516.

@jjbayer jjbayer force-pushed the ref/buffer-service-2 branch from e5a8eaf to 3101478 Compare August 29, 2024 09:04
@jjbayer jjbayer marked this pull request as ready for review August 29, 2024 09:05
@jjbayer jjbayer requested a review from a team as a code owner August 29, 2024 09:05
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, I like the new select mechanism. Isn't there a tokio primitive that can function in place of wait_for_changes?

@jjbayer jjbayer merged commit aa246e8 into master Aug 29, 2024
24 checks passed
@jjbayer jjbayer deleted the ref/buffer-service-2 branch August 29, 2024 13:12
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