refactor(s3-batch-exports): Swap to asyncio s3 client #17673
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Temporal activities are not heartbeating. I believe this is because the heartbeat is spawning an async task, but since our code is not doing any await, we never yield the main thread for it to run the async task. By swapping to aioboto3 we now await on each upload part which should allow us to heartbeat while the part is uploaded.
tomasfarias
force-pushed
the
fix/attempt-heartbeating-via-async-function
branch
from
16e5434 to
df377a0
Compare
tomasfarias
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
We are still not heartbeatting even after #17642 and #17670. I believe this is because the heartbeat is spawning an async task, but since our code is not doing any
await-ing, we never yield the main thread for it to run the async task.By swapping to aioboto3 we now await on each upload part which should allow us to heartbeat while the part is uploaded.
Changes
aioboto3 is mostly compatible with the boto3 interface with 1 big difference: client must be initialized as a context manager. Besides that, I mostly added
async/awaitin every call.In order for aioboto3 to be compatible with our existing boto3 dependency, I've had to bump it slightly to 1.26.76 from 1.26.66. We are taking steps to split up the temporal bits of code to avoid conflicting with the django monolith. In the meantime, I hope this bump won't cause issues. Reviewing the changelog reveals no significant changes between versions.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Next steps
Add heartbeating to every destination
Use asyncio (aiofiles) for
BatchExportTemporaryFile.How did you test this code?
Updated unit tests to use async code, which is just more of the same: Use context manager, and add
async/awaiteverywhere.