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

Create a executorService with a separate pool of threads for channelpool #836

Conversation

tonytanger
Copy link
Contributor

Refreshing ChannelPool shares the executorService with gRPC. This could lead to undesirable behaviour. If it takes a while (a few seconds) to refresh a channel and many channels are being refreshed at the same time, it could occupy all the threads in the executorService and starve gRPC from using them.

Proposed is to create a separate executorService with its own pools of threads. It shouldn't need many threads to refresh the channels.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 18, 2019
@codecov
Copy link

codecov bot commented Dec 18, 2019

Codecov Report

Merging #836 into master will decrease coverage by 0.11%.
The diff coverage is 52.63%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #836      +/-   ##
===========================================
- Coverage     78.72%   78.6%   -0.12%     
- Complexity     1146    1149       +3     
===========================================
  Files           202     202              
  Lines          5099    5113      +14     
  Branches        405     410       +5     
===========================================
+ Hits           4014    4019       +5     
- Misses          912     921       +9     
  Partials        173     173
Impacted Files Coverage Δ Complexity Δ
...api/gax/grpc/InstantiatingGrpcChannelProvider.java 79.89% <100%> (-0.22%) 35 <0> (ø)
...main/java/com/google/api/gax/grpc/ChannelPool.java 47.61% <50%> (-1.32%) 14 <2> (+3)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68ae22d...877bfa8. Read the comment docs.

Copy link
Contributor

@igorbernstein2 igorbernstein2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@igorbernstein2 igorbernstein2 merged commit f18673a into googleapis:master Jan 6, 2020
@kolea2 kolea2 mentioned this pull request Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants