dedis / onet

Overlay Network for distributed protocols
GNU Lesser General Public License v3.0
50 stars 29 forks source link

What is the use case for BidirectionalStreamer #670

Open tharvik opened 3 years ago

tharvik commented 3 years ago

I'm refactoring the streaming capabilities of onet. Now, I'm having issues understanding what is the need for a bidirectional stream. Which real use case cannot be solved by opening a new stream for each client input?

I was thinking of removing the bidirectionnal support, and having the streaming calls returning a new connection/channel for each.

ineiti commented 3 years ago

@nkcr added this for the columbus explorer. I got to admit that I didn't follow very closely the use-case, so it might be possible to optimize it. However, as far as I know, it's in use, so it shouldn't be removed!

nkcr commented 3 years ago

I am using this functionality for the PaginateBlock streaming handler. Bi-directional streamer should be supported since we are using websocket (if not why using a full-duplex communication protocol ?).

tharvik commented 3 years ago

Bi-directional streamer should be supported since we are using websocket (if not why using a full-duplex communication protocol ?).

Still, I don't quite see an actual use case for that yet.

But we can also support it, and for that, we should give more control of the stream to the service, such as calling it with a recv-channel of messages, not calling it on each message. That way, the service can actually handle multiple message without keeping the state inside the service's struct (and we all know how it goes when using a changing shared state...).

Using a recv-channel for incoming message does make the implementation easier: closing the channel means the client closed the connection, no need for an explicit "service stop channel". That actually solve most of the points I raised.

I am using this functionality for the PaginateBlock streaming handler.

If I'm comparing it with TestServiceProcessor_ProcessClientStreamRequest, it is incorrectly implementing a streaming handler, as returned channels are created each time the function is called but from the test, it should return the same channels for a given client if some exchange is already in progress.

So we agree that at least the documentation/tests should be updated :)