Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Easy way to reuse channel #120

Closed
mziccard opened this issue Oct 10, 2016 · 3 comments
Closed

Easy way to reuse channel #120

mziccard opened this issue Oct 10, 2016 · 3 comments
Assignees
Labels
🚨 This issue needs some love. triage me I really want to be triaged.

Comments

@mziccard
Copy link

Following this comment from Garrett I am trying to reuse the same channel to instantiate multiple service objects.

I can do that by creating the channel myself and passing it to ServiceApiSettings.provideChannelWith(ManagedChannel, boolean), this however has several drawbacks:

A method like ServiceApiSettings.setChannelProvider(ChannelProvider) could really help on this. So that I can pass ServiceApiSettings.getChannelProvider() of a previously created settings object.

Related to this issue, the semantics of channel providers created with ServiceApiSettings.createChannelProvider(ConnectionSettings) is a bit confusing to me. The method getOrBuildChannel seems to create a new channel every time it's called, rather than creating it only the first time. Is this behavior intended?

@garrettjonesgoogle
Copy link
Member

It is indeed intentional for getOrBuildChannel() to create a new channel every time it's called. The problem is that the settings class is meant to be stateless and re-usable, but if we keep returning the same channel and that channel gets closed, then that settings object can't be used any more. A related problem is that if the same channel gets passed to multiple API wrapper objects and shouldAutoClose is set to true for two or more of them, then once one API wrapper object gets closed, the other API wrappers will be holding onto a closed channel and will fail any future calls.

Longer term, we will want to use whatever the solution is to grpc/grpc-java#2370 , which is a request to support channel pooling in grpc-java.

For now, maybe this might work:

  • Add a CachedChannelProvider implementation of ChannelProvider, and provide a static method in the API wrapper to create one with service defaults
    • CachedChannelProvider would create a channel on first use, and keep returning the same channel from getOrBuildChannel
    • CachedChannelProvider would return false for shouldAutoClose
    • CachedChannelProvider should probably provide its own close method
  • Add a method ServiceApiSettings.provideChannelWith(ChannelProvider) to allow users to provide it.
CachedChannelProvider channelProvider = PublisherApi.createCachedChannelProvider();
PublisherApi publisher1 = 
    PublisherApi.create(PublisherSettings.newBuilder().provideChannelWith(channelProvider).build());
PublisherApi publisher2 = 
    PublisherApi.create(PublisherSettings.newBuilder().provideChannelWith(channelProvider).build());
... do stuff ...
channelProvider.close();
// don't use publisher1 or publisher2 after this point

What are your thoughts on this option?

@mziccard
Copy link
Author

mziccard commented Oct 26, 2016

It is indeed intentional for getOrBuildChannel() to create a new channel every time it's called.

I see your point but the name rather suggests that the channel is "built" only once and then returned.

What are your thoughts on this option?

Proposed solution sounds fine to me. However, createCachedChannelProvider should take ConnectionSettings, to allow us configuring credentials, endpoint and port. Such method should probably live in ServiceApiSettings.

@garrettjonesgoogle
Copy link
Member

I have implemented a variant of my idea in 3f38463 , so I am resolving this issue now.

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. triage me I really want to be triaged. labels Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

3 participants