-
Notifications
You must be signed in to change notification settings - Fork 42
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(filter): reliability monitor as a separate class to handle reliability logic #2117
Conversation
size-limit report 📦
|
f36812f
to
f387f59
Compare
@@ -0,0 +1,119 @@ | |||
import type { Peer, PeerId } from "@libp2p/interface"; |
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.
since it is a new entity - can you, please, add UT for it? it can be a follow up PR
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.
let's, please, check how it works in dogfooding app
|
…-reliability-split
@danisharora099 just to double check is this PR main and #2127 should be dropped or vice versa? |
We will use this PR instead. |
@@ -195,6 +196,7 @@ export class WakuNode implements Waku { | |||
} | |||
|
|||
public async stop(): Promise<void> { | |||
ReliabilityMonitorManager.destroyAll(); |
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.
nit: to disposal
this.monitorManager = ReliabilityManager.create();
this.reliabilityMonitor = this.monitorManager.create();
this.filter = new Filter (this.reliabilityManager);
...
stop() {
this.monitorManager.dispose();
}
5ae69f0
to
2eddaf5
Compare
…-reliability-split
Problem
Based on #2075, the logic to detect messages that were missed from the Filter protocol were being detected inside of the Filter SDK implementation. This leads to a tight couple of primary objective of FilterSDK, and reliability goals.
This obstacle also extends to adding further actions that might be taken with additional protocols such as Store pings, and LightPush retries for certain messages.
Solution
Decouple reliability logic into a new ReliabilityMonitor class.
Notes
Contribution checklist:
!
in title if breaks public API;