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

feat(spooler): Implement backpressure in spooler via bounded queues #4052

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

iambriccardo
Copy link
Member

@iambriccardo iambriccardo commented Sep 20, 2024

This PR implements a backpressure mechanism in the spooler which uses a bounded buffer of size x. The bounded buffer is used exclusively for sending envelopes to the project cache.

Closes: https://github.com/getsentry/team-ingest/issues/541

#skip-changelog

Copy link
Member

@jjbayer jjbayer left a comment

Choose a reason for hiding this comment

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

Looks good! Some minor comments

relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/service.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Outdated Show resolved Hide resolved
relay-server/src/services/buffer/mod.rs Show resolved Hide resolved
@iambriccardo iambriccardo marked this pull request as ready for review September 20, 2024 11:42
@iambriccardo iambriccardo requested a review from a team as a code owner September 20, 2024 11:42
@@ -955,6 +960,9 @@ pub struct EnvelopeSpool {
/// internal page stats.
#[serde(default = "spool_disk_usage_refresh_frequency_ms")]
disk_usage_refresh_frequency_ms: u64,
/// The amount of envelopes that can be put in the bounded buffer.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// The amount of envelopes that can be put in the bounded buffer.
/// The amount of envelopes that the envelope buffer can push to its output queue.

@@ -1489,6 +1483,11 @@ impl Service for ProjectCacheService {
broker.handle_periodic_unspool()
})
}
Some(message) = envelopes_rx.recv() => {
metric!(timer(RelayTimers::ProjectCacheTaskDuration), task = "handle_envelope", {
Copy link
Member

Choose a reason for hiding this comment

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

Could you also log a metric for the current size of the bounded queue somewhere (e.g. in the ticker)?

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