snapshot save command does not timeout

[jingyih]

Currently etcdctl snapshot save does not timeout. It could hang indefinitely. Besides, if user specifies command timeout in flag --command-timeout, the command will not use this timeout.

etcd/etcdctl/ctlv3/command/snapshot_command.go

Lines 106 to 108 in 7f450bf

	if err := sp.Save(context.TODO(), *cfg, path); err != nil {
	ExitWithError(ExitInterrupted, err)
	}

One possible solution is to route --command-timeout to snapshot save command. But the default timeout is only 5 seconds. This might cause more issues to users if they do not specify timeout when calling snapshot save.

etcd/etcdctl/ctlv3/ctl.go

Line 31 in 9454c4c

defaultCommandTimeOut = 5 * time.Second

Any ideas on how to resolve this?

/cc @jpbetz @gyuho @xiang90

[gyuho]

Sure, we can set the context timeout?

etcd/clientv3/maintenance.go

Lines 60 to 63 in 7f450bf

	// Snapshot provides a reader for a point-in-time snapshot of etcd.
	// If the context "ctx" is canceled or timed out, reading from returned
	// "io.ReadCloser" would error out (e.g. context.Canceled, context.DeadlineExceeded).
	Snapshot(ctx context.Context) (io.ReadCloser, error)

[xiang90]

@jingyih Probably set a different default timeout (keep current infinity or something like 10minutes ) for snapshot command. When users specify the command timeout explicitly, overwrite it.

[jpbetz]

Ah, maybe just document that the timeout is different for the snapshot operation and implement it in code to be special cased? I'm in favor of having the default be "no timeout", mostly for backward compatibility. We should probably apply the same treatment to the recover operation as well.

[jingyih]

Thanks for all the inputs:) I think we agreed on what the behavior should be. I'll send a fix.