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

Update blockbuilder to periodically flush wals and sort traces #4550

Merged
merged 6 commits into from
Jan 15, 2025

Conversation

mdisibio
Copy link
Contributor

@mdisibio mdisibio commented Jan 13, 2025

What this PR does:
Block builder flush performance contained an accidental regression that make it a lot slower than generators and ingesters at flushing blocks. It created the WAL blocks unordered, which meant that a lot of random seeking around the files was required in order to iterator the traces in order by ID and copy them to the final backend block. And it didn't periodically flush the WAL blocks, instead it was one huge flush at the end. Now it works exactly like generators and ingesters, in that it periodically flushes idle traces, and each flush is sorted by ID. Then there is no file seeking required when it completes the WAL to the final backend block. This gives about 10x speed improvement.

Additional changes:

  • Fix deletion of WAL/complete blocks.
  • Clear entire WAL folder before each consumption cycle
  • Add benchmark
  • Honor tenant max trace size

TODO

  • Moved liveTraces to shared pkg and updated the generators. We can update the ingesters as well. I wasn't sure if there was any remaining feature gap, and wanted to keep ingesters out of this PR.
  • Now that we are using liveTraces, we can now easily honor tenant max trace size too.

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]

…dress performance bottleneck in block completion time. Also add benchmark, and disk cleanup fixes
@mdisibio mdisibio marked this pull request as ready for review January 14, 2025 18:26
@mdisibio mdisibio merged commit fad4ff7 into grafana:main Jan 15, 2025
16 checks passed
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.

2 participants