-
Notifications
You must be signed in to change notification settings - Fork 13
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
Improve compaction #149
Improve compaction #149
Conversation
728d136
to
1ee6dd4
Compare
SupersededCount = 100 | ||
otelName = "sqllog" | ||
SupersededCount = 100 | ||
compactBatchSize = 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a compaction txn timeout?
1ee6dd4
to
5fcee8e
Compare
Depends on #140 |
Benchmark Result
Current status
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work Marco, excited to watch this land! Let's test this against the k8s-snap and microk8s and view traces as discussed.
5227860
to
9e4df6b
Compare
Given the discussion we had with @ktsakalozos, it is best not to change schema in this version, so now this query depends on #136 instead. I expect the query to take a little bit longer than before, as the index is not covering anymore for the deleted rows, but it is still an improvement over what we had before. |
be67dee
to
f3ffd98
Compare
Marking as ready since #136 was meged. |
f3ffd98
to
42d97f5
Compare
Description
This PR rewrites the compaction step in a similar way upstream is doing. This way, the throughput should be a lot higher. The main differences from upstream are:
This query does not use indexes right now. It's still way faster than what we have and what upstream has. Maybe we could improve things with a covering index if/when it's needed for the Watch/TTL query. Otherwise we might be ok with what we have.
Improvements to the previous implementation
Up to this point we've been doing compaction on each revision within the compaction range row by row. Looping over all revisions takes about 7s on an idle cluster, rare issues have been recorded where compaction takes minutes. With this compaction query improvement we are doing a compaction batch in around 40ms and a max <100ms on an idle cluster. (Note: the idle cluster only needs 1 batch to complete the compaction under normal circumstances).
This PR addresses: #145 and #146