*: add 'endpoint hashkv' command #8351

gyuho · 2017-08-02T00:41:17Z

Fix #8348.
Address #8305.

xiang90 · 2017-08-02T04:25:13Z

etcdctl/README.md

+------------------------+------------+
+|        ENDPOINT        |    HASH    |
+------------------------+------------+
+|  http://127.0.0.1:2379 | 1084519789 |


extra space here.

xiang90 · 2017-08-02T04:25:37Z

@gyuho Thanks for getting this done SO FAST!

heyitsanthony · 2017-08-02T19:31:41Z

Documentation/op-guide/v2-migration.md

@@ -28,6 +28,12 @@ Sometimes an etcd cluster will possibly have v3 data which should not be overwri
 ETCDCTL_API=3 etcdctl get "" --from-key --keys-only --limit 1 | wc -l
 ```

+In case v2 data were migrated with TTLs, ensure that stores are consistent by checking the hashes:


heyitsanthony · 2017-08-02T19:33:24Z

clientv3/integration/maintenance_test.go

+	for i := 0; i < 3; i++ {
+		cli := clus.Client(i)
+
+		hresp, err := cli.HashKV(context.Background(), clus.Members[0].GRPCAddr(), 0)


heyitsanthony · 2017-08-02T19:34:24Z

clientv3/integration/maintenance_test.go

+	var hv uint32
+	for i := 0; i < 3; i++ {
+		cli := clus.Client(i)
+


can get rid of the unreliable time.Sleep:

if _, err := cli.Get(context.TODO(), "foo"); err != nil { t.Fatal(err) }

heyitsanthony · 2017-08-02T19:36:07Z

clientv3/maintenance.go

@@ -50,6 +51,11 @@ type Maintenance interface {
 	// Status gets the status of the endpoint.
 	Status(ctx context.Context, endpoint string) (*StatusResponse, error)

+	// HashKV returns the state of backend database at the time of the RPC.


s/the state of/a hash of the/
s/backend database/KV state/? it's not the entire backend...

heyitsanthony · 2017-08-02T19:37:00Z

clientv3/maintenance.go

@@ -50,6 +51,11 @@ type Maintenance interface {
 	// Status gets the status of the endpoint.
 	Status(ctx context.Context, endpoint string) (*StatusResponse, error)

+	// HashKV returns the state of backend database at the time of the RPC.
+	// If revision is non-zero, hash is computed only in key bucket and keys at or below


// If revision is zero, the hash is computed on all keys. If the revision is non-zero, the hash is computed on all keys at or below the given revision.

I wonder should the comment for maintenance HashKV the same as the one in rpc?
HashKV computes the hash of all MVCC keys up to a given revision.
https://github.com/coreos/etcd/blob/master/etcdserver/etcdserverpb/rpc.proto#L187

clientv3 Compact and the rpc.proto Compact have different comments too; the difference here seems to be similar. Although to be closer to clientv3 Compact it should talk about KV history instead of a "backend database".

Duplicated comments (/anything) can easily get out of sync and I think there are too many exceptions to using the RPC comment directly to justify tooling to keep it synced up.

heyitsanthony · 2017-08-02T19:37:48Z

etcdctl/README.md

@@ -641,6 +641,49 @@ Get the status for all endpoints in the cluster associated with the default endp
 +------------------------+------------------+----------------+---------+-----------+-----------+------------+
 ```

+### ENDPOINT HASH
+
+ENDPOINT HASH fetches the backend state of each endpoint.


fetches the hash of the key-value store of an endpoint.

heyitsanthony · 2017-08-02T19:40:05Z

etcdctl/ctlv3/command/ep_command.go

+		Short: "Prints out backend database states specified in `--endpoints` flag",
+		Run:   epHashCommandFunc,
+	}
+	hc.PersistentFlags().Int64Var(&epHashRev, "rev", 0, "if non-zero, hash keys at or below the given revision")


"maximum revision to hash (default: all revisions)"

heyitsanthony · 2017-08-02T19:42:56Z

etcdctl/ctlv3/command/ep_command.go

