From d226d63da7c7de1a2a80adf8a4e2cb7c39c880e3 Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Sat, 10 Oct 2020 19:22:27 +0800 Subject: [PATCH 1/8] fix: fix destory unregister bug --- registry/consul/registry.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/registry/consul/registry.go b/registry/consul/registry.go index c425c5ec20..7f237a1f24 100644 --- a/registry/consul/registry.go +++ b/registry/consul/registry.go @@ -187,5 +187,10 @@ func (r *consulRegistry) IsAvailable() bool { // Destroy consul registry center func (r *consulRegistry) Destroy() { + if r.URL != nil{ + if err := r.UnRegister(*r.URL); err != nil{ + logger.Errorf("consul registry unregister with err: %s", err.Error()) + } + } close(r.done) } From 366911db2fc5936851ea2be21b90837ad3eb30c3 Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Mon, 12 Oct 2020 12:41:07 +0800 Subject: [PATCH 2/8] fix: add consul destroy timeout and test file --- registry/consul/registry.go | 26 ++++++++++++++++++++++++-- registry/consul/registry_test.go | 16 ++++++++++++++++ registry/consul/utils_test.go | 4 ++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/registry/consul/registry.go b/registry/consul/registry.go index 7f237a1f24..33bf6d2c38 100644 --- a/registry/consul/registry.go +++ b/registry/consul/registry.go @@ -37,6 +37,7 @@ import ( const ( registryConnDelay = 3 + registryDestroyDefaultTimeout = time.Second ) func init() { @@ -55,6 +56,9 @@ type consulRegistry struct { // Done field represents whether // consul registry is closed. done chan struct{} + + // time wait when destroy + timeOut time.Duration } func newConsulRegistry(url *common.URL) (registry.Registry, error) { @@ -68,6 +72,7 @@ func newConsulRegistry(url *common.URL) (registry.Registry, error) { URL: url, client: client, done: make(chan struct{}), + timeOut: registryDestroyDefaultTimeout, } return r, nil @@ -188,8 +193,25 @@ func (r *consulRegistry) IsAvailable() bool { // Destroy consul registry center func (r *consulRegistry) Destroy() { if r.URL != nil{ - if err := r.UnRegister(*r.URL); err != nil{ - logger.Errorf("consul registry unregister with err: %s", err.Error()) + done := make(chan struct{}, 1) + ticker := time.NewTicker(r.timeOut) + go func(){ + defer func(){ + if e := recover(); e != nil{ + logger.Errorf("consulRegistry destory with panic: %v", e) + } + done <- struct{}{} + }() + if err := r.UnRegister(*r.URL); err != nil{ + logger.Errorf("consul registry unregister with err: %s", err.Error()) + } + }() + select { + case <- done: + logger.Infof("consulRegistry unregister done") + case <- ticker.C: + logger.Errorf("consul unregister timeout") + ticker.Stop() } } close(r.done) diff --git a/registry/consul/registry_test.go b/registry/consul/registry_test.go index 94718f5ab6..bc4fd400eb 100644 --- a/registry/consul/registry_test.go +++ b/registry/consul/registry_test.go @@ -55,3 +55,19 @@ func (suite *consulRegistryTestSuite) testSubscribe() { assert.NoError(suite.t, err) suite.listener = listener } + +func (suite *consulRegistryTestSuite) testDestroy(){ + consumerRegistryUrl := newConsumerRegistryUrl(registryHost, registryPort) + consumerRegistry, _ := newConsulRegistry(consumerRegistryUrl) + consulRegistryImp := consumerRegistry.(*consulRegistry) + assert.True(suite.t, consulRegistryImp.IsAvailable()) + consulRegistryImp.Destroy() + assert.False(suite.t, consulRegistryImp.IsAvailable()) + + consumerRegistry, _ = newConsulRegistry(consumerRegistryUrl) + consulRegistryImp = consumerRegistry.(*consulRegistry) + consulRegistryImp.URL = nil + assert.True(suite.t, consulRegistryImp.IsAvailable()) + consulRegistryImp.Destroy() + assert.False(suite.t, consulRegistryImp.IsAvailable()) +} diff --git a/registry/consul/utils_test.go b/registry/consul/utils_test.go index 939352dc08..819feb2916 100644 --- a/registry/consul/utils_test.go +++ b/registry/consul/utils_test.go @@ -163,6 +163,7 @@ func test1(t *testing.T) { suite.testListener(remoting.EventTypeAdd) suite.testUnregister() suite.testListener(remoting.EventTypeDel) + suite.testDestroy() } // subscribe -> register -> unregister @@ -183,8 +184,11 @@ func test2(t *testing.T) { suite.testListener(remoting.EventTypeAdd) suite.testUnregister() suite.testListener(remoting.EventTypeDel) + suite.testDestroy() } + + func TestConsulRegistry(t *testing.T) { t.Run("test1", test1) t.Run("test2", test2) From 44d2947e14f81cc41a8e4c7af53a022d4ee6ae9a Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Mon, 12 Oct 2020 15:45:39 +0800 Subject: [PATCH 3/8] fix: fmt file --- registry/consul/registry.go | 22 +++++++++++----------- registry/consul/registry_test.go | 2 +- registry/consul/utils_test.go | 2 -- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/registry/consul/registry.go b/registry/consul/registry.go index 33bf6d2c38..a00ec25188 100644 --- a/registry/consul/registry.go +++ b/registry/consul/registry.go @@ -36,7 +36,7 @@ import ( ) const ( - registryConnDelay = 3 + registryConnDelay = 3 registryDestroyDefaultTimeout = time.Second ) @@ -69,9 +69,9 @@ func newConsulRegistry(url *common.URL) (registry.Registry, error) { } r := &consulRegistry{ - URL: url, - client: client, - done: make(chan struct{}), + URL: url, + client: client, + done: make(chan struct{}), timeOut: registryDestroyDefaultTimeout, } @@ -192,24 +192,24 @@ func (r *consulRegistry) IsAvailable() bool { // Destroy consul registry center func (r *consulRegistry) Destroy() { - if r.URL != nil{ + if r.URL != nil { done := make(chan struct{}, 1) ticker := time.NewTicker(r.timeOut) - go func(){ - defer func(){ - if e := recover(); e != nil{ + go func() { + defer func() { + if e := recover(); e != nil { logger.Errorf("consulRegistry destory with panic: %v", e) } done <- struct{}{} }() - if err := r.UnRegister(*r.URL); err != nil{ + if err := r.UnRegister(*r.URL); err != nil { logger.Errorf("consul registry unregister with err: %s", err.Error()) } }() select { - case <- done: + case <-done: logger.Infof("consulRegistry unregister done") - case <- ticker.C: + case <-ticker.C: logger.Errorf("consul unregister timeout") ticker.Stop() } diff --git a/registry/consul/registry_test.go b/registry/consul/registry_test.go index bc4fd400eb..b300f7536d 100644 --- a/registry/consul/registry_test.go +++ b/registry/consul/registry_test.go @@ -56,7 +56,7 @@ func (suite *consulRegistryTestSuite) testSubscribe() { suite.listener = listener } -func (suite *consulRegistryTestSuite) testDestroy(){ +func (suite *consulRegistryTestSuite) testDestroy() { consumerRegistryUrl := newConsumerRegistryUrl(registryHost, registryPort) consumerRegistry, _ := newConsulRegistry(consumerRegistryUrl) consulRegistryImp := consumerRegistry.(*consulRegistry) diff --git a/registry/consul/utils_test.go b/registry/consul/utils_test.go index 819feb2916..0e5bffe457 100644 --- a/registry/consul/utils_test.go +++ b/registry/consul/utils_test.go @@ -187,8 +187,6 @@ func test2(t *testing.T) { suite.testDestroy() } - - func TestConsulRegistry(t *testing.T) { t.Run("test1", test1) t.Run("test2", test2) From fb04876b10f64da9a079f8f0811f1c3214998c1d Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Mon, 12 Oct 2020 22:48:17 +0800 Subject: [PATCH 4/8] fix: change ticker to after --- registry/consul/registry.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/registry/consul/registry.go b/registry/consul/registry.go index a00ec25188..3a4bee2866 100644 --- a/registry/consul/registry.go +++ b/registry/consul/registry.go @@ -194,7 +194,7 @@ func (r *consulRegistry) IsAvailable() bool { func (r *consulRegistry) Destroy() { if r.URL != nil { done := make(chan struct{}, 1) - ticker := time.NewTicker(r.timeOut) + ticker := time.After(r.timeOut) go func() { defer func() { if e := recover(); e != nil { @@ -209,9 +209,8 @@ func (r *consulRegistry) Destroy() { select { case <-done: logger.Infof("consulRegistry unregister done") - case <-ticker.C: + case <-ticker: logger.Errorf("consul unregister timeout") - ticker.Stop() } } close(r.done) From 2a48d426fd719459367ee47409a1d14ef92d6345 Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Tue, 13 Oct 2020 19:19:25 +0800 Subject: [PATCH 5/8] chore: change time.After usage --- go.sum | 1 + protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go | 2 +- registry/consul/registry.go | 12 +++++------- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/go.sum b/go.sum index 91cdb0da19..76fb0c6e11 100644 --- a/go.sum +++ b/go.sum @@ -883,6 +883,7 @@ k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a h1:UcxjrRMyNx/i/y8G7kPvLy k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E= k8s.io/utils v0.0.0-20190801114015-581e00157fb1 h1:+ySTxfHnfzZb9ys375PXNlLhkJPLKgHajBU0N62BDvE= k8s.io/utils v0.0.0-20190801114015-581e00157fb1/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= +launchpad.net/gocheck v0.0.0-20140225173054-000000000087/go.mod h1:hj7XX3B/0A+80Vse0e+BUHsHMTEhd0O4cpUHr/e/BUM= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI= sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs= diff --git a/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go b/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go index 1af4fafdc6..ae9cc72d09 100644 --- a/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go +++ b/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go @@ -240,7 +240,7 @@ func (g *dubboGrpc) generateClientSignature(servName string, method *pb.MethodDe } respName := "out *" + g.typeName(method.GetOutputType()) if method.GetServerStreaming() || method.GetClientStreaming() { - respName = servName + "_" + generator.CamelCase(origMethName) + "Client" + respName = "out * "+ servName + "_" + generator.CamelCase(origMethName) + "Client" } return fmt.Sprintf("%s func(ctx %s.Context%s, %s) error", methName, contextPkg, reqArg, respName) } diff --git a/registry/consul/registry.go b/registry/consul/registry.go index 3a4bee2866..78caa5a183 100644 --- a/registry/consul/registry.go +++ b/registry/consul/registry.go @@ -37,7 +37,7 @@ import ( const ( registryConnDelay = 3 - registryDestroyDefaultTimeout = time.Second + registryDestroyDefaultTimeout = time.Second * 3 ) func init() { @@ -69,10 +69,9 @@ func newConsulRegistry(url *common.URL) (registry.Registry, error) { } r := &consulRegistry{ - URL: url, - client: client, - done: make(chan struct{}), - timeOut: registryDestroyDefaultTimeout, + URL: url, + client: client, + done: make(chan struct{}), } return r, nil @@ -194,7 +193,6 @@ func (r *consulRegistry) IsAvailable() bool { func (r *consulRegistry) Destroy() { if r.URL != nil { done := make(chan struct{}, 1) - ticker := time.After(r.timeOut) go func() { defer func() { if e := recover(); e != nil { @@ -209,7 +207,7 @@ func (r *consulRegistry) Destroy() { select { case <-done: logger.Infof("consulRegistry unregister done") - case <-ticker: + case <-time.After(registryDestroyDefaultTimeout): logger.Errorf("consul unregister timeout") } } From 255fbc4e876b35a4e376b8a1358431ee4bcd8420 Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Tue, 13 Oct 2020 19:22:18 +0800 Subject: [PATCH 6/8] chore: bugs --- protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go | 2 +- registry/consul/registry.go | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go b/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go index ae9cc72d09..b34ecfaa95 100644 --- a/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go +++ b/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go @@ -240,7 +240,7 @@ func (g *dubboGrpc) generateClientSignature(servName string, method *pb.MethodDe } respName := "out *" + g.typeName(method.GetOutputType()) if method.GetServerStreaming() || method.GetClientStreaming() { - respName = "out * "+ servName + "_" + generator.CamelCase(origMethName) + "Client" + respName = servName + "_" + generator.CamelCase(origMethName) + "Client" } return fmt.Sprintf("%s func(ctx %s.Context%s, %s) error", methName, contextPkg, reqArg, respName) } diff --git a/registry/consul/registry.go b/registry/consul/registry.go index 78caa5a183..b92e335fdb 100644 --- a/registry/consul/registry.go +++ b/registry/consul/registry.go @@ -56,9 +56,6 @@ type consulRegistry struct { // Done field represents whether // consul registry is closed. done chan struct{} - - // time wait when destroy - timeOut time.Duration } func newConsulRegistry(url *common.URL) (registry.Registry, error) { From 60fabc615b823f2cc4f4f0de4caf7098393794a1 Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Tue, 13 Oct 2020 19:24:17 +0800 Subject: [PATCH 7/8] chore: fix fmt --- protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go b/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go index b34ecfaa95..1af4fafdc6 100644 --- a/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go +++ b/protocol/grpc/protoc-gen-dubbo/plugin/dubbo/dubbo.go @@ -240,7 +240,7 @@ func (g *dubboGrpc) generateClientSignature(servName string, method *pb.MethodDe } respName := "out *" + g.typeName(method.GetOutputType()) if method.GetServerStreaming() || method.GetClientStreaming() { - respName = servName + "_" + generator.CamelCase(origMethName) + "Client" + respName = servName + "_" + generator.CamelCase(origMethName) + "Client" } return fmt.Sprintf("%s func(ctx %s.Context%s, %s) error", methName, contextPkg, reqArg, respName) } From a407f629918242737e3ffa6964dd5690f18bf800 Mon Sep 17 00:00:00 2001 From: "382673304@qq.com" <382673304@qq.com> Date: Tue, 13 Oct 2020 19:45:15 +0800 Subject: [PATCH 8/8] chore: delete unused pkg --- go.sum | 1 - 1 file changed, 1 deletion(-) diff --git a/go.sum b/go.sum index 76fb0c6e11..91cdb0da19 100644 --- a/go.sum +++ b/go.sum @@ -883,7 +883,6 @@ k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a h1:UcxjrRMyNx/i/y8G7kPvLy k8s.io/kube-openapi v0.0.0-20191107075043-30be4d16710a/go.mod h1:1TqjTSzOxsLGIKfj0lK8EeCP7K1iUG65v09OM0/WG5E= k8s.io/utils v0.0.0-20190801114015-581e00157fb1 h1:+ySTxfHnfzZb9ys375PXNlLhkJPLKgHajBU0N62BDvE= k8s.io/utils v0.0.0-20190801114015-581e00157fb1/go.mod h1:sZAwmy6armz5eXlNoLmJcl4F1QuKu7sr+mFQ0byX7Ew= -launchpad.net/gocheck v0.0.0-20140225173054-000000000087/go.mod h1:hj7XX3B/0A+80Vse0e+BUHsHMTEhd0O4cpUHr/e/BUM= rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8= sigs.k8s.io/structured-merge-diff v0.0.0-20190525122527-15d366b2352e/go.mod h1:wWxsB5ozmmv/SG7nM11ayaAW51xMvak/t1r0CSlcokI= sigs.k8s.io/yaml v1.1.0 h1:4A07+ZFc2wgJwo8YNlQpr1rVlgUDlxXHhPJciaPY5gs=