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

Shutdown GRPC managed channel properly on close - #628

Merged
andreamlin merged 4 commits into
googleapis:masterfrom
ajaaym:fix_3912
Jan 10, 2019
Merged

Shutdown GRPC managed channel properly on close#628
andreamlin merged 4 commits into
googleapis:masterfrom
ajaaym:fix_3912

Conversation

@ajaaym

@ajaaym ajaaym commented Dec 26, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

codecov-io commented Dec 26, 2018 •
edited
Loading

Copy link
Copy Markdown

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
Comment thread gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcTransportChannel.java Outdated
@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
Comment thread 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
Comment thread gax-grpc/src/main/java/com/google/api/gax/grpc/GrpcTransportChannel.java Outdated
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

@andreamlin andreamlin left a comment

Copy link
Copy Markdown
Contributor

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

2 more reviewers
@sduskis sduskis sduskis left review comments
@andreamlin andreamlin andreamlin approved these changes
Reviewers whose approvals may not affect merge requirements

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