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

Expose DynamicStreamService #1231

Open
conradludgate opened this issue Jan 11, 2023 · 3 comments
Open

Expose DynamicStreamService #1231

conradludgate opened this issue Jan 11, 2023 · 3 comments

Comments

@conradludgate
Copy link
Contributor

Feature Request

Crates

tonic

Motivation

In TrueLayer/ginepro#39 we discovered that tonic's Channel::balance_channel only does random distribution.

We would like to experiment with enabling other Load implementations in the discovery in tower::Balance, but this requires us to re-implement Connection

Proposal

Make a type that has a generic parameter of

D: Discover<Key = K, Service = Endpoint>,
D::Error: Into<tonic::Error>

Such that it returns a stream of Connection. This would require Connection to be stabilised, but no methods to be exposed apart from Service and Load.

DynamicServiceStream<K: Hash + Eq + Clone> is then just a type alias for this, taking a channel Receiver as the Discover impl (potentially with a .map(Ok) on the stream).

Alternatives

Expose Connection as a fully stable type with stable methods to construct the connection from a hyper connection.

@LucioFranco
Copy link
Member

Would it make sense for ginepro to just use tower balance directly and to implement some of the transport features? I'd like to nix all of the transport module in the future so I am weary of adding new things to it.

@conradludgate
Copy link
Contributor Author

Would it make sense for ginepro to just use tower balance directly

That's the current plan

and to implement some of the transport features

And that's what I was trying to avoid 😅

I don't mind vendoring it, but it feels like more points of failure.

@LucioFranco
Copy link
Member

I know its not ideal but I think ginepro could be a good place to have a load balanced channel impl of its own that kinda follows what tonic does. I hope in the future I we can 1) make tower easier to config so you can make these changes 2) provide a better thick client that isn't grpc specific but has more config options. Basically extract what we have now but build it better lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants