From 018706112dac5c7d8d8dcad1dd4b03b836ae436d Mon Sep 17 00:00:00 2001 From: "Sahdev P. Zala" Date: Wed, 24 Oct 2018 14:58:28 -0400 Subject: [PATCH 1/5] ClientV3: Add integration test for server timeout Partially Fixes #8645 --- clientv3/integration/black_hole_test.go | 44 +++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/clientv3/integration/black_hole_test.go b/clientv3/integration/black_hole_test.go index 0ff3f73f14f..3a09b587ab7 100644 --- a/clientv3/integration/black_hole_test.go +++ b/clientv3/integration/black_hole_test.go @@ -26,8 +26,52 @@ import ( "go.etcd.io/etcd/integration" "go.etcd.io/etcd/pkg/testutil" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) +func TestServerGRPCKeepAliveTimeout(t *testing.T) { + defer testutil.AfterTest(t) + + clus := integration.NewClusterV3(t, &integration.ClusterConfig{ + Size: 2, + GRPCKeepAliveInterval: time.Second, + GRPCKeepAliveTimeout: time.Second, + }) + defer clus.Terminate(t) + + eps := []string{clus.Members[0].GRPCAddr()} + + ccfg := clientv3.Config{ + Endpoints: []string{eps[0]}, + } + + cli, err := clientv3.New(ccfg) + if err != nil { + t.Fatal(err) + } + defer cli.Close() + + if _, err = clus.Client(1).Put(context.TODO(), "foo", "bar"); err != nil { + t.Fatal(err) + } + clus.Members[1].Blackhole() + time.Sleep(10 * time.Second) + // remove blackhole but connection should be unavailable now + clus.Members[1].Unblackhole() + if _, err = clus.Client(1).Put(context.TODO(), "foo1", "bar1"); err != nil { + ev, ok := status.FromError(err) + if !ok { + t.Fatal(err) + } + if ev.Code() != codes.Unavailable { + t.Fatal(err) + } + } else { + t.Error("rpc error expected") + } +} + // TestBalancerUnderBlackholeKeepAliveWatch tests when watch discovers it cannot talk to // blackholed endpoint, client balancer switches to healthy one. // TODO: test server-to-client keepalive ping From 3f6d5085c61c66cb1e15a1ee2d0923880802924d Mon Sep 17 00:00:00 2001 From: "Sahdev P. Zala" Date: Wed, 24 Oct 2018 23:07:24 -0400 Subject: [PATCH 2/5] ClientV3: Add integration test for server timeout Partially Fixes #8645 --- clientv3/integration/black_hole_test.go | 26 +++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/clientv3/integration/black_hole_test.go b/clientv3/integration/black_hole_test.go index 3a09b587ab7..a4102db85fe 100644 --- a/clientv3/integration/black_hole_test.go +++ b/clientv3/integration/black_hole_test.go @@ -30,6 +30,11 @@ import ( "google.golang.org/grpc/status" ) +// TestServerGRPCKeepAliveTimeout tests ServerParameters keepalive ping on server-side that +// after having pinged for keepalive check, the server waits for a duration of Timeout and +// if no activity is seen even after that the connection is closed. The keepalive takes few +// seconds to get in impact so typically the the time it takes to close the connection is +// somewhat higher than specified Timeout value. func TestServerGRPCKeepAliveTimeout(t *testing.T) { defer testutil.AfterTest(t) @@ -57,19 +62,20 @@ func TestServerGRPCKeepAliveTimeout(t *testing.T) { } clus.Members[1].Blackhole() time.Sleep(10 * time.Second) - // remove blackhole but connection should be unavailable now + // remove blackhole but by now the keepalive ping should have triggered server to + // close server-to-client connection. clus.Members[1].Unblackhole() - if _, err = clus.Client(1).Put(context.TODO(), "foo1", "bar1"); err != nil { - ev, ok := status.FromError(err) - if !ok { - t.Fatal(err) - } - if ev.Code() != codes.Unavailable { - t.Fatal(err) - } - } else { + _, err = clus.Client(1).Put(context.TODO(), "foo1", "bar1") + if err == nil { t.Error("rpc error expected") } + ev, ok := status.FromError(err) + if !ok { + t.Fatal(err) + } + if ev.Code() != codes.Unavailable { + t.Fatal(err) + } } // TestBalancerUnderBlackholeKeepAliveWatch tests when watch discovers it cannot talk to From e48af62da3e405fcf67212adb78f0ea72fbfdabc Mon Sep 17 00:00:00 2001 From: "Sahdev P. Zala" Date: Thu, 25 Oct 2018 09:20:04 -0400 Subject: [PATCH 3/5] ClientV3: Add integration test for server timeout Partially Fixes #8645 --- clientv3/integration/black_hole_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/clientv3/integration/black_hole_test.go b/clientv3/integration/black_hole_test.go index a4102db85fe..be9bc51a54c 100644 --- a/clientv3/integration/black_hole_test.go +++ b/clientv3/integration/black_hole_test.go @@ -32,16 +32,16 @@ import ( // TestServerGRPCKeepAliveTimeout tests ServerParameters keepalive ping on server-side that // after having pinged for keepalive check, the server waits for a duration of Timeout and -// if no activity is seen even after that the connection is closed. The keepalive takes few -// seconds to get in impact so typically the the time it takes to close the connection is -// somewhat higher than specified Timeout value. +// if no activity is seen even after that the connection is closed on server-side. The keepalive +// takes few extra seconds to get in impact so typically the time it takes to close the connection +// is somewhat higher than specified Timeout value. func TestServerGRPCKeepAliveTimeout(t *testing.T) { defer testutil.AfterTest(t) clus := integration.NewClusterV3(t, &integration.ClusterConfig{ Size: 2, - GRPCKeepAliveInterval: time.Second, - GRPCKeepAliveTimeout: time.Second, + GRPCKeepAliveInterval: 500 * time.Millisecond, + GRPCKeepAliveTimeout: 500 * time.Millisecond, }) defer clus.Terminate(t) From 01afcf120ccdbd84f4e40e2d788014c41560721e Mon Sep 17 00:00:00 2001 From: "Sahdev P. Zala" Date: Mon, 5 Nov 2018 23:19:37 -0500 Subject: [PATCH 4/5] ClientV3: Add integration test for server timeout Partially Fixes #8645 --- clientv3/integration/black_hole_test.go | 44 +++++++++++++++---------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/clientv3/integration/black_hole_test.go b/clientv3/integration/black_hole_test.go index be9bc51a54c..8906567e04b 100644 --- a/clientv3/integration/black_hole_test.go +++ b/clientv3/integration/black_hole_test.go @@ -40,42 +40,52 @@ func TestServerGRPCKeepAliveTimeout(t *testing.T) { clus := integration.NewClusterV3(t, &integration.ClusterConfig{ Size: 2, - GRPCKeepAliveInterval: 500 * time.Millisecond, - GRPCKeepAliveTimeout: 500 * time.Millisecond, + GRPCKeepAliveInterval: 2 * time.Second, + GRPCKeepAliveTimeout: 1 * time.Second, }) defer clus.Terminate(t) eps := []string{clus.Members[0].GRPCAddr()} - ccfg := clientv3.Config{ Endpoints: []string{eps[0]}, } - cli, err := clientv3.New(ccfg) if err != nil { t.Fatal(err) } defer cli.Close() + // give keepalive some time + time.Sleep(4 * time.Second) + if _, err = clus.Client(1).Put(context.TODO(), "foo", "bar"); err != nil { t.Fatal(err) } - clus.Members[1].Blackhole() - time.Sleep(10 * time.Second) - // remove blackhole but by now the keepalive ping should have triggered server to - // close server-to-client connection. - clus.Members[1].Unblackhole() - _, err = clus.Client(1).Put(context.TODO(), "foo1", "bar1") + // TODO: keepalive sometimes doesn't work on first attempt. + for i := 0; i < 5; i++ { + clus.Members[1].Blackhole() + time.Sleep(10 * time.Second) + // remove blackhole but by now the keepalive ping should have triggered server to + // close server-to-client connection. + clus.Members[1].Unblackhole() + _, err = clus.Client(1).Put(context.TODO(), "foo1", "bar1") + if err != nil { + ev, ok := status.FromError(err) + if !ok { + t.Fatal(err) + } + if ev.Code() != codes.Unavailable { + t.Fatal(err) + } + break + } else { + t.Logf("info: expected an error, received none.") + } + } + if err == nil { t.Error("rpc error expected") } - ev, ok := status.FromError(err) - if !ok { - t.Fatal(err) - } - if ev.Code() != codes.Unavailable { - t.Fatal(err) - } } // TestBalancerUnderBlackholeKeepAliveWatch tests when watch discovers it cannot talk to From 332e0a8497bdac17c4e907dbefab4bdbbc74b57b Mon Sep 17 00:00:00 2001 From: "Sahdev P. Zala" Date: Thu, 29 Nov 2018 13:21:24 -0500 Subject: [PATCH 5/5] ClientV3: Add integration test for server timeout Partially Fixes #8645 --- .words | 1 + clientv3/integration/black_hole_test.go | 17 +++++++++-------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.words b/.words index 0e9b4992b2a..c4db0ae9de0 100644 --- a/.words +++ b/.words @@ -104,3 +104,4 @@ PermitWithoutStream __lostleader ErrConnClosing unfreed +ServerParameters diff --git a/clientv3/integration/black_hole_test.go b/clientv3/integration/black_hole_test.go index 8906567e04b..be023e0e0cb 100644 --- a/clientv3/integration/black_hole_test.go +++ b/clientv3/integration/black_hole_test.go @@ -39,15 +39,16 @@ func TestServerGRPCKeepAliveTimeout(t *testing.T) { defer testutil.AfterTest(t) clus := integration.NewClusterV3(t, &integration.ClusterConfig{ - Size: 2, - GRPCKeepAliveInterval: 2 * time.Second, - GRPCKeepAliveTimeout: 1 * time.Second, + Size: 1, + GRPCKeepAliveInterval: 1 * time.Second, + GRPCKeepAliveTimeout: 500 * time.Millisecond, }) defer clus.Terminate(t) eps := []string{clus.Members[0].GRPCAddr()} ccfg := clientv3.Config{ - Endpoints: []string{eps[0]}, + Endpoints: []string{eps[0]}, + DialTimeout: 12 * time.Second, } cli, err := clientv3.New(ccfg) if err != nil { @@ -58,17 +59,17 @@ func TestServerGRPCKeepAliveTimeout(t *testing.T) { // give keepalive some time time.Sleep(4 * time.Second) - if _, err = clus.Client(1).Put(context.TODO(), "foo", "bar"); err != nil { + if _, err = cli.Put(context.TODO(), "foo", "bar"); err != nil { t.Fatal(err) } // TODO: keepalive sometimes doesn't work on first attempt. for i := 0; i < 5; i++ { - clus.Members[1].Blackhole() + clus.Members[0].Blackhole() time.Sleep(10 * time.Second) // remove blackhole but by now the keepalive ping should have triggered server to // close server-to-client connection. - clus.Members[1].Unblackhole() - _, err = clus.Client(1).Put(context.TODO(), "foo1", "bar1") + clus.Members[0].Unblackhole() + _, err = cli.Put(context.TODO(), "foo1", "bar1") if err != nil { ev, ok := status.FromError(err) if !ok {