Skip to content

dedup: fix stale valueType in boundedSeriesIterator.Next() - #9028

Open
anusha19murthy wants to merge 2 commits into
thanos-io:mainfrom
anusha19murthy:fix/bounded-iterator-stale-valuetype
Open

anusha19murthy wants to merge 2 commits into
thanos-io:mainfrom
anusha19murthy:fix/bounded-iterator-stale-valuetype

Conversation

@anusha19murthy

Copy link
Copy Markdown
Contributor

What

boundedSeriesIterator.Next() returns a stale valueType when it seeks past samples before mint.

When the first sample falls before mint, the iterator seeks forward correctly but still returns the valueType from the original sample (before the seek) instead of the one it landed on. If a series changes from float to histogram right at the query boundary, we end up returning ValFloat when the actual sample is a histogram.

The fix

I capture the return value of Seek() back into valueType so the caller gets the correct type:

// before
if it.Seek(it.mint) == chunkenc.ValNone {

// after
valueType = it.Seek(it.mint)
if valueType == chunkenc.ValNone {

This was introduced in 789046ad1 when the return type changed from bool to chunkenc.ValueType — the old bool version couldn't have this mismatch.

Tests

Added TestBoundedSeriesIteratorNextValueType with a mixedTypeIterator helper that supports interleaved float/histogram samples. Covers:

  • float before mint, histogram at mint (the actual bug)
  • float before mint, float at mint
  • sample already within bounds
  • all samples before mint
  • sample beyond maxt

@GiedriusS GiedriusS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not add mixed type support to type sample?

@anusha19murthy
anusha19murthy force-pushed the fix/bounded-iterator-stale-valuetype branch from 2456510 to 869dd7c Compare September 17, 2026 12:11
@anusha19murthy

Copy link
Copy Markdown
Contributor Author

I've added the required changes

@GiedriusS

Copy link
Copy Markdown
Member

Linter errors seem related

@anusha19murthy
anusha19murthy force-pushed the fix/bounded-iterator-stale-valuetype branch from 869dd7c to 6fa670e Compare September 19, 2026 05:01
Signed-off-by: anusha19murthy <anusha19murthy@gmail.com>
@anusha19murthy
anusha19murthy force-pushed the fix/bounded-iterator-stale-valuetype branch from 6fa670e to 871c6a7 Compare September 19, 2026 05:10
@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.54%. Comparing base (050d688) to head (871c6a7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9028      +/-   ##
==========================================
- Coverage   64.55%   64.54%   -0.02%     
==========================================
  Files         289      289              
  Lines       37412    37413       +1     
==========================================
- Hits        24153    24149       -4     
- Misses      11158    11164       +6     
+ Partials     2101     2100       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
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.

2 participants