KAFKA-19018,KAFKA-19063: Implement maxRecords and acquisition lock timeout in share fetch request and response resp. #19334
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
github-actions
bot
added
triage
apoorvmittal10
added
ci-approved
and removed
performance
triage
apoorvmittal10
force-pushed
the
KAFKA-19018
branch
from
8bfe7d5 to
a24c2b9
Compare
…meout in share fetch request and response resp.
apoorvmittal10
force-pushed
the
KAFKA-19018
branch
from
a24c2b9 to
e6789a5
Compare
AndrewJSchofield
requested changes
Apr 1, 2025
| @@ -39,6 +39,8 @@ | |||
| "about": "The top-level response error code." }, | |||
| { "name": "ErrorMessage", "type": "string", "versions": "0+", "nullableVersions": "0+", "default": "null", | |||
| "about": "The top-level error message, or null if there was no error." }, | |||
| { "name": "AcquisitionLockTimeout", "type": "int64", "versions": "0+", | |||
There was a problem hiding this comment.
Please can you add Ms to the field name? In RPCs, the convention for timeouts is always to have Ms on the end, such as RebalanceTimeoutMs. Also, they are always int32, which is probably appropriate here too.
| "about": "The maximum bytes to fetch. See KIP-74 for cases where this limit may not be honored." }, | ||
| "about": "The maximum bytes to fetch. See KIP-74 for cases where this limit may not be honored." }, | ||
| { "name": "MaxRecords", "type": "int32", "versions": "0+", | ||
| "about": "The maximum number of records to fetch. This is a soft limit and records can exceed upto the end of fetch log batch." }, |
There was a problem hiding this comment.
I suggest for the second sentence "This limit can be exceeded for alignment of batch boundaries".
AndrewJSchofield
approved these changes
Apr 1, 2025
janchilling
pushed a commit
to janchilling/kafka
that referenced
this pull request
Apr 4, 2025
…meout in share fetch request and response resp. (apache#19334) PR add `MaxRecords` to share fetch request and also adds `AcquisitionLockTimeout` to share fetch response. PR also removes internal broker config of `max.fetch.records`. Reviewers: Andrew Schofield <[email protected]>
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.
PR add
MaxRecordsto share fetch request and also addsAcquisitionLockTimeoutto share fetch response. PR also removes internal broker config ofmax.fetch.records.