gaoxinge commented on a change in pull request #1010:
URL: https://github.com/apache/dubbo-go/pull/1010#discussion_r586977500
##########
File path: metadata/report/zookeeper/report.go
##########
@@ -22,14 +22,16 @@ import (
"time"
)
+import (
+ gxzookeeper "github.com/dubbogo/gost/database/kv/zk"
+)
import (
Review comment:
add blank line between two imports
##########
File path: registry/zookeeper/service_discovery.go
##########
@@ -19,15 +19,15 @@ package zookeeper
import (
"fmt"
+ gxzookeeper "github.com/dubbogo/gost/database/kv/zk"
Review comment:
move to sencond import block
##########
File path: remoting/exchange_client.go
##########
@@ -21,6 +21,9 @@ import (
"time"
)
+import (
+ uatomic "go.uber.org/atomic"
+)
import (
Review comment:
add blank line between two import
##########
File path: protocol/dubbo/dubbo_invoker.go
##########
@@ -196,8 +196,12 @@ func (di *DubboInvoker) Destroy() {
di.BaseInvoker.Destroy()
client := di.getClient()
if client != nil {
+ activeNumber := client.DecreaseActiveNumber()
di.setClient(nil)
- client.Close()
+ if activeNumber == 0 {
Review comment:
- If destroy method is responsible for client close, then we should
close client whatever active number equals or not equals to zero.
- If destroy method is not responsible for client close, then we should not
close client in destroy method.
So
- Should we close client in destroy method?
- If we should, should we close client after active number check?
##########
File path: registry/zookeeper/registry.go
##########
@@ -129,7 +130,7 @@ func (r *zkRegistry) InitListeners() {
defer oldDataListener.mutex.Unlock()
r.dataListener.closed = true
recovered := r.dataListener.subscribed
- if len(recovered) > 0 {
+ if recovered != nil && len(recovered) > 0 {
Review comment:
only need `len(recovered) > 0`
##########
File path: registry/zookeeper/registry.go
##########
@@ -297,9 +294,9 @@ func (r *zkRegistry) getCloseListener(conf *common.URL)
(*RegistryConfigurationL
r.dataListener.mutex.Lock()
configurationListener := r.dataListener.subscribed[conf.ServiceKey()]
if configurationListener != nil {
- rcListener, _ :=
configurationListener.(*RegistryConfigurationListener)
- if rcListener != nil {
- if rcListener.isClosed {
+ zkListener, _ =
configurationListener.(*RegistryConfigurationListener)
+ if zkListener != nil {
+ if zkListener.isClosed {
Review comment:
merge two lines to one line: `if zkListener != nil &&
zkListener.isClosed`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
