[GitHub] [dubbo-go] jack15083 commented on a change in pull request #985: Fix: consumer invoker cache set nil after the ZK connection is lost

[GitBox <gi...@apache.org>]

jack15083 commented on a change in pull request #985:
URL: https://github.com/apache/dubbo-go/pull/985#discussion_r561639948



##########
File path: remoting/zookeeper/listener.go
##########
@@ -301,11 +299,8 @@ func (l *ZkEventListener) listenDirEvent(conf *common.URL, zkPath string, listen
 			go func(zkPath string, listener remoting.DataListener) {
 				if l.listenServiceNodeEvent(zkPath) {
 					listener.DataChange(remoting.Event{Path: zkPath, Action: remoting.EventTypeDel})
-					l.pathMapLock.Lock()
-					defer l.pathMapLock.Unlock()

Review comment:
       因为我把delete 逻辑放在了 listenServiceNodeEvent 方法里，放在这里会比较好，因为有其它地方会监听deleteNode消息，如handleZkNodeEvent也会监听，放在这个地方改的并不全

##########
File path: remoting/zookeeper/listener.go
##########
@@ -113,6 +113,9 @@ func (l *ZkEventListener) listenServiceNodeEvent(zkPath string, listener ...remo
 			case zk.EventNotWatching:
 				logger.Warnf("zk.ExistW(key{%s}) = event{EventNotWatching}", zkPath)
 			case zk.EventNodeDeleted:
+				l.pathMapLock.Lock()
+				delete(l.pathMap, zkPath)
+				l.pathMapLock.Unlock()

Review comment:
       不太明白你的意思  ExistW 并没有报错，这个改动我已经在公司的项目中验证是可行的，并没有error级别的报错参见下面这个改动
   ![image](https://user-images.githubusercontent.com/2142066/105314630-28894080-5bf9-11eb-8213-75693c365b97.png)
   

##########
File path: remoting/zookeeper/client.go
##########
@@ -259,14 +259,14 @@ func (z *ZookeeperClient) HandleZkEvent(session <-chan zk.Event) {
 			switch (int)(event.State) {
 			case (int)(zk.StateDisconnected):
 				logger.Warnf("zk{addr:%s} state is StateDisconnected, so close the zk client{name:%s}.", z.ZkAddrs, z.name)
-				z.stop()
 				z.Lock()
 				conn := z.Conn
 				z.Conn = nil
 				z.Unlock()
 				if conn != nil {
 					conn.Close()
 				}
+				z.stop()

Review comment:
       是的

##########
File path: remoting/zookeeper/listener.go
##########
@@ -163,18 +167,17 @@ func (l *ZkEventListener) handleZkNodeEvent(zkPath string, children []string, li
 			logger.Errorf("Get new node path {%v} 's content error,message is  {%v}", newNode, perrors.WithStack(err))
 		}
 
-		if !listener.DataChange(remoting.Event{Path: zkPath, Action: remoting.EventTypeAdd, Content: string(content)}) {
+		if !listener.DataChange(remoting.Event{Path: newNode, Action: remoting.EventTypeAdd, Content: string(content)}) {
 			continue
 		}
 		// listen l service node
 		l.wg.Add(1)
 		go func(node string, zkPath string, listener remoting.DataListener) {
-			logger.Infof("delete zkNode{%s}", node)
 			if l.listenServiceNodeEvent(node, listener) {
-				logger.Infof("delete content{%s}", node)
-				listener.DataChange(remoting.Event{Path: zkPath, Action: remoting.EventTypeDel})
+				logger.Warnf("delete zkNode{%s}", node)
+				listener.DataChange(remoting.Event{Path: node, Action: remoting.EventTypeDel})
 			}
-			logger.Warnf("listenSelf(zk path{%s}) goroutine exit now", zkPath)
+			logger.Warnf("handleZkNodeEvent->listenSelf(zk path{%s}) goroutine exit now", zkPath)

Review comment:
       是的，这个地方我改一下




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org