Skip to content

commitlog: Truncate segment before resuming#5116

Open
kim wants to merge 4 commits into
masterfrom
kim/commitlog/truncate-segment-before-resume
Open

commitlog: Truncate segment before resuming#5116
kim wants to merge 4 commits into
masterfrom
kim/commitlog/truncate-segment-before-resume

Conversation

@kim
Copy link
Copy Markdown
Contributor

@kim kim commented May 26, 2026

read_exact doesn't distinguish between EOF and not enough bytes to fill the given buffer. We may thus consider a segment valid to be resumed, but leave in trailing bytes (less than the commit header length). That can cause silent data loss, because appending more data will render the segment corrupt: a restart will then start a new segment, leaving anything after the trailing bytes unreachable.

To fix this, truncate the segment to the size determined by Metadata::extract before resuming writes.

Expected complexity level and risk

2

Testing

Added a test

@kim kim requested a review from Shubham8287 May 26, 2026 11:04
@Shubham8287
Copy link
Copy Markdown
Contributor

Don't know if it's a github bug or what but It is looking like a empty commit.

`read_exact` doesn't distinguish between EOF and not enough bytes to
fill the given buffer. We may thus consider a segment valid to be
resumed, but leave in trailing bytes (less than the commit header
length). That can cause silent data loss, because appending more data
will render the segment corrupt: a restart will then start a new
segment, leaving anything after the trailing bytes unreachable.

To fix this, truncate the segment to the size determined by
`Metadata::extract` before resuming writes.
@kim kim force-pushed the kim/commitlog/truncate-segment-before-resume branch from 8bd704b to f6afc9a Compare May 26, 2026 13:17
Comment thread crates/commitlog/src/repo/mod.rs Outdated
@kim kim enabled auto-merge May 27, 2026 11:23
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