Skip to content

correct lengthRemain handling into message_read.readMessageV2#1411

Open
Dmitriy-Kulagin wants to merge 1 commit into
segmentio:mainfrom
Dmitriy-Kulagin:patch-1
Open

correct lengthRemain handling into message_read.readMessageV2#1411
Dmitriy-Kulagin wants to merge 1 commit into
segmentio:mainfrom
Dmitriy-Kulagin:patch-1

Conversation

@Dmitriy-Kulagin

Copy link
Copy Markdown

In topic with log compaction the incorrect handling of lengthRemain into readMessageV2 lead to endlessly reading of the same compacted batch. It is happens because skipping offset to batch.lastOffset assumes that lengthRemain ==0 but without this correction lengthRemain became negative and comparison with zero doesn't work.

@Dmitriy-Kulagin

Copy link
Copy Markdown
Author

I'm pretty sure that this MR can fix several different issues that I've described in this issue

Comment thread message_reader.go Outdated
Comment thread message_reader.go Outdated
greg0ire added a commit to greg0ire/kafka-go that referenced this pull request Jan 8, 2026
@greg0ire

greg0ire commented Jan 8, 2026

Copy link
Copy Markdown

I've asked my LLM to create this failing test: #1420

It fails without your patch, and works with it (when I try locally). Feel free to incorporate it in your PR.

Also, my colleagues experienced the issue, and they could confirm this fixes it.

greg0ire added a commit to greg0ire/kafka-go that referenced this pull request Jan 8, 2026
greg0ire added a commit to greg0ire/kafka-go that referenced this pull request Jan 8, 2026
@greg0ire

greg0ire commented Jan 9, 2026

Copy link
Copy Markdown

You should probably squash all 3 commits into one and force push, maybe the build will pass this time.

@petedannemann @hhahn-tw please review 🙏

Co-authored-by: Grégoire Paris <postmaster@greg0ire.fr>
@Dmitriy-Kulagin

Copy link
Copy Markdown
Author

The squasing of commits helped to pass all checks!

@Dmitriy-Kulagin

Copy link
Copy Markdown
Author

@petedannemann @hhahn-tw please review it please 🙏 or suggest somebody else who can finally merge it.
issue 1416 has been opened months ago without any reaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants