[LI-HOTFIX] Return valid data during throttling #514
Merged
Conversation
This file contains 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
groelofs
approved these changes
Jun 6, 2024
There was a problem hiding this comment.
This seems straightforward as a fix (and much simpler than I was expecting), but approval is conditional on demonstrating that it actually does fix the "stuck consumers" issue while not completely bypassing quota-enforcement. IOW, we'll want to test this carefully.
Yellow-Rice
reviewed
Jun 6, 2024
CCisGG
added a commit
that referenced
this pull request
Jun 11, 2024
This reverts commit d4f6f91. For the "consumer get stuck during throttling" issue, we realized there is a safer solution than the above commit. The alternative solution is to increase the maxThrottleTime by increase the quota window size, which should effectively reduce the overall throughput. We will go with that path first. If the issue is still not mitigated, we can fallback to this solution and re-apply this patch.
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.
BUGS=LIKAFKA-59133
Description
In KIP-219, people decided to changed the throttling behavior from "wait for throttleTimeMs before sending back response" to "return immediately with empty response and mute the channel by throttleTimeMs". While it is reasonable in terms of it strictly limit the Fetch Bytes Rate to below the quota value, it introduces a new problem that the request already takes the broker system resource but the consumer does not get any useful data. As a result, the broker could be busy with handling throttled requests and keep throttle incoming request, while none of the request has data and consumers get stuck.
To mitigate it, we propose to actually return the data in the throttled response. By doing it, the consumer can still slowly proceed even though all the requests are throttled.
Summary of testing strategy
We likely need to add some tests in our certification process. To force throttling happens, we need to set a low quota. During the throttling, we need to assert: