-
Notifications
You must be signed in to change notification settings - Fork 649
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
cmd: add meta page related surgery commands #539
Conversation
Linked to #370 |
cc @fuweid |
} | ||
defer f.Close() | ||
|
||
buf := make([]byte, 1024) |
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 it be page-size buf?
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.
It should be fine, because the meta data is just about 52 bytes.
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.
It is fine from code correctness, but it's not great from readability perspective. It's confusing to have arbitrary high value set. Can you just use pagesize, or leave a comment why 1024?
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.
Added a comment,
// The meta page is just 64 bytes, and definitely less than 1024 bytes,
// so it's fine to only read 1204 bytes. Note we don't care about the
// pageSize when reading the first meta page, because we always read the
// file starting from offset 0. Actually the passed pageSize is 0 when
// reading the first meta page in the `surgery meta update` command.
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.
Ugh, there is typo in the comment. Changed "1204" to "1024"
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
@ptabor Resolved all the comments, PTAL, thx |
@ptabor @serathius @mitake @spzala PTAL |
} | ||
|
||
for _, field := range o.fields { | ||
kv := strings.Split(field, ":") |
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.
kv := strings.Split(field, ":") | |
kv := strings.SplitN(field, ":", 2) |
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.
updated.
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.
Don't see the change.
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 intentionally changed it back. Because I need to validate the format is exactly in the format of name:value
; the slice returned by strings.Split
should contain exactly two elements.
kv := strings.Split(field, ":")
if len(kv) != 2 {
return fmt.Errorf("invalid key-value pair: %s", field)
}
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! Thanks @ahrtr
Two command: - surgery meta validate - surgery meta update Signed-off-by: Benjamin Wang <[email protected]>
thx all for the review. @fuweid @ptabor @serathius @spzala merging... |
What this PR fix
This PR provides two surgery commands to validate and update meta pages separately.
It validates whether the meta pages have corrupted.
It supports updating fields: root, freelist, pageSize and pgid.
Exampe: how to fix #537 using this PR
In #537, the meta pages corrupted. The roots in both meta pages somehow point to a freelist. Obviously it isn't correct.
The possible correct root page should be one of 4, 23, 27 or 39 (Note the root page must contains buckets). Changing the root page ID in both meta pages to any one of them can fix the db. But I am not sure which one is the real root page, because I am not familiar with how influxd uses bbolt.
The command of changing the root page to 4 are as below. You can use the similar command to change the root page to 23, 27 or 39, you just need to replace 4 (after
root:
) with one of them.I already generated 4 db files (each of them has one of the root pages) as attached,
Next step
We should provide more command to inspect the data inside each bucket, especially inline bucket. In this case, it can help users to identify which page is the real root page.