From 7cd985bdac7ab374a0f7b44aad96dbe4fad06695 Mon Sep 17 00:00:00 2001 From: Gyuho Lee Date: Mon, 18 Dec 2017 11:40:30 -0800 Subject: [PATCH 1/2] clientv3: translate Snapshot API gRPC status error To be consistent with other APIs. Signed-off-by: Gyuho Lee --- clientv3/integration/maintenance_test.go | 106 +++++++++++++++++++++++ clientv3/maintenance.go | 12 ++- 2 files changed, 117 insertions(+), 1 deletion(-) diff --git a/clientv3/integration/maintenance_test.go b/clientv3/integration/maintenance_test.go index 27b3b0eab73..d25c4e9bad2 100644 --- a/clientv3/integration/maintenance_test.go +++ b/clientv3/integration/maintenance_test.go @@ -15,11 +15,20 @@ package integration import ( + "bytes" "context" + "fmt" + "io" + "io/ioutil" + "path/filepath" "testing" + "time" "github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes" "github.com/coreos/etcd/integration" + "github.com/coreos/etcd/lease" + "github.com/coreos/etcd/mvcc" + "github.com/coreos/etcd/mvcc/backend" "github.com/coreos/etcd/pkg/testutil" ) @@ -84,3 +93,100 @@ func TestMaintenanceMoveLeader(t *testing.T) { t.Fatalf("new leader expected %d, got %d", target, lead) } } + +// TestMaintenanceSnapshotError ensures that context cancel/timeout +// before snapshot reading returns corresponding context errors. +func TestMaintenanceSnapshotError(t *testing.T) { + defer testutil.AfterTest(t) + + clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1}) + defer clus.Terminate(t) + + // reading snapshot with canceled context should error out + ctx, cancel := context.WithCancel(context.Background()) + rc1, err := clus.RandClient().Snapshot(ctx) + if err != nil { + t.Fatal(err) + } + defer rc1.Close() + + cancel() + _, err = io.Copy(ioutil.Discard, rc1) + if err != context.Canceled { + t.Errorf("expected %v, got %v", context.Canceled, err) + } + + // reading snapshot with deadline exceeded should error out + ctx, cancel = context.WithTimeout(context.Background(), time.Second) + defer cancel() + rc2, err := clus.RandClient().Snapshot(ctx) + if err != nil { + t.Fatal(err) + } + defer rc2.Close() + + time.Sleep(2 * time.Second) + + _, err = io.Copy(ioutil.Discard, rc2) + if err != nil && err != context.DeadlineExceeded { + t.Errorf("expected %v, got %v", context.DeadlineExceeded, err) + } +} + +// TestMaintenanceSnapshotErrorInflight ensures that inflight context cancel/timeout +// fails snapshot reading with corresponding context errors. +func TestMaintenanceSnapshotErrorInflight(t *testing.T) { + defer testutil.AfterTest(t) + + clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1}) + defer clus.Terminate(t) + + // take about 1-second to read snapshot + clus.Members[0].Stop(t) + dpath := filepath.Join(clus.Members[0].DataDir, "member", "snap", "db") + b := backend.NewDefaultBackend(dpath) + s := mvcc.NewStore(b, &lease.FakeLessor{}, nil) + rev := 100000 + for i := 2; i <= rev; i++ { + s.Put([]byte(fmt.Sprintf("%10d", i)), bytes.Repeat([]byte("a"), 1024), lease.NoLease) + } + s.Close() + b.Close() + clus.Members[0].Restart(t) + + // reading snapshot with canceled context should error out + ctx, cancel := context.WithCancel(context.Background()) + rc1, err := clus.RandClient().Snapshot(ctx) + if err != nil { + t.Fatal(err) + } + defer rc1.Close() + + donec := make(chan struct{}) + go func() { + time.Sleep(300 * time.Millisecond) + cancel() + close(donec) + }() + _, err = io.Copy(ioutil.Discard, rc1) + if err != nil && err != context.Canceled { + t.Errorf("expected %v, got %v", context.Canceled, err) + } + <-donec + + // reading snapshot with deadline exceeded should error out + ctx, cancel = context.WithTimeout(context.Background(), time.Second) + defer cancel() + rc2, err := clus.RandClient().Snapshot(ctx) + if err != nil { + t.Fatal(err) + } + defer rc2.Close() + + // 300ms left and expect timeout while snapshot reading is in progress + time.Sleep(700 * time.Millisecond) + _, err = io.Copy(ioutil.Discard, rc2) + if err != nil && err != context.DeadlineExceeded { + t.Errorf("expected %v, got %v", context.DeadlineExceeded, err) + } +} diff --git a/clientv3/maintenance.go b/clientv3/maintenance.go index 25abc9c9100..3156770137b 100644 --- a/clientv3/maintenance.go +++ b/clientv3/maintenance.go @@ -196,7 +196,17 @@ func (m *maintenance) Snapshot(ctx context.Context) (io.ReadCloser, error) { } pw.Close() }() - return pr, nil + return &snapshotReadCloser{ctx: ctx, ReadCloser: pr}, nil +} + +type snapshotReadCloser struct { + ctx context.Context + io.ReadCloser +} + +func (rc *snapshotReadCloser) Read(p []byte) (n int, err error) { + n, err = rc.ReadCloser.Read(p) + return n, toErr(rc.ctx, err) } func (m *maintenance) MoveLeader(ctx context.Context, transfereeID uint64) (*MoveLeaderResponse, error) { From c8a516d515ab982b762c6762f3a12cfe6c33f362 Mon Sep 17 00:00:00 2001 From: Gyuho Lee Date: Mon, 18 Dec 2017 13:08:02 -0800 Subject: [PATCH 2/2] Documentation/upgrades: document Snapshot API error handling Signed-off-by: Gyuho Lee --- Documentation/upgrades/upgrade_3_3.md | 48 ++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/Documentation/upgrades/upgrade_3_3.md b/Documentation/upgrades/upgrade_3_3.md index 164b60258a7..77b12b24b35 100644 --- a/Documentation/upgrades/upgrade_3_3.md +++ b/Documentation/upgrades/upgrade_3_3.md @@ -74,7 +74,7 @@ Set `embed.Config.Debug` field to `true` to enable gRPC server logs. #### Change in `/health` endpoint response value -Previously, `[endpoint]:[client-port]/health` returns manually marshaled JSON value. 3.3 instead defines [`etcdhttp.Health`](https://godoc.org/github.com/coreos/etcd/etcdserver/api/etcdhttp#Health) struct and returns properly encoded JSON value with errors, if any. +Previously, `[endpoint]:[client-port]/health` returned manually marshaled JSON value. 3.3 instead defines [`etcdhttp.Health`](https://godoc.org/github.com/coreos/etcd/etcdserver/api/etcdhttp#Health) struct and returns properly encoded JSON value with errors, if any. Before @@ -127,6 +127,52 @@ After docker pull gcr.io/etcd-development/etcd:v3.3.0 ``` +#### Change in `Snapshot` API error type + +Previously, clientv3 `Snapshot` API returned raw [`grpc/*status.statusError`] type error. v3.3 now translates those errors to corresponding public error types, to be consistent with other APIs. + +Before + +```go +import "context" + +// reading snapshot with canceled context should error out +ctx, cancel := context.WithCancel(context.Background()) +rc, _ := cli.Snapshot(ctx) +cancel() +_, err := io.Copy(f, rc) +err.Error() == "rpc error: code = Canceled desc = context canceled" + +// reading snapshot with deadline exceeded should error out +ctx, cancel = context.WithTimeout(context.Background(), time.Second) +defer cancel() +rc, _ = cli.Snapshot(ctx) +time.Sleep(2 * time.Second) +_, err = io.Copy(f, rc) +err.Error() == "rpc error: code = DeadlineExceeded desc = context deadline exceeded" +``` + +After + +```go +import "context" + +// reading snapshot with canceled context should error out +ctx, cancel := context.WithCancel(context.Background()) +rc, _ := cli.Snapshot(ctx) +cancel() +_, err := io.Copy(f, rc) +err == context.Canceled + +// reading snapshot with deadline exceeded should error out +ctx, cancel = context.WithTimeout(context.Background(), time.Second) +defer cancel() +rc, _ = cli.Snapshot(ctx) +time.Sleep(2 * time.Second) +_, err = io.Copy(f, rc) +err == context.DeadlineExceeded +``` + #### Deprecate `golang.org/x/net/context` imports `clientv3` has deprecated `golang.org/x/net/context`. If a project vendors `golang.org/x/net/context` in other code (e.g. etcd generated protocol buffer code) and imports `github.com/coreos/etcd/clientv3`, it requires Go 1.9+ to compile.