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.
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
store: added option to reencode and compress postings before storing them to the cache #2297
store: added option to reencode and compress postings before storing them to the cache #2297
Changes from all commits
c6e8d7a
35a31f9
f480494
fb2f0df
e672219
51e2feb
391da98
a57624b
1ab0142
eb8fca9
5347e89
eec1a07
85b6a2b
13adc82
ef74bae
564f6f3
2317b03
e2203aa
eacd394
a2ee495
8c438d5
22a1338
644d09d
c754543
91860cf
869ac4d
5c7dd1b
6a8d5b6
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Have you considered adding a couple of metrics with postings raw size and the compressed one, to measure the compression ratio in the wild?
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.
Good point
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.
I've added following metrics:
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.
Oh, one more thing... I've moved compression outside of the lock. However, storing data back to cache is still done with lock held, which looks wrong. I haven't fixed that part, because this PR ir not about it, but it looks like it should be moved outside as well. I haven't looked into more details here (like, does it break anything?)
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.
Also added number of decompressions, decompression errors, and total decompression time (snappy part only, we don't time decoding varints and adding values together)
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.
Could be worth logging the error, otherwise we'll never found out when it happens.
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.
We will know as querying would fail?
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.
That was my thinking here... if it we get error here, it means postings was corrupted, so it will still be corrupted few lines later when it's used, and error will be reported there.
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.
These errors are now tracked via metric.