Skip to content

feat: recycle channel on consecutive new stream failures#2903

Open
nimf wants to merge 1 commit intomainfrom
cons_fails_recycle
Open

feat: recycle channel on consecutive new stream failures#2903
nimf wants to merge 1 commit intomainfrom
cons_fails_recycle

Conversation

@nimf
Copy link
Copy Markdown
Contributor

@nimf nimf commented Apr 24, 2026

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)
  • Rollback plan is reviewed and LGTMed
  • All new data plane features have a completed end to end testing plan

Fixes #<issue_number_goes_here> ☕️

If you write sample code, please follow the samples format.

@nimf nimf requested review from a team as code owners April 24, 2026 00:01
@product-auto-label product-auto-label Bot added size: m Pull request size is medium. api: bigtable Issues related to the googleapis/java-bigtable API. labels Apr 24, 2026
} else if (!status.isOk() && status.getCode() != Code.CANCELLED) {
channelWrapper.consecutiveFailures++;
}
releaseChannel(channelWrapper, status);
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.

Not related to this PR, but I feel releaseChannel should be called releaseStream or something, with recycleChannel the naming gets a little confusing..

}

private static boolean shouldRecycleChannel(Status status) {
private static boolean shouldRecycleChannel(ChannelWrapper channelWrapper, Status status) {
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.

Also unrelated to this PR.. I'm wondering how this works..

if we get unimplemented, shouldn't the client fallback to unary? (iiuc which doesn't use this channel pool) recycling the channel and try to create a new one likely still won't work?

private static final String DEFAULT_LOG_NAME = "pool";
private static final AtomicInteger INDEX = new AtomicInteger();

private static final int CONSECUTIVE_OPEN_FAILURE_THRESHOLD = 5;
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.

wondering if this should be called CONSECUTIVE_OPEN_SESSION_FAILURE_THRESHOLD.

And I think this should come from client config?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigtable Issues related to the googleapis/java-bigtable API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants