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(store): Multithread the store using a rayon pool #3837

Merged
merged 4 commits into from
Jul 22, 2024

Conversation

Dav1dde
Copy link
Member

@Dav1dde Dav1dde commented Jul 19, 2024

We've seen the store is not being able to keep up if the processor gets parallelized too much. This switches the CPU heavy processing of the store to its own thread pool.

#skip-changelog

@Dav1dde Dav1dde requested a review from a team as a code owner July 19, 2024 08:46
@Dav1dde Dav1dde self-assigned this Jul 19, 2024
@@ -59,8 +60,10 @@ impl Validator {
.map(drop)
}

fn get_schema(&mut self, topic: KafkaTopic) -> Result<Option<&SentrySchema>, SchemaError> {
Ok(match self.schemas.entry(topic) {
fn get_schema(&self, topic: KafkaTopic) -> Result<Option<Arc<SentrySchema>>, SchemaError> {
Copy link
Member Author

Choose a reason for hiding this comment

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

Returns an Arc to keep the critical section short.

@@ -39,13 +40,13 @@ pub enum SchemaError {
#[derive(Debug, Default)]
pub struct Validator {
/// Caches the schema for given topics.
schemas: BTreeMap<KafkaTopic, Option<SentrySchema>>,
schemas: Mutex<BTreeMap<KafkaTopic, Option<Arc<SentrySchema>>>>,
Copy link
Member Author

Choose a reason for hiding this comment

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

Required to make this and the producer Send

@Dav1dde Dav1dde force-pushed the dav1d/store-metric-conc branch 3 times, most recently from b812cfc to 3940224 Compare July 19, 2024 08:57
@@ -332,3 +329,67 @@ impl Producer {
})
}
}

struct Debounced {
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed now because we need a Producer: Send and the atomic impl is a bit more elaborate

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.

See question about WorkerGroup.

relay-server/src/utils/thread_pool.rs Outdated Show resolved Hide resolved
relay-server/src/utils/thread_pool.rs Outdated Show resolved Hide resolved
relay-server/src/utils/thread_pool.rs Outdated Show resolved Hide resolved
Dav1dde and others added 3 commits July 19, 2024 13:22
Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
@Dav1dde Dav1dde enabled auto-merge (squash) July 22, 2024 05:54
@Dav1dde Dav1dde merged commit 4d5941a into master Jul 22, 2024
23 checks passed
@Dav1dde Dav1dde deleted the dav1d/store-metric-conc branch July 22, 2024 06:13
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