Skip to content

Fix native histogram data loss in split_by_interval merge due to broken minTime() sort#7555

Merged
yeya24 merged 1 commit into
cortexproject:masterfrom
PaurushGarg:fix/native-histogram-mintime-sort
May 28, 2026
Merged

Fix native histogram data loss in split_by_interval merge due to broken minTime() sort#7555
yeya24 merged 1 commit into
cortexproject:masterfrom
PaurushGarg:fix/native-histogram-mintime-sort

Conversation

@PaurushGarg
Copy link
Copy Markdown
Contributor

…ordering for split_by_interval merge

What this PR does:
Fixes minTime() in pkg/querier/tripperware/merge.go to handle native histogram-only responses when sorting sub-interval responses before merging.

When split_by_interval splits a range query and merges the results, MergeResponse sorts responses by minTime() to ensure chronological order before the dedup/merge logic runs. However, minTime() only checked SampleStream.Samples[0] — for native histogram queries like sum(rate(nh_metric[20m])), the result is a FloatHistogram stored in SampleStream.Histograms with Samples empty. This caused minTime() to return -1 for all sub-interval responses.

Tests
Added TestMinTime covering: empty matrix, float-only, histogram-only, both, and empty stream cases.

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

…ordering for split_by_interval merge

Signed-off-by: Paurush Garg <paurushg@amazon.com>
@dosubot dosubot Bot added go Pull requests that update Go code type/bug labels May 22, 2026
@SungJin1212
Copy link
Copy Markdown
Member

Thanks, can you add a changelog?

Copy link
Copy Markdown
Contributor

@danielblando danielblando left a comment

Choose a reason for hiding this comment

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

Thanks, but before merging lets add the CHANGELOG for bugfix as Sungjin commented

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 28, 2026
Copy link
Copy Markdown
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Thanks!

@yeya24 yeya24 merged commit 0c146ef into cortexproject:master May 28, 2026
68 of 69 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code lgtm This PR has been approved by a maintainer size/L type/bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants