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

Remove multistream.transport as an entity of type network.multiplexer #1854

Closed
Tracked by #1789
julian88110 opened this issue Nov 3, 2022 · 2 comments · Fixed by #1885
Closed
Tracked by #1789

Remove multistream.transport as an entity of type network.multiplexer #1854

julian88110 opened this issue Nov 3, 2022 · 2 comments · Fixed by #1885
Assignees

Comments

@julian88110
Copy link
Contributor

julian88110 commented Nov 3, 2022

In the process of building the integration test for early muxer negotiation #1836, this enhancement came into being as suggested by @marten-seemann while we discussed the options of making the implementation of #1851 cleaner.

The existence of multistream.transport as a network.multiplexer type becomes an obvious obstacle of sharing muxer state info internally, especially in the concurrent context which may require locking to avoid data race.

This issue ticket proposes to get rid of the multistream.transport entity and merge its function into the upgrader. This solution may require fair amount of refactoring of the config and initialization code too, a lot of test code will also be affected, but in long term it should be worth the effort for simplified upgrade logic and cleaner code structure. There is no reason the multistream.transport should be a network.Transport type and be restricted to that interface, which becomes awkward in facilitating new features such as early muxer negotiation.

@BigLep
Copy link
Contributor

BigLep commented Nov 7, 2022

@julian88110 : lets reference #1851 (comment) in the description.

@julian88110
Copy link
Contributor Author

julian88110 commented Nov 7, 2022

@julian88110 : lets reference #1851 (comment) in the description.

Good point. updated the description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment