Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Querier: improve merging of batchStream objects #7478

Merged
merged 9 commits into from
Mar 5, 2024

Conversation

duricanikolic
Copy link
Contributor

What this PR does

This PR refactors batchStream type, which represents a stream of chunk.Batch objects which are sorted by their timestamps.

In particular:

  • Methods HasNext(), Next(), AtTime(), At(), AtHistogram() and AtFloatHistograms() are defined on the chunk.Batch type.
  • Type batch.batchStream is not a slice of chunk.Batchs anymore, but a struct.
  • Method batch.mergeStreams() has been replaced with a method merge() defined on batch.batchStream type.
  • Type mergeIterator has been simplified by removing different unnecessary attributes. This could be achieved thanks to the refactoring done in batch.batchStream type.

Which issue(s) this PR fixes or relates to

Part of #7427

Checklist

  • Tests updated.
  • [na] Documentation added.
  • [na] CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • [na] about-versioning.md updated with experimental features.

@duricanikolic duricanikolic self-assigned this Feb 27, 2024
@duricanikolic duricanikolic force-pushed the yuri/native-histograms-benchmarks-next-step branch 4 times, most recently from 6376a74 to cd87968 Compare February 29, 2024 14:46
Copy link
Contributor

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

Looks very good from first read through, I just have a comment on the tests.

pkg/querier/batch/stream_test.go Outdated Show resolved Hide resolved
@duricanikolic duricanikolic force-pushed the yuri/native-histograms-benchmarks-next-step branch 2 times, most recently from be4bf10 to 7154358 Compare March 4, 2024 11:32
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
@duricanikolic duricanikolic force-pushed the yuri/native-histograms-benchmarks-next-step branch from 7154358 to 53c242a Compare March 5, 2024 18:40
Signed-off-by: Yuri Nikolic <durica.nikolic@grafana.com>
@duricanikolic duricanikolic marked this pull request as ready for review March 5, 2024 18:44
@duricanikolic duricanikolic requested a review from a team as a code owner March 5, 2024 18:44
Copy link
Contributor

@krajorama krajorama left a comment

Choose a reason for hiding this comment

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

LGTM

@duricanikolic duricanikolic merged commit 049e870 into main Mar 5, 2024
29 checks passed
@duricanikolic duricanikolic deleted the yuri/native-histograms-benchmarks-next-step branch March 5, 2024 19:12
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.

None yet

2 participants