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

PDS: handle S3 upload timeout more gracefully #2429

Merged
merged 3 commits into from
Apr 19, 2024
Merged

Conversation

devinivy
Copy link
Collaborator

This makes upload timeouts to S3 handled a bit more consistently, and surfaces them to the user in a more legible way.

  • Support a config on S3BlobStore for upload timeouts.
  • Apply S3BlobStore timeouts to both putTemp() and putPermanent().
  • Clear upload timeouts more reliably (e.g. in cases where upload fails for some reason other that the timeout).
  • Add PDS config PDS_BLOBSTORE_S3_UPLOAD_TIMEOUT_MS, defaulting to 20000. The previous timeout was 10sec which was too short in enough cases that it was worth increasing to 20sec.
  • Surface better error to user when repo.uploadBlob endpoint fails due to a timeout.

const abortController = new AbortController()
const timeout = setTimeout(
() => abortController.abort('upload timed out'),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The aws-sdk was throwing the reason passed to abort() away, which was actually a little confusing, so I nixed the message/reason here.

Copy link
Collaborator

@pfrazee pfrazee left a comment

Choose a reason for hiding this comment

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

Cool this lgtm

Copy link
Collaborator

@dholms dholms left a comment

Choose a reason for hiding this comment

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

looks great 👍

@devinivy devinivy merged commit 1faf634 into main Apr 19, 2024
10 checks passed
@devinivy devinivy deleted the pds-s3-upload-timeout branch April 19, 2024 22:29
estrattonbailey added a commit that referenced this pull request Apr 29, 2024
* origin/main: (23 commits)
  Feed context & sendInteractions impl (#2402)
  Pass through topics headers (#2447)
  Version packages (#2446)
  ✨ Allow muting reporter (#2390)
  Version packages (#2443)
  Add `savedFeedsPrefV2` and new methods (#2427)
  fix(ozone): properly import "lande" ES module (from CJS) (#2441)
  Appview: remove replies to blocked posts from feeds (#2430)
  PDS: handle S3 upload timeout more gracefully (#2429)
  Appview: maintain language info when going out to a suggestions service (#2424)
  Appview: ensure hydration context tracks viewer did and not full service ref (#2423)
  ✨ Detect language from record content if lang property is not set (#2301)
  Version packages (#2422)
  Add email auth factor tools to API (#2419)
  Version packages (#2417)
  Lexicons: email auth factor (#2416)
  Suggestions skeleton impl (#2403)
  getSuggestionsSkeleton lexicon (#2399)
  Version packages (#2401)
  Appview: support new post search params, viewer context in search (#2409)
  ...
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.

3 participants