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

relay: remove listener streams in client::Transport #2782

Open
elenaf9 opened this issue Jul 29, 2022 · 1 comment
Open

relay: remove listener streams in client::Transport #2782

elenaf9 opened this issue Jul 29, 2022 · 1 comment

Comments

@elenaf9
Copy link
Contributor

elenaf9 commented Jul 29, 2022

Description

With #2652 the concept of listeners was removed from the Transport in the sense that having listener streams is not enforced anymore. Instead the transport is responsible for managing its listeners and polling them.

As part of #2652 the relay ClientTransport was adjusted to the trait changes so that now the ClientTransport manages and drives its listener streams. The underlying architecture was not changed and the concept of listeners remains internally.
How it currently works is that:

  • for each relayed listening address, we create a new Listener stream and drive it for events
  • communication from the Behaviour to the ClientTransport (necessary for informing the Listener of the result of a reservation and of incoming relayed connections) is done via a channel from the Handler (of the connection to the relay) to the Listener
  • said channel is created in the ClientTransport, then sent via a channel from transport to behaviour, and then passed from behaviour to the handler.

With the transport changes of #2652, we may be able to reduce the complexity here by removing the listener streams.

The client transport must be aware of the different listeners, but it may not need to drive an extra stream for each listener.
One idea would be to just have one channel between behaviour and transport for each direction and track the necessary states directly in the ClientTransport and Behaviour.
I originally considere to do something like this in #2652, but eventually decided against it because of the reasons described in #2652 (comment).

However, I did not look into it in too much detail, and I think it is still worth exploring. Maybe @mxinden you can also give some input here as the original author of the relay_v2 protocol.

@mxinden
Copy link
Member

mxinden commented Aug 2, 2022

Thanks for documenting this!

Maybe @mxinden you can also give some input here as the original author of the relay_v2 protocol.

I am in favor of the above suggestion. That said, I haven't played through it in detail myself, thus I can't comment whether it will turn out to be fruitful.

@thomaseizinger thomaseizinger changed the title protocols/relay: Reconsider listener streams in ClientTransport relay: remove listener streams in ClientTransport Oct 1, 2023
@thomaseizinger thomaseizinger changed the title relay: remove listener streams in ClientTransport relay: remove listener streams in client::Transport Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants