Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Use tempo's iterators #180

Merged
merged 2 commits into from
Sep 1, 2022
Merged

Use tempo's iterators #180

merged 2 commits into from
Sep 1, 2022

Conversation

simonswine
Copy link
Collaborator

@simonswine simonswine commented Aug 31, 2022

This imports tempo's parquetquery into pkgs/firedb/query. It makes use of its iterators to gather data that matches.

This avoids reconstruction of data using reflection and also reuses parquet.Values wherever possible

@simonswine simonswine added the kind/enhancement New feature or request label Aug 31, 2022
@simonswine simonswine force-pushed the 20220830_use_tempo_iters branch 3 times, most recently from b700a1b to 9644a6f Compare August 31, 2022 16:00
@simonswine simonswine changed the title 20220830 use tempo iters Use tempo's iterators Aug 31, 2022
@simonswine simonswine marked this pull request as ready for review August 31, 2022 16:28
return exists
}

func (b *singleBlockQuerier) forMatchingProfiles(ctx context.Context, matchers []*labels.Matcher, start, end model.Time,
Copy link
Collaborator

Choose a reason for hiding this comment

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

💡
I think this should return an iterator type. Ultimately I should be able to use that iterator to read profile data, and stacktraces samples. Ordered by ts and labels.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact I think the profileSelector should be an iterator, although I just realised that merging across different block/head will cause issues if we don't properly merge symbols.

Copy link
Collaborator

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

Lgtm

Imported from github.com/grafana/tempo/pkg/parquetquery

d1fc482f4b2c1679bc2deca6055129c0168fcfb0
@simonswine simonswine force-pushed the 20220830_use_tempo_iters branch from 9644a6f to dbbe62d Compare September 1, 2022 12:49
@simonswine simonswine merged commit 2f93879 into main Sep 1, 2022
@simonswine simonswine deleted the 20220830_use_tempo_iters branch September 1, 2022 13:01
simonswine added a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants