-
Notifications
You must be signed in to change notification settings - Fork 84
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
fix: Add a throwException behavior when the StreamWriter inflight queue is full #1642
Conversation
Status.fromCode(Code.RESOURCE_EXHAUSTED) | ||
.withDescription( | ||
"Exceeds client side inflight buffer, consider add more buffer or open more connections.")); | ||
} else if (this.limitExceededBehavior == FlowController.LimitExceededBehavior.Ignore) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be the ignore option just a break; at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
L735, by default it is Block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I meant was, that the actual meaning of LimitExceededBehavior.Ignore is, that this inflight quota counting is simply ignored. So the StreamWriter just continues sending. But this option is probably not that relevant for productive setups.
} else if (this.limitExceededBehavior == FlowController.LimitExceededBehavior.Ignore) { | |
} else if (this.limitExceededBehavior == FlowController.LimitExceededBehavior.Ignore) { | |
break; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to think we shouldn't allow it...
🤖 I have created a release *beep* *boop* --- ## [2.14.0](v2.13.0...v2.14.0) (2022-05-19) ### Features * add build scripts for native image testing in Java 17 ([#1440](#1440)) ([#1655](#1655)) ([ac2dfaf](ac2dfaf)) ### Bug Fixes * Add a throwException behavior when the StreamWriter inflight queue is full ([#1642](#1642)) ([4dcf0d5](4dcf0d5)) * add extra JsonWriterTest to show that the LimitBehavior addition is not breaking ([#1643](#1643)) ([320f5fc](320f5fc)) * ints/longs are numerics ([#1596](#1596)) ([d046c8d](d046c8d)), closes [#1516](#1516) ### Dependencies * update arrow.version to v8 ([#1645](#1645)) ([06e3c34](06e3c34)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v2.12.0 ([#1654](#1654)) ([ec4f60b](ec4f60b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Add a throwException behavior when the StreamWriter inflight queue is full.
Fixes #1539 ☕️
If you write sample code, please follow the samples format.