[GitHub] [dubbo-go] pantianying commented on a change in pull request #510: Ftr : add UnRegister and UnSubscribe for zookeeper Registry

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

pantianying commented on a change in pull request #510:
URL: https://github.com/apache/dubbo-go/pull/510#discussion_r426146354



##########
File path: registry/base_registry.go
##########
@@ -323,39 +376,54 @@ func sleepWait(n int) {
 }
 
 // Subscribe :subscribe from registry, event will notify by notifyListener
-func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener) {
+func (r *BaseRegistry) Subscribe(url *common.URL, notifyListener NotifyListener) error {
 	n := 0
 	for {
 		n++
+		err := r.processNotify(url, notifyListener, r.facadeBasedRegistry.DoSubscribe)
+		if err == nil {
+			return nil
+		}
+		sleepWait(n)
+	}
+}
+
+// UnSubscribe :UnSubscribeURL
+func (r *BaseRegistry) UnSubscribe(url *common.URL, notifyListener NotifyListener) error {
+	return r.processNotify(url, notifyListener, r.facadeBasedRegistry.DoUnsubscribe)
+}
+
+// processNotify can process notify listener when Subscribe or UnSubscribe
+func (r *BaseRegistry) processNotify(url *common.URL, notifyListener NotifyListener, f func(conf *common.URL) (Listener, error)) error {
+
+	if !r.IsAvailable() {
+		logger.Warnf("event listener game over.")
+		return nil
+	}
+
+	listener, err := f(url)
+	if err != nil {
 		if !r.IsAvailable() {
 			logger.Warnf("event listener game over.")
-			return
+			return nil
 		}
+		logger.Warnf("getListener() = err:%v", perrors.WithStack(err))
+		time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
+		return perrors.WithStack(err)
+	}
 
-		listener, err := r.facadeBasedRegistry.DoSubscribe(url)
-		if err != nil {
-			if !r.IsAvailable() {
-				logger.Warnf("event listener game over.")
-				return
-			}
-			logger.Warnf("getListener() = err:%v", perrors.WithStack(err))
-			time.Sleep(time.Duration(RegistryConnDelay) * time.Second)
-			continue
+	for {
+		if serviceEvent, err := listener.Next(); err != nil {
+			logger.Warnf("Selector.watch() = error{%v}", perrors.WithStack(err))
+			listener.Close()
+			break

Review comment:
       这里break，return nil会造成整个Subscribe退出 ，这样会造成zk重连后丢失Subscribe




----------------------------------------------------------------
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