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(pool): Use new AsyncPool for the processor and store services #4520

Merged
merged 15 commits into from
Mar 3, 2025

Conversation

iambriccardo
Copy link
Member

@iambriccardo iambriccardo commented Feb 24, 2025

This PR integrates the new AsyncPool in the EnvelopeProcessorService and StoreService.

Please note that currently the integration doesn't leverage the benefits of async, since all the functions called by the pool are blocking. The support for async in the I/O operations of the EnvelopeProcessorService will be coming.

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

@iambriccardo iambriccardo changed the title riccardo/feat/new async pool integration feat(pool): Use new AsyncPool for the processor and store services Feb 24, 2025
@iambriccardo iambriccardo changed the title feat(pool): Use new AsyncPool for the processor and store services feat(pool): Use new AsyncPool for the processor and store services Feb 24, 2025
Copy link

codecov bot commented Feb 24, 2025

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
4 1 3 0
View the top 1 failed test(s) by shortest run time
_integration-test.test_01_basics::test_receive_event
Stack Traces | 65.4s run time
client_login = (<httpx.Client object at 0x7faa689a6930>, <Response [200 OK]>)

    #x1B[0m#x1B[94mdef#x1B[39;49;00m #x1B[92mtest_receive_event#x1B[39;49;00m(client_login):#x1B[90m#x1B[39;49;00m
        event_id = #x1B[94mNone#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
        client, _ = client_login#x1B[90m#x1B[39;49;00m
        #x1B[94mwith#x1B[39;49;00m sentry_sdk.init(dsn=get_sentry_dsn(client)):#x1B[90m#x1B[39;49;00m
            event_id = sentry_sdk.capture_exception(#x1B[96mException#x1B[39;49;00m(#x1B[33m"#x1B[39;49;00m#x1B[33ma failure#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m))#x1B[90m#x1B[39;49;00m
        #x1B[94massert#x1B[39;49;00m event_id #x1B[95mis#x1B[39;49;00m #x1B[95mnot#x1B[39;49;00m #x1B[94mNone#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
>       response = poll_for_response(#x1B[90m#x1B[39;49;00m
            #x1B[33mf#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m#x1B[33m{#x1B[39;49;00mSENTRY_TEST_HOST#x1B[33m}#x1B[39;49;00m#x1B[.../sentry/internal/events/#x1B[39;49;00m#x1B[33m{#x1B[39;49;00mevent_id#x1B[33m}#x1B[39;49;00m#x1B[33m/#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m, client#x1B[90m#x1B[39;49;00m
        )#x1B[90m#x1B[39;49;00m

#x1B[1m#x1B[31m_integration-test/test_01_basics.py#x1B[0m:129: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

request = 'http://localhost:.../internal/events/bfa1bc914f3442bb86b695395287d56a/'
client = <httpx.Client object at 0x7faa689a6930>, validator = None

    #x1B[0m#x1B[94mdef#x1B[39;49;00m #x1B[92mpoll_for_response#x1B[39;49;00m(#x1B[90m#x1B[39;49;00m
        request: #x1B[96mstr#x1B[39;49;00m, client: httpx.Client, validator: Callable = #x1B[94mNone#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
    ) -> httpx.Response:#x1B[90m#x1B[39;49;00m
        #x1B[94mfor#x1B[39;49;00m i #x1B[95min#x1B[39;49;00m #x1B[96mrange#x1B[39;49;00m(TIMEOUT_SECONDS):#x1B[90m#x1B[39;49;00m
            response = client.get(#x1B[90m#x1B[39;49;00m
                request, follow_redirects=#x1B[94mTrue#x1B[39;49;00m, headers={#x1B[33m"#x1B[39;49;00m#x1B[33mReferer#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m: SENTRY_TEST_HOST}#x1B[90m#x1B[39;49;00m
            )#x1B[90m#x1B[39;49;00m
            #x1B[94mif#x1B[39;49;00m response.status_code == #x1B[94m200#x1B[39;49;00m:#x1B[90m#x1B[39;49;00m
                #x1B[94mif#x1B[39;49;00m validator #x1B[95mis#x1B[39;49;00m #x1B[94mNone#x1B[39;49;00m #x1B[95mor#x1B[39;49;00m validator(response.text):#x1B[90m#x1B[39;49;00m
                    #x1B[94mbreak#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
            time.sleep(#x1B[94m1#x1B[39;49;00m)#x1B[90m#x1B[39;49;00m
        #x1B[94melse#x1B[39;49;00m:#x1B[90m#x1B[39;49;00m
>           #x1B[94mraise#x1B[39;49;00m #x1B[96mAssertionError#x1B[39;49;00m(#x1B[90m#x1B[39;49;00m
                #x1B[33m"#x1B[39;49;00m#x1B[33mtimeout waiting for response status code 200 or valid data#x1B[39;49;00m#x1B[33m"#x1B[39;49;00m#x1B[90m#x1B[39;49;00m
            )#x1B[90m#x1B[39;49;00m
#x1B[1m#x1B[31mE           AssertionError: timeout waiting for response status code 200 or valid data#x1B[0m

#x1B[1m#x1B[31m_integration-test/test_01_basics.py#x1B[0m:40: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@iambriccardo iambriccardo requested a review from Dav1dde February 24, 2025 15:22
@iambriccardo iambriccardo marked this pull request as ready for review February 24, 2025 15:22
@iambriccardo iambriccardo requested a review from a team as a code owner February 24, 2025 15:22
@iambriccardo iambriccardo force-pushed the riccardo/feat/new-async-pool-integration branch from 8408e01 to d2b7601 Compare February 24, 2025 16:46
Comment on lines 155 to 160
// We report how many tasks are being concurrently polled in this future.
relay_statsd::metric!(
gauge(AsyncPoolGauges::AsyncPoolFuturesPerThread) = this.tasks.len() as u64,
pool_name = &this.pool_name,
thread_name = &this.thread_name
);
Copy link
Member

Choose a reason for hiding this comment

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

this is probably too high cardinality, maybe we just emit the total size somewhere or the max?

Also we don't have to do it on each poll only when the futures length changes (e.g. on push)

@iambriccardo iambriccardo requested a review from Dav1dde February 25, 2025 12:23
@iambriccardo iambriccardo enabled auto-merge (squash) March 3, 2025 08:10
@iambriccardo iambriccardo merged commit 0f1167f into master Mar 3, 2025
22 checks passed
@iambriccardo iambriccardo deleted the riccardo/feat/new-async-pool-integration branch March 3, 2025 08:18
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