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

GAX-JAVA: Allow users to customize ChannelPoolSettings RESIZE_INTERVAL and MAX_RESIZE_DELTA #2746

Open
lkleeven opened this issue May 7, 2024 · 3 comments
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@lkleeven
Copy link

lkleeven commented May 7, 2024

We are creating our own in company Google Pub/Sub client. To improve performance we've recently switched to using the InstantiatingGrpcChannelProvider. This already gave a performance improvement, however to be able to deal with big spikes of load, we'd like to be able to also set the resize interval and max resize delta, currently present as package private static fields in ChannelPoolSettings. This would allow us to set clients up to respond faster to big spikes in load.

We've considered keeping a higher minimum channel count open, but that feels rather wasteful, especially if the spikes are few and far between.

We are willing to contribute if that helps to speed things up. And are also open to suggestions on how to otherwise deal with these load spikes from a client perspective.

@mpeddada1 mpeddada1 added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. labels May 8, 2024
@blakeli0
Copy link
Collaborator

Hi @lkleeven, thanks for reporting this! I think your request is reasonable, but we may not expose these two settings at this moment. Because

  1. Channel resizing is expensive and could be error prone if misconfigured. The settings you mentioned were not exposed on purpose because we want the channel resizing process to have minimum impact on the normal operations, as vast of majority of clients are good with just 1 channel. If RESIZE_INTERVAL is exposed and it is set to something like 5 seconds, it may actually hurt the performance.
  2. Channel pool was designed for moderate spikes(~2x), not big spikes(10x). If there is a big spike, I would first consider optimizing in other ways like adding more machines. If you are stuck with limited machines and the big spike is predictable, one possible way you can do in code is to create another client with a different number of channels.

That being said, if you still think this feature is of great value, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response and proper prioritization.

@lkleeven
Copy link
Author

lkleeven commented Jun 4, 2024

Hi @blakeli0, thanks for the reply. I understand that these are settings that can have a big impact and users should be careful using it. But I'd think that could be addressed by adding the appropriate warnings in the documentation/Javadoc.

Now you mention that it's not meant to be used for to catch bigger spikes and those should be handled with more machines or adding another client. But if we add another client, won't we be also creating those channels which are expensive? I ask because scaling up with more machines isn't always as easy for our users and we've heard that they usually still have enough power on their machines to add more clients/channels.

@blakeli0
Copy link
Collaborator

blakeli0 commented Jun 4, 2024

if we add another client, won't we be also creating those channels which are expensive?

Yes creating channels are expensive, and it is unavoidable either we resize channels in the same client or create a new client. But the process that checks for resizing is not a cheap operation either, which may hurt the performance if configured too frequent.
Also the additional client can be closed after the spike is done, which should help free up the resources.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

3 participants