fix(consumer): stuck on the batch with zero records length#3221
fix(consumer): stuck on the batch with zero records length#3221
Conversation
Signed-off-by: Sterligov Denis <[email protected]>
6bcf24e to
7a66726
Compare
puellanivis
left a comment
There was a problem hiding this comment.
I suppose, the only feedback I could give is that it would be nice to have something explaining that this field doesn’t actually appear “on the wire”? I mean, that was my first thought, “wait, are we renaming an encoded field in the fetch response block?” But it’s not, so that is why renaming it shouldn’t be a big deal, right?
But also, this is technically changing an exported field on an exported type… 😬 Although, a quick search of github suggests the only people referencing the field are from forks/clones.…
e2fc916 to
533d7a1
Compare
I've added a comment explaining this field.
In my opinion, changing the name is not a problem, this field is only used in a few places and it was added in a previous PR that solved this problem in a way that was not quite correct. |
|
We could consider unexporting the name as well? This would make it perfectly clear that it’s not a part of the exported API? |
This is computed locally and isn't part of the protocol so shouldn't be part of the API either. As we're renaming it as part of this PR we may as well also unexport it at this stage too Signed-off-by: Dominic Evans <[email protected]>
3ec27b0 to
e55a4e6
Compare
This pull request addresses the known issue described in KAFKA-5443, which was initially analyzed in #2053. A prior attempt to fix it was made in #2057, but the fix was applied incorrectly and did not fully resolve the root cause.
I’ve provided a more detailed explanation of the underlying issue and the reasoning behind this fix in this comment.