-
Notifications
You must be signed in to change notification settings - Fork 823
Add type header to WAL/Checkpoint records and use TSDB records #2436
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
Conversation
3ee493f
to
d5c2e21
Compare
PR is ready for review |
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 think we don't handle the migration well. I can't see where we are handling the errors returned by failed proto marshalling
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.
Actually nvm, I missed the merr = nil
bits :) It's all good! LGTM!
After some offline discussion with Goutham, we decided to add the Prometheus TSDB records in this PR itself to not have many versions or records, as the current record would be obsolete immediately after TSDB records are in. |
f7e26c3
to
5cbf9a4
Compare
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
4cc3c02
to
508fc08
Compare
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
508fc08
to
569d375
Compare
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
675b9a1
to
31e9162
Compare
I wrote a benchmark for WAL replay (will open PR after #2465 is merged as it depends on that) to compare the new records, and the results look good. Largely depends on Checkpoint/WAL ratio. The benchmark had equal number of samples in WAL and Checkpoint.
|
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.
LGTM with some minor comments. processWAL
method seems pretty convoluted, but is not part of this PR.
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Tests are failing here now. |
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
8d9a76b
to
0ae3252
Compare
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
a5decf4
to
a09b735
Compare
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
a09b735
to
61fa5b2
Compare
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
What this PR does:
Add a single-byte type header to WAL and Checkpoint records and way to handle old records concurrently. Additionally, replace existing proto records in WAL with Prometheus TSDB records.
Which issue(s) this PR fixes:
Fixes #2426
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]