-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
*: snapshot RPC #5012
*: snapshot RPC #5012
Conversation
bw += n | ||
} | ||
} | ||
pw.Close() |
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.
defer pw.Close()
? Or do we only do this when it was exited with break
?
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.
There are failure paths that do CloseWithError
so defer pw.Close()
would try to double close.
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 there's CloseWithError
. Got it.
all fixed up ptal /cc @xiang90 I've been testing with: # feed keys into V3Demo cluster before this
ETCDCTL_API=3 ./bin/etcdctl snapshot save test.db
# kill V3Demo cluster here
rm -rf infra1-test.etcd/; ETCDCTL_API=3 ./bin/etcdctl snapshot restore test.db --name="infra1-test" --initial-cluster="infra1-test=http://localhost:2380" --initial-advertise-peer-urls="http://localhost:2380"
# start etcd on infra1-test |
wr.CompactRevision = 1 | ||
func NewSnapshotRestoreCommand() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "restore <filename>", |
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.
<directory name>
?
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 changed it to use the node name so it matches etcd directory naming behavior. Should it be inconsistent with that?
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... it should be consistent...
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.
--data-dir '${name}.etcd'
path to the data directory.
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.
OK; default to the name if --data-dir
is not specified?
LGTM. Remove the last commit? |
fixed a bug in 3-cluster restore and added support for deleting member bucket contents. ptal /cc @xiang90 |
LGTM |
Includes updated etcdctl snapshot. Restore support still WIP