-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Deadlock issues #1884
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
Closed
Closed
Deadlock issues #1884
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…on to (googleapis#1645)". Compression is not fully supported in gRPC, can't have it in the library yet. This reverts commit a599972.
…Consumer interface, it really is no useful to be able to set an exception as ack reply, since the result is the same as nack, if we ever require another result then we can just add one more value to the AckReply enum. Also adding a fail-safe catch so if the receiver ever throws an exception we will interpret that as a nack and keep going.
This prevents the library extending messages infinitevely if the user code has lost track of the ack consumer handle.
the next extension surpasses it.
user code
- Separated the handling of batched received messages from the
per-message dispatching of message to the user code. In order to
accomplish this I had to keep the batch in memory while the processing
of each message will draw from the in-memory batch until completely
depleted.
- Drawing flow controller permits on a per message basis (used to
try to draw permits for the whole batch potentially deadlocking the
whole subscriber), this addresses the deadlock condition raised here
googleapis#1868
- No longer using blocking flow controller, instead pausing and
resuming pulls/streamed-messages based on the flow controller
feedback and when new permits become available.
- A separate executor for alarms (2 threads in it has showed up to
scale pretty well with many subscriber, given our ack operations
are pretty lightweight)
- Setting the maximum of messages to pull per request based on the
number requested by the user in the flow controller (if any), this
in a best effort addresses
googleapis#1868
Fixes googleapis#1868, fixes googleapis#1865 and fixes googleapis#1855
Author
|
I'm dropping this PR and have it reviewed in my repo first by the team to then merge it here. |
meltsufin
pushed a commit
that referenced
this pull request
Dec 22, 2025
* chore(main): release 3.23.8 * chore: generate libraries at Thu Nov 13 19:28:54 UTC 2025 --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: cloud-java-bot <cloud-java-bot@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@tbkneeland @kamalaboulhosn PTAL