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

feat: explore and implement stream management options #962

Closed
4 tasks
chair28980 opened this issue Dec 14, 2023 · 5 comments
Closed
4 tasks

feat: explore and implement stream management options #962

chair28980 opened this issue Dec 14, 2023 · 5 comments
Labels
status-waku-integ All issues relating to the Status Waku integration.

Comments

@chair28980
Copy link
Contributor

chair28980 commented Dec 14, 2023

Problem

Quoting @fryorcraken from discord thread:

Discussing with Prem about status-go queueing message before pushing in light push. I want to highlight some optimisation we did on js-waku for silence labs:

First, note that my understanding and we can double check with libp2p team, is that libp2p stream for req-res protocol are meant to be used once: Alice sends a request over the stream (outbound channel), Bob sends a response over the same stream. stream closes.

Also note that in js-libp2p, by default only one stream per protocol per remote peer is allowed. It is however possible to override this number. Not sure how go-libp2p handles it.

A pattern of

  1. Alice wants to send message
  2. Alice opens stream on existing connection
  3. Alice pushes data

is inefficient.

Suggested solution

in js-waku, we solved that in a few manner:

  1. pre-emptive stream opening: as soon as we connect to a light push/filter/server servide node, we pre emptively open a stream ready to send a message. In the case of go-waku, it may make sense to actually have a pool of streams: feat: pre-emptive stream creation for protocols js-waku#1516

  2. similarly to what status-go does with store, with select the light push node to open stream with based on node latency. we use ipfs/ping to measure latenccy

  3. A change that came later in js-libp2p and should already be present in go-libp2p (not sure if enabled by default) is the optmist stream opening: Implement multistream optimist selection vacp2p/nim-libp2p#746

Where the stream opener does not wait for remote node to ack the stream before pushing data.

Additional Context

Quoting @chaitanyaprem from discord thread:

There are 2 separate points to be addressed/looked at here.

  1. Delay observed when using lightpush. Ref: ⁠Delay in messages using lightpush: bug: delay when using lightpush status-im/status-go#4462

  2. Improvements that can be made in go-waku to manage streams better for req-resp protocols. As of now in go-waku, a new stream is opened whenever a message has to be sent for a req-resp protocol like lightpush. This will have to be optimized further by providing strategies mentioned above. But as of now at least this is not the cause of delay observed, nevertheless this would definitely provide improvements which maybe required at a later stage.

Acceptance Criteria

  • Ensure optimist stream creation from go-libp2p is used
  • pre-emptively open stream for req-res protocols
  • select lowest latency peer for req-res protocols
  • manage a pool of several stresm for req-res protocols
@fryorcraken
Copy link
Collaborator

This is low priority as the gain would be few hundred ms.

@chair28980 chair28980 added the status-waku-integ All issues relating to the Status Waku integration. label Dec 19, 2023
@chaitanyaprem
Copy link
Collaborator

3. A change that came later in js-libp2p and should already be present in go-libp2p (not sure if enabled by default) is the optmist stream opening:

Verified that this is already present in go-libp2p where stream opening doesn't wait for negotiation to complete with the peer.

@chaitanyaprem
Copy link
Collaborator

chaitanyaprem commented Dec 27, 2023

First, note that my understanding and we can double check with libp2p team, is that libp2p stream for req-res protocol are meant to be used once: Alice sends a request over the stream (outbound channel), Bob sends a response over the same stream. stream closes.

This doesn't seem to be the case.

As per explanation here streams are bi-directional channels or abstractions to underlying physical connection(like TCP). This means that streams can be left open by application protocol such as Filter/lightpush/relay for longer duration rather than opening up a new stream everytime a message has to be sent.

Since streams are kind of like logical connections, it would make sense to have stream management done in go-waku that would optimally use streams rather than opening one for each message to be sent.

@chaitanyaprem
Copy link
Collaborator

This can be taken up as part of bindings milestone in nwaku waku-org/pm#121.

Closing this as it is descoped from go-waku.

@chaitanyaprem
Copy link
Collaborator

cc @waku-org/nwaku-developers to consider this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-waku-integ All issues relating to the Status Waku integration.
Projects
Status: Done
Development

No branches or pull requests

3 participants