[eas-update] Increase asset upload timeout to 90s and reset on upload retry for slow connections #2085
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
|
Size Change: +1.49 kB (0%) Total Size: 42.6 MB
|
… retry for slow connections
wschurman
force-pushed
the
@wschurman/slow-connection-wat
branch
from
644f57c to
0c6c5d4
Compare
|
✅ Thank you for adding the changelog entry! |
Codecov Report
@@ Coverage Diff @@
## main #2085 +/- ##
==========================================
- Coverage 54.07% 54.07% -0.00%
==========================================
Files 508 508
Lines 18669 18673 +4
Branches 3735 3735
==========================================
+ Hits 10094 10095 +1
- Misses 8554 8557 +3
Partials 21 21
|
wschurman
requested review from
EvanBacon,
byCedric,
szdziedzic and
khamilowicz
as code owners
wschurman
requested review from
quinlanj
and removed request for
byCedric,
khamilowicz,
EvanBacon and
szdziedzic
quinlanj
approved these changes
Oct 16, 2023
1 task
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.
Why
For slow connections that take a while to upload, 60s is not enough of a timeout. For example, lets say there's only one asset to upload, and it takes even 1 second to upload and 60s to process (the max of the cloud function), it'll time out. The p99 of the processing cloud function is 29s, so changing this timeout to 90s gives a upload timeout of 60s.
We also change the semantics of when to reset the timeout, and now reset it when the upload is retried. So, in the example above, let's say the asset takes 59 seconds to upload, but fails at the 58th second, it will be retried and the timeout will reset to give it an additional 60 seconds to upload.
Test Plan
This is tough to test. Would need to simulate a super slow connection and some failures.
Tested manually by uploading 5k assets (1 failed randomly, not sure why, but lucky that it did and timeout still worked) and seeing that the timeout still worked.