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

[eas-update] Increase asset upload timeout to 90s and reset on upload retry for slow connections #2085

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

wschurman
Copy link
Member

@wschurman wschurman commented Oct 11, 2023

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.

@github-actions
Copy link

github-actions bot commented Oct 11, 2023

Size Change: +1.49 kB (0%)

Total Size: 42.6 MB

Filename Size Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz 42.6 MB +1.49 kB (0%)

compressed-size-action

@wschurman wschurman force-pushed the @wschurman/slow-connection-wat branch from 644f57c to 0c6c5d4 Compare October 13, 2023 21:07
@github-actions
Copy link

✅ Thank you for adding the changelog entry!

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

Merging #2085 (0c6c5d4) into main (49b6e1f) will decrease coverage by 0.00%.
The diff coverage is 44.45%.

@@            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              
Files Coverage Δ
packages/eas-cli/src/project/publish.ts 76.72% <100.00%> (ø)
packages/eas-cli/src/uploads.ts 14.00% <0.00%> (-0.28%) ⬇️
packages/eas-cli/src/commands/update/index.ts 71.00% <40.00%> (-0.57%) ⬇️

@wschurman wschurman marked this pull request as ready for review October 13, 2023 22:41
@wschurman wschurman requested review from quinlanj and removed request for byCedric, khamilowicz, EvanBacon and szdziedzic October 13, 2023 22:41
@wschurman wschurman merged commit 91787e1 into main Oct 16, 2023
9 checks passed
@wschurman wschurman deleted the @wschurman/slow-connection-wat branch October 16, 2023 17:30
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.

None yet

2 participants