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.

Sign up for GitHub

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

Jump to bottom

fix: readable stream locked #120

Merged
merged 16 commits into from
Jun 14, 2024
Merged

fix: readable stream locked #120

merged 16 commits into from
Jun 14, 2024

Conversation

bobbyg603
Copy link
Member

@bobbyg603 bobbyg603 commented Jun 10, 2024 •
edited
Loading

Description

  1. Don't fire an unlimited number of promises, this seems to crater node when uploading 1000s of files
  2. When we retry, we need to recreate the stream if we're going to destroy it
  3. Update bugsplat-js-api-client so that we close the teed stream after we're done reading, this alleviates backpressure which was causing us to leak memory
  4. Update bugsplat-js-api-client so that we close the upload read stream when the upload fails
  5. Use proper tmpSubDir to prevent file overwrites

TODO BG

  • Figure out why our temp directory is tmp/!

Checklist

  • Tested manually
  • Unit tests pass with no errors or warnings
  • Documentation updated (if applicable)
  • Reviewed by at least 1 other contributor

@bobbyg603 bobbyg603 requested a review from daveplunkett June 10, 2024 13:30
bobbyg603
bobbyg603 commented Jun 13, 2024
View reviewed changes
src/upload.ts Outdated Show resolved Hide resolved
@bobbyg603 bobbyg603 mentioned this pull request Jun 13, 2024
Closed
@bobbyg603
fix: don't fire uploads promises all at once (#123)
180df63 
* fix: await worker promises instead of firing them all off

* fix: max workers should be cpus minus one for main thread
@bobbyg603 bobbyg603 changed the title fix: readable stream locked on retry fix: readable stream locked Jun 14, 2024
@bobbyg603 bobbyg603 merged commit 68c9cf6 into main Jun 14, 2024
3 checks passed
@bobbyg603 bobbyg603 deleted the fix-stream-locked branch June 14, 2024 21:45
@bobbyg603 bobbyg603 mentioned this pull request Jun 14, 2024
Closed
daveplunkett
daveplunkett reviewed Jun 16, 2024
View reviewed changes
Copy link
Member

@daveplunkett daveplunkett left a comment

Choose a reason for hiding this comment

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

Approved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daveplunkett daveplunkett daveplunkett approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bobbyg603 @daveplunkett