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

Shutdown GRPC managed channel properly on close #628

Merged
merged 4 commits into from
Jan 10, 2019
Merged

Shutdown GRPC managed channel properly on close #628

merged 4 commits into from
Jan 10, 2019

Conversation

ajaaym
Copy link
Contributor

@ajaaym ajaaym commented Dec 26, 2018 •
edited
Loading

Fix #3912, 3693 & 3648 Shutdown GRPC Managed channel properly on closing of background resources.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 26, 2018
@codecov-io
Copy link

codecov-io commented Dec 26, 2018 •
edited
Loading

Codecov Report

Merging #628 into master will decrease coverage by 0.13%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #628      +/-   ##
============================================
- Coverage     74.94%   74.81%   -0.14%     
- Complexity      935      936       +1     
============================================
  Files           177      178       +1     
  Lines          4099     4117      +18     
  Branches        328      329       +1     
============================================
+ Hits           3072     3080       +8     
- Misses          874      884      +10     
  Partials        153      153
Impacted Files Coverage Δ Complexity Δ
.../com/google/api/gax/grpc/GrpcTransportChannel.java 59.09% <0%> (-13.14%) 9 <0> (ø)
...om/google/api/gax/core/ResourceCloseException.java 0% <0%> (ø) 0 <0> (?)
...va/com/google/api/gax/retrying/RetryAlgorithm.java 75% <0%> (-10.72%) 7% <0%> (ø)
.../google/api/gax/retrying/TimedAttemptSettings.java 100% <0%> (ø) 2% <0%> (ø) ⬇️
...ogle/api/gax/retrying/StreamingRetryAlgorithm.java 0% <0%> (ø) 0% <0%> (ø) ⬇️
...le/api/gax/retrying/ExponentialRetryAlgorithm.java 97.56% <0%> (+0.12%) 10% <0%> (ø) ⬇️
...api/gax/grpc/InstantiatingGrpcChannelProvider.java 66.25% <0%> (+0.87%) 27% <0%> (+1%) ⬆️

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 1d492dc...6a57417. Read the comment docs.

sduskis
sduskis reviewed Dec 26, 2018
View reviewed changes
gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcTransportChannel.java Outdated
@@ -91,6 +91,13 @@ public boolean awaitTermination(long duration, TimeUnit unit) throws Interrupted
@Override
public void close() {
getManagedChannel().shutdown();
boolean terminated = false;

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@sduskis sduskis requested review from a team and andreamlin January 4, 2019 15:47
andreamlin
andreamlin reviewed Jan 4, 2019
View reviewed changes
gax/src/main/java/com/google/api/gax/core/ResourceCloseException.java Outdated
@@ -0,0 +1,40 @@
/*
* Copyright 2018 Google LLC

This comment was marked as spam.

Sign in to view

andreamlin
andreamlin reviewed Jan 4, 2019
View reviewed changes
gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcTransportChannel.java Outdated
@@ -91,6 +92,11 @@ public boolean awaitTermination(long duration, TimeUnit unit) throws Interrupted
@Override
public void close() {
getManagedChannel().shutdown();
try {
getManagedChannel().awaitTermination(6, TimeUnit.MINUTES);

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

andreamlin
andreamlin approved these changes Jan 4, 2019
View reviewed changes
Copy link
Contributor

@andreamlin andreamlin left a comment

Choose a reason for hiding this comment

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

LGTM

@andreamlin andreamlin merged commit f66cc12 into googleapis:master Jan 10, 2019
This was referenced Jan 11, 2019
Closed
Closed
Closed
@wilhuff wilhuff mentioned this pull request Jan 16, 2019
Closed
@chsi13 chsi13 mentioned this pull request Jan 25, 2019
Open
@ajaaym ajaaym mentioned this pull request Mar 28, 2019
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sduskis sduskis sduskis left review comments

@andreamlin andreamlin andreamlin approved these changes

Assignees
No one assigned
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ajaaym @codecov-io @sduskis @andreamlin @googlebot