+	display.EndpointHash(hashList)
+
+	if err != nil {
+		os.Exit(ExitError)


ExitWithError?

heyitsanthony

looks ok in general

heyitsanthony · 2017-08-03T23:05:09Z

Documentation/op-guide/v2-migration.md

+In case v2 data was migrated with TTLs, ensure that stores are consistent by checking the hashes:
+
+```sh
+ETCDCTL_API=3 etcdctl endpoint hash --cluster


Can this be hashkv? The naming should be consistent with the underlying RPC / client call if possible (e.g., what if etcdctl ever gets support for the old hash rpc)

Given that the failure mode for inconsistent data is "member is completely broken", I would recommend increasing the warnings given here.

"You must ensure all members have consistent data before migrating, otherwise some members will not be able to correctly serve queries for new data."

This really should also explain what to do if someone has frequently expiring keys - for instance, someone with keys expiring every second will have an extremely high chance of having inconsistent data. Should they keep retrying?

heyitsanthony · 2017-08-03T23:42:10Z

etcdctl/README.md

+
+##### Simple format
+
+Prints a humanized table of each endpoint URL and backend hash.


s/backend/KV history/?

heyitsanthony · 2017-08-03T23:44:00Z

etcdctl/ctlv3/command/ep_command.go

+func newEpHashCommand() *cobra.Command {
+	hc := &cobra.Command{
+		Use:   "hash",
+		Short: "Prints out backend database states specified in `--endpoints` flag",


"Prints the KV history hash for each endpoint in --endpoints"?

gyuho · 2017-08-04T18:11:31Z

Documentation/op-guide/v2-migration.md

+For etcd v3.3+, run `ETCDCTL_API=3 etcdctl endpoint hashkv --cluster` to ensure key-value stores are consistent post migration.
+
+**Warn**: When v2 store has expiring TTL keys and migrate command intends to preserve TTLs, it is possible that inconsistent data is migrated. TODO?
+


Any thoughts on this?

@heyitsanthony @xiang90

ref. #8305 (comment)

s/it is possible that inconsistent data is migrated. TODO/migration may be inconsistent with the last committed v2 state when run on any member with a raft index less than the last leader's raft index./
?

Signed-off-by: Gyu-Ho Lee <[email protected]>

heyitsanthony

lgtm thanks

gyuho added kind/release-note labels Aug 2, 2017

xiang90 reviewed Aug 2, 2017

View reviewed changes

gyuho force-pushed the hash branch from bba3f69 to 2ac7053 Compare August 2, 2017 16:19

gyuho removed the WIP label Aug 2, 2017

gyuho requested a review from heyitsanthony August 2, 2017 16:20

heyitsanthony suggested changes Aug 2, 2017

View reviewed changes

gyuho added the WIP label Aug 2, 2017

gyuho force-pushed the hash branch 2 times, most recently from c0491b0 to 518a6f7 Compare August 3, 2017 09:21

gyuho removed the WIP label Aug 3, 2017

gyuho force-pushed the hash branch 2 times, most recently from cc99ec8 to 33282b5 Compare August 3, 2017 09:25

heyitsanthony suggested changes Aug 3, 2017

View reviewed changes

gyuho changed the title *: add 'endpoint hash' command *: add 'endpoint hashkv' command Aug 4, 2017

gyuho added the WIP label Aug 4, 2017

gyuho force-pushed the hash branch 3 times, most recently from 80b2412 to 1ee4267 Compare August 4, 2017 18:09

gyuho commented Aug 4, 2017

View reviewed changes

gyuho force-pushed the hash branch from 1ee4267 to 0d56e83 Compare August 4, 2017 20:11

gyuho added WIP and removed WIP labels Aug 4, 2017

gyuho added 5 commits August 5, 2017 18:17

clientv3: add 'HashKV' to 'Maintenance' interface

8c32cd9

Signed-off-by: Gyu-Ho Lee <[email protected]>

clientv3/integration: add 'TestMaintenanceHashKV'

9982cd0

Signed-off-by: Gyu-Ho Lee <[email protected]>

ctlv3: add 'endpoint hashkv' command

5176b63

Signed-off-by: Gyu-Ho Lee <[email protected]>

e2e: test 'endpoint hashkv' command

43ccc54

Signed-off-by: Gyu-Ho Lee <[email protected]>

op-guide/v2-migration: endpoint hashkv post migration

e4e6147

Signed-off-by: Gyu-Ho Lee <[email protected]>

gyuho force-pushed the hash branch from 0d56e83 to e4e6147 Compare August 6, 2017 20:50

gyuho removed the WIP label Aug 6, 2017

heyitsanthony approved these changes Aug 7, 2017

View reviewed changes

gyuho merged commit a9b9ef5 into etcd-io:master Aug 7, 2017

gyuho deleted the hash branch August 7, 2017 16:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add 'endpoint hashkv' command #8351

*: add 'endpoint hashkv' command #8351

gyuho commented Aug 2, 2017 •

edited

Loading

xiang90 Aug 2, 2017

xiang90 commented Aug 2, 2017

heyitsanthony Aug 2, 2017

heyitsanthony Aug 2, 2017

heyitsanthony Aug 2, 2017

heyitsanthony Aug 2, 2017

heyitsanthony Aug 2, 2017

fanminshi Aug 2, 2017

heyitsanthony Aug 3, 2017

heyitsanthony Aug 2, 2017

heyitsanthony Aug 2, 2017

heyitsanthony Aug 2, 2017

heyitsanthony left a comment

heyitsanthony Aug 3, 2017

smarterclayton Aug 4, 2017

heyitsanthony Aug 3, 2017

heyitsanthony Aug 3, 2017

gyuho Aug 4, 2017 •

edited

Loading

heyitsanthony Aug 5, 2017

heyitsanthony left a comment


		##### Simple format

		Prints a humanized table of each endpoint URL and backend hash.

		For etcd v3.3+, run `ETCDCTL_API=3 etcdctl endpoint hashkv --cluster` to ensure key-value stores are consistent post migration.

		Warn: When v2 store has expiring TTL keys and migrate command intends to preserve TTLs, it is possible that inconsistent data is migrated. TODO?

*: add 'endpoint hashkv' command #8351

*: add 'endpoint hashkv' command #8351

Conversation

gyuho commented Aug 2, 2017 • edited Loading

Choose a reason for hiding this comment

xiang90 commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gyuho Aug 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heyitsanthony left a comment

Choose a reason for hiding this comment

gyuho commented Aug 2, 2017 •

edited

Loading

gyuho Aug 4, 2017 •

edited

Loading