-
Notifications
You must be signed in to change notification settings - Fork 248
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
bug: optimize filter registrations to avoid some issues noticed #5262
Comments
Oh, this is interesting for js-waku indeed. I noticed something similar when working on introducing peer management for LightPush, but this is directly applicable for Filter as well: I'm curious: in your approach where you queue these requests, do you block the initial library consumer's function invocation, of for example the cc @weboko |
I did not think of reporting failure back to caller as of now. But rather i am expecting SDK to keep retrying internally. |
Filter subscription async and queuing subscriptions when no connectivity is there(Points 1 & 4 above), are implemented as part of 817e959 Others will be taken up in a separate PR. |
Batching of subscriptions may not be required unless we start seeing an issue. The only possible challenge i see at this point with not batching them could be in case the serviceNode starts rate-limiting filter-subscribe requests which would then start failing. |
This is being taken up now since it is causing delay and bloat of subscriptions. |
Closing as completed as part of #5440 |
Problem
While dogfooding #4665 with status-desktop, i had noticed few issues that need to be addressed.
Implementation
Following approach would solve both the problems and also address the issue #4526
Solution is as below:
Acceptance Criteria
Need to validate all Filter test cases along with dogfooding lightClient.
cc @richard-ramos @vitvly @cammellos : Do review the above and provide any feedback/suggestions.
@weboko @danisharora099 : This might be something that is useful in js-waku Filter-SDK as well.
The text was updated successfully, but these errors were encountered: