-
Notifications
You must be signed in to change notification settings - Fork 104
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
feat: two explicit queues for session work #538
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self review. ready for @achingbrain
}) | ||
queue.addEventListener('error', () => {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if adding this empty error handler is necessary?
run benchmarks to see if regression or improvement. |
Title
feat: two explicit queues for session work
Description
This PR originally aimed to fix the infinite loop we hit when we were only getting a single provider (this only occurring in test envs that i'm aware of, but certainly possible elsewhere). However, it's now been updated to refactor the provider finding and querying logic so that it's a little more obvious what is happening.
Notes & open questions
A few main things that change with this PR:
findProviders
task when findProviders is done (idle
event)provider
event, or when an existing session is queried with a new cid (we will query any currently known and non-evicted providers).provider
events nowfound < count
logic to determine if we found enough providers, or throw an errorfailure
event), we set a flag indicating that we failed to find more providers.idle
will handle when triggered.idle
retrieve
promiseretrieve
promise: new error that we're done querying providers and could not get the blockChange checklist