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

chore: use map to reduce writes to clickhouse tag tables #177

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

makeavish
Copy link
Member

@srikanthccv
Copy link
Member

Please share how you measured the change and the difference observed with some non-trivial workloads.

@makeavish
Copy link
Member Author

With this change CPU time of writeTagBatch decreases by 30-70% depending on the duplicates attributes in batch.
Tested this with Flask application running at 100rps. Didn't see any significant changes in memory profile.

@srikanthccv
Copy link
Member

I think 100 rps is not enough for this kind of benchmark. Can you use some synthetic load generator like https://github.com/Omnition/synthetic-load-generator to create a much higher load around 5k-10k span/s? I would put a load on the collector as much as it starts to give the context deadline error and see when it reaches this peak state before and after the change. And it helps if you share the before and after profiles as well.

@makeavish
Copy link
Member Author

I think 100 rps is not enough for this kind of benchmark. Can you use some synthetic load generator like https://github.com/Omnition/synthetic-load-generator to create a much higher load around 5k-10k span/s? I would put a load on the collector as much as it starts to give the context deadline error and see when it reaches this peak state before and after the change. And it helps if you share the before and after profiles as well.

I think synthetic load generator doesn't reproduce real-world use case well so tried 1000rps(generating around 5-6k spans/s) with flask app with DB calls. The result was the same.

@ankitnayan
Copy link
Contributor

this PR should be part of next release

@makeavish makeavish merged commit 2855716 into main Oct 5, 2023
3 checks passed
@makeavish makeavish deleted the chore/trac-tag-batch-perf branch October 5, 2023 15:34
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