-
Notifications
You must be signed in to change notification settings - Fork 222
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
br: add br backup info #836
Conversation
proto/brpb.proto
Outdated
// for filling the diff data during conf changing. | ||
// The current implementation scan [0, now) for every task ranges newly added, | ||
// So this field is reserved for future usage. | ||
reserved 6; reserved "last_backup_ts"; |
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.
last_backup_ts
=> last_update_ts
🤔
shall we have something like create ts
?
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 guess the start-ts
would perform as create ts?
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.
start-ts
could be earlier than task create time. but for now it's not required.
StorageBackend storage = 1; | ||
// The time range for backing up. | ||
uint64 start_ts = 2; | ||
uint64 end_ts = 3; |
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.
will tikv remove this task if resolved_ts
reached end_ts
?
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 guess we can leave it a "done" status and let the user to delete?
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.
The task would exist, but the observing of stream-log has been stoped. And user can status/stop the task in this situation ?
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
undo the changes to |
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.
rest lgtm
proto/brpb.proto
Outdated
// We use '/tidb/br-stream/ranges/<task-name>/<start-key> -> <end-key>' | ||
// as the default key prefix for the target ranges for the task. | ||
// Given the path unchanged, the dynamic config(or, hyperlink?) seems useless. | ||
reserved "backup_range_key_prefix"; |
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.
why do we even need this config 🤔
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.
Just a placeholder for the dead 'ranges' 🤔, maybe just remove it
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.
Rename it into "ranges" in memory of the original ranges
field.
e24f47f
to
c2ecb52
Compare
* br: undo changes for coprocessor and pdpb * br: align the spaces
cc pingcap/tidb#29969
For better utilize the already defined
StorageBackend
and better encoding binary keys, I decided to encode the taskinfo as protocol buffer. 🤔