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

Speedup collection of results from ingesters in the querier #4100

Merged
merged 12 commits into from
Sep 25, 2024

Conversation

electron0zero
Copy link
Member

@electron0zero electron0zero commented Sep 20, 2024

What this PR does:
Earlier we used to make calls to ingesters, get results and itarate over these results to collect them and then return them to query-frontend.

Iterating and then collecting the results was sync and a bottleneck and showed up in traces and profiles.

This PR pushed down the collection of results and that makes it a parallel and removes the iteration step.

now we query an ingester and collect the results where we query, and once all ingesters response, we read the collected results and return them to query-frontend for further combining.

see this trace for example:
image

along with these changes, It also does the following:

  • removed the type assertions and use of interface{} return types from forIngesterRings and callback function forEachFn
  • added locking in ScopedDistinctString and DistinctString collectors to make the safe to use.

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]

pkg/collector/generic_collector.go Outdated Show resolved Hide resolved
modules/querier/querier.go Outdated Show resolved Hide resolved
modules/querier/querier.go Show resolved Hide resolved
modules/querier/querier.go Show resolved Hide resolved
Copy link
Contributor

@mdisibio mdisibio left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@electron0zero electron0zero merged commit 109f8d0 into grafana:main Sep 25, 2024
16 checks passed
@electron0zero electron0zero deleted the ingester_collection_perf branch September 25, 2024 07:05
knylander-grafana pushed a commit to knylander-grafana/tempo-doc-work that referenced this pull request Sep 26, 2024
…4100)

* Add Generic Collector for collection of things

* push collection down to calls to ingester

* fixup generic collector

* cleanup comments and TODOs

* Update CHANGELOG.md

* match span name with func name

* inline the collection of results in SearchRecent

* remove break goto

* make ScopedDistinctString and DistinctString safe + test
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.

3 participants