-
Notifications
You must be signed in to change notification settings - Fork 244
Add optional backend for ClickHouse storage #6194
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
Open
danipozo
wants to merge
7
commits into
parca-dev:main
Choose a base branch
from
danipozo:danipozo/clickhouse-backend
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+2,455
−200
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add a new optional storage backend backed by a ClickHouse database. This should improve performance over the FrostDB backend and help users send data from more nodes and query it efficiently. This commit is the first working version of the code, improvements will follow in later commits.
- move Symbolizer interface to the symbolizer module - remove deduplication and ordering of results on the Go side, relying on the database instead - in QueryRange, move grouping by labels to ClickHouse - in QuerySingle and QueryMerge, match FrostDB aggregation behavior - remove dead code - formatting
The function was moved to that module so it makes sense for the test to be there too. Remove test that only checked constants had specific values.
Contributor
Author
Contributor
|
Awesome! Thanks for this. Will review it next week |
213a51a to
597c762
Compare
Contributor
|
This looks great! Thanks for the contribution. I would like to mark this as an experimental feature for now. As such can we move the clickhouse flags into the |
Hide them by default as it's an experimental feature
Contributor
Author
|
Sure! Done in the latest commit |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds an optional flag to enable storage of traces in a ClickHouse server. This enables better scalability for users who wish to ingest and query data from many nodes, as the current FrostDB storage struggles with heavy use cases.
This introduces a new module implementing the same public interface as the
parcacolmodule, based on FrostDB, and initializes the selected backend depending on the flag. Traces are inserted into and queried from aMergeTreetable, the basic engine in ClickHouse. This could be modified to be aReplicatedMergeTreeto enable more complex setups. For the labels, it uses a column with the JSON type, which stores most frequent labels in separate columns for query efficiency (this was a design objective of FrostDB AFAIK).Caveats