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

segmentChangesUpdater - limit concurrency #254

Open
wants to merge 8 commits into
base: development
Choose a base branch
from

Conversation

scottbessler
Copy link

@scottbessler scottbessler commented Oct 2, 2023

Javascript commons library

What did you accomplish?

Segment fetching was currently doing a Promise.all across every segment on startup. This would result in [# of segments] simultaneous IO requests, which can cause troubles for node. It is better to ensure a max # of concurrent requests.

  • converted the segmentChangesUpdater to use async/await syntax instead of promise chains
  • updated .editorconfig to include quote preference
  • made the segment fetching happen in chunks of 10 rather than all at once
  • added configuration option numConcurrentSegmentFetches and defaulted it to 10

How do we test the changes introduced in this PR?

  • ensure segment syncing still works

Extra Notes

@scottbessler scottbessler requested a review from a team as a code owner October 2, 2023 21:06
@scottbessler scottbessler changed the title segmentChangesUpdated convert to async/await syntax segmentChangesUpdater - limit concurrency Oct 2, 2023
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.

1 participant