Skip to content

Fix: EncodeAggrChunk created corrupt block if the last chunk is empty#8684

Open
daijy wants to merge 4 commits intothanos-io:mainfrom
daijy:fix_aggrchunk
Open

Fix: EncodeAggrChunk created corrupt block if the last chunk is empty#8684
daijy wants to merge 4 commits intothanos-io:mainfrom
daijy:fix_aggrchunk

Conversation

@daijy
Copy link
Copy Markdown

@daijy daijy commented Feb 17, 2026

This PR fixes a decode failure for aggregate chunks when the last aggregate (AggrCounter) is missing.
In this case, decoding could return invalid size instead of ErrAggrNotExist. Decoder logic expects at least one byte (+1 for encoding marker) even for zero-length entries during pre-check. For the final nil aggregate, that extra byte does not exist in the stream.

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Signed-off-by: Daniel Dai <jidai@roku.com>
Signed-off-by: Daniel Dai <jidai@roku.com>
Signed-off-by: Daniel Dai <jidai@roku.com>
Signed-off-by: Daniel Dai <jidai@roku.com>
Copy link
Copy Markdown
Member

@GiedriusS GiedriusS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this ever happen in practice or is this theoretical?

@MichelHollands
Copy link
Copy Markdown

Does this ever happen in practice or is this theoretical?

We have seen it in production. The cause is still unclear.

@daijy
Copy link
Copy Markdown
Author

daijy commented Feb 21, 2026

It happens when the AggrCounter is missing for the AggrChunk, as shown in the test case. In AggrChunk.Get, it erroneously return "invalid size" instead of ErrAggrNotExist. Should be quite frequent, severity depends on how the caller response. In our parquet downsample converter, it stops converting the rest of tsdb downsample. I fixed it in the write side to write extra bytes for AggrCounter if it's empty, so the reader won't trigger "invalid size".

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants