You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2020/09/07 10:07:36 UTC

[GitHub] [dubbo-go] beiwei30 opened a new pull request #741: Address notification batch mode

beiwei30 opened a new pull request #741:
URL: https://github.com/apache/dubbo-go/pull/741


   <!--  Thanks for sending a pull request! 
   -->
   
   **What this PR does**:
   
   **Which issue(s) this PR fixes**:
   <!--
   *Automatically closes linked issue when PR is merged.
   Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
   _If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
   -->
   Fixes #
   
   **Special notes for your reviewer**:
   
   **Does this PR introduce a user-facing change?**:
   <!--
   If no, just write "NONE" in the release-note block below.
   If yes, a release note is required:
   Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
   -->
   ```release-note
   
   ```


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

[GitHub] [dubbo-go] AlexStocks commented on a change in pull request #741: Address notification batch mode

Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #741:
URL: https://github.com/apache/dubbo-go/pull/741#discussion_r484366009



##########
File path: registry/directory/directory.go
##########
@@ -104,72 +104,129 @@ func (dir *RegistryDirectory) subscribe(url *common.URL) {
 }
 
 // Notify monitor changes from registry,and update the cacheServices
-func (dir *RegistryDirectory) Notify(event *registry.ServiceEvent) {
-	go dir.update(event)
+func (dir *RegistryDirectory) Notify(event ...*registry.ServiceEvent) {
+	go dir.update(event...)
 }
 
 // update the cacheServices and subscribe service from registry
-func (dir *RegistryDirectory) update(res *registry.ServiceEvent) {
-	if res == nil {
-		return
+func (dir *RegistryDirectory) update(events ...*registry.ServiceEvent) {
+	for _, event := range events {
+		logger.Debugf("registry update, result{%s}", event)
+		logger.Debugf("update service name: %s!", event.Service)
 	}
-	logger.Debugf("registry update, result{%s}", res)
-	logger.Debugf("update service name: %s!", res.Service)
-	dir.refreshInvokers(res)
+
+	dir.refreshInvokers(events...)
 }
 
-func (dir *RegistryDirectory) refreshInvokers(res *registry.ServiceEvent) {
-	var (
-		url        *common.URL
-		oldInvoker protocol.Invoker = nil
-	)
-	// judge is override or others
-	if res != nil {
-		url = &res.Service
-		// 1.for override url in 2.6.x
-		if url.Protocol == constant.OVERRIDE_PROTOCOL ||
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.CONFIGURATORS_CATEGORY {
-			dir.configurators = append(dir.configurators, extension.GetDefaultConfigurator(url))
-			url = nil
-		} else if url.Protocol == constant.ROUTER_PROTOCOL || // 2.for router
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.ROUTER_CATEGORY {
-			url = nil
+// refreshInvokers refreshes service's events. It supports two modes: incremental mode and batch mode. If a single
+// service event is passed in, then it is incremental mode, and if an array of service events are passed in, it is
+// batch mode, in this mode, we assume the registry center have the complete list of the service events, therefore
+// in this case, we can safely assume any cached invoker not in the incoming list can be removed. It is necessary
+// since in batch mode, the register center handles the different type of events by itself, then notify the directory
+// a batch of 'Update' events, instead of omit the different type of event one by one.
+func (dir *RegistryDirectory) refreshInvokers(events ...*registry.ServiceEvent) {
+	var oldInvokers []protocol.Invoker
+
+	// in batch mode, it is safe to remove since we have the complete list of events.
+	if events != nil && len(events) > 1 {

Review comment:
       events != nil && len(events) > 1 ==> len(events) > 1

##########
File path: registry/protocol/protocol.go
##########
@@ -242,7 +242,12 @@ func newOverrideSubscribeListener(overriderUrl *common.URL, invoker protocol.Inv
 }
 
 // Notify will be triggered when a service change notification is received.
-func (nl *overrideSubscribeListener) Notify(event *registry.ServiceEvent) {
+func (nl *overrideSubscribeListener) Notify(events ...*registry.ServiceEvent) {
+	if events == nil || len(events) == 0 {

Review comment:
       events == nil || len(events) == 0 ==> len(events) == 0




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

[GitHub] [dubbo-go] beiwei30 commented on a change in pull request #741: Address notification batch mode

Posted by GitBox <gi...@apache.org>.
beiwei30 commented on a change in pull request #741:
URL: https://github.com/apache/dubbo-go/pull/741#discussion_r484664193



##########
File path: registry/directory/directory.go
##########
@@ -104,72 +104,129 @@ func (dir *RegistryDirectory) subscribe(url *common.URL) {
 }
 
 // Notify monitor changes from registry,and update the cacheServices
-func (dir *RegistryDirectory) Notify(event *registry.ServiceEvent) {
-	go dir.update(event)
+func (dir *RegistryDirectory) Notify(event ...*registry.ServiceEvent) {
+	go dir.update(event...)
 }
 
 // update the cacheServices and subscribe service from registry
-func (dir *RegistryDirectory) update(res *registry.ServiceEvent) {
-	if res == nil {
-		return
+func (dir *RegistryDirectory) update(events ...*registry.ServiceEvent) {
+	for _, event := range events {
+		logger.Debugf("registry update, result{%s}", event)
+		logger.Debugf("update service name: %s!", event.Service)
 	}
-	logger.Debugf("registry update, result{%s}", res)
-	logger.Debugf("update service name: %s!", res.Service)
-	dir.refreshInvokers(res)
+
+	dir.refreshInvokers(events...)
 }
 
-func (dir *RegistryDirectory) refreshInvokers(res *registry.ServiceEvent) {
-	var (
-		url        *common.URL
-		oldInvoker protocol.Invoker = nil
-	)
-	// judge is override or others
-	if res != nil {
-		url = &res.Service
-		// 1.for override url in 2.6.x
-		if url.Protocol == constant.OVERRIDE_PROTOCOL ||
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.CONFIGURATORS_CATEGORY {
-			dir.configurators = append(dir.configurators, extension.GetDefaultConfigurator(url))
-			url = nil
-		} else if url.Protocol == constant.ROUTER_PROTOCOL || // 2.for router
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.ROUTER_CATEGORY {
-			url = nil
+// refreshInvokers refreshes service's events. It supports two modes: incremental mode and batch mode. If a single
+// service event is passed in, then it is incremental mode, and if an array of service events are passed in, it is
+// batch mode, in this mode, we assume the registry center have the complete list of the service events, therefore
+// in this case, we can safely assume any cached invoker not in the incoming list can be removed. It is necessary
+// since in batch mode, the register center handles the different type of events by itself, then notify the directory
+// a batch of 'Update' events, instead of omit the different type of event one by one.
+func (dir *RegistryDirectory) refreshInvokers(events ...*registry.ServiceEvent) {
+	var oldInvokers []protocol.Invoker
+
+	// in batch mode, it is safe to remove since we have the complete list of events.
+	if len(events) > 1 {
+		dir.cacheInvokersMap.Range(func(k, v interface{}) bool {
+			if !dir.eventMatched(k.(string), events) {
+				if invoker := dir.uncacheInvokerWithKey(k.(string)); invoker != nil {
+					oldInvokers = append(oldInvokers, invoker)
+				}
+			}
+			return true
+		})
+	}
 
+	for _, event := range events {
+		if oldInvoker, _ := dir.cacheInvokerByEvent(event); oldInvoker != nil {
+			oldInvokers = append(oldInvokers, oldInvoker)

Review comment:
       done.




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

[GitHub] [dubbo-go] zouyx commented on a change in pull request #741: Address notification batch mode

Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #741:
URL: https://github.com/apache/dubbo-go/pull/741#discussion_r484619015



##########
File path: registry/directory/directory.go
##########
@@ -104,72 +104,129 @@ func (dir *RegistryDirectory) subscribe(url *common.URL) {
 }
 
 // Notify monitor changes from registry,and update the cacheServices
-func (dir *RegistryDirectory) Notify(event *registry.ServiceEvent) {
-	go dir.update(event)
+func (dir *RegistryDirectory) Notify(event ...*registry.ServiceEvent) {
+	go dir.update(event...)
 }
 
 // update the cacheServices and subscribe service from registry
-func (dir *RegistryDirectory) update(res *registry.ServiceEvent) {
-	if res == nil {
-		return
+func (dir *RegistryDirectory) update(events ...*registry.ServiceEvent) {
+	for _, event := range events {
+		logger.Debugf("registry update, result{%s}", event)
+		logger.Debugf("update service name: %s!", event.Service)

Review comment:
       I think should move this block to line 142?
   
   https://github.com/apache/dubbo-go/pull/741/files#diff-f1103e9118b5d7dcaaa8b1dac15314d7R142

##########
File path: registry/directory/directory.go
##########
@@ -104,72 +104,129 @@ func (dir *RegistryDirectory) subscribe(url *common.URL) {
 }
 
 // Notify monitor changes from registry,and update the cacheServices
-func (dir *RegistryDirectory) Notify(event *registry.ServiceEvent) {
-	go dir.update(event)
+func (dir *RegistryDirectory) Notify(event ...*registry.ServiceEvent) {
+	go dir.update(event...)
 }
 
 // update the cacheServices and subscribe service from registry
-func (dir *RegistryDirectory) update(res *registry.ServiceEvent) {
-	if res == nil {
-		return
+func (dir *RegistryDirectory) update(events ...*registry.ServiceEvent) {
+	for _, event := range events {
+		logger.Debugf("registry update, result{%s}", event)
+		logger.Debugf("update service name: %s!", event.Service)
 	}
-	logger.Debugf("registry update, result{%s}", res)
-	logger.Debugf("update service name: %s!", res.Service)
-	dir.refreshInvokers(res)
+
+	dir.refreshInvokers(events...)
 }
 
-func (dir *RegistryDirectory) refreshInvokers(res *registry.ServiceEvent) {
-	var (
-		url        *common.URL
-		oldInvoker protocol.Invoker = nil
-	)
-	// judge is override or others
-	if res != nil {
-		url = &res.Service
-		// 1.for override url in 2.6.x
-		if url.Protocol == constant.OVERRIDE_PROTOCOL ||
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.CONFIGURATORS_CATEGORY {
-			dir.configurators = append(dir.configurators, extension.GetDefaultConfigurator(url))
-			url = nil
-		} else if url.Protocol == constant.ROUTER_PROTOCOL || // 2.for router
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.ROUTER_CATEGORY {
-			url = nil
+// refreshInvokers refreshes service's events. It supports two modes: incremental mode and batch mode. If a single
+// service event is passed in, then it is incremental mode, and if an array of service events are passed in, it is
+// batch mode, in this mode, we assume the registry center have the complete list of the service events, therefore
+// in this case, we can safely assume any cached invoker not in the incoming list can be removed. It is necessary
+// since in batch mode, the register center handles the different type of events by itself, then notify the directory
+// a batch of 'Update' events, instead of omit the different type of event one by one.
+func (dir *RegistryDirectory) refreshInvokers(events ...*registry.ServiceEvent) {
+	var oldInvokers []protocol.Invoker
+
+	// in batch mode, it is safe to remove since we have the complete list of events.
+	if len(events) > 1 {
+		dir.cacheInvokersMap.Range(func(k, v interface{}) bool {
+			if !dir.eventMatched(k.(string), events) {
+				if invoker := dir.uncacheInvokerWithKey(k.(string)); invoker != nil {
+					oldInvokers = append(oldInvokers, invoker)
+				}
+			}
+			return true
+		})
+	}
 
+	for _, event := range events {
+		if oldInvoker, _ := dir.cacheInvokerByEvent(event); oldInvoker != nil {
+			oldInvokers = append(oldInvokers, oldInvoker)

Review comment:
       ```suggestion
   	for _, event := range events {
   	        logger.Debugf("registry update, result{%s}", event)
   		logger.Debugf("update service name: %s!", event.Service)
   		if oldInvoker, _ := dir.cacheInvokerByEvent(event); oldInvoker != nil {
   			oldInvokers = append(oldInvokers, oldInvoker)
   ```




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

[GitHub] [dubbo-go] zouyx merged pull request #741: Address notification batch mode

Posted by GitBox <gi...@apache.org>.
zouyx merged pull request #741:
URL: https://github.com/apache/dubbo-go/pull/741


   


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

[GitHub] [dubbo-go] beiwei30 commented on a change in pull request #741: Address notification batch mode

Posted by GitBox <gi...@apache.org>.
beiwei30 commented on a change in pull request #741:
URL: https://github.com/apache/dubbo-go/pull/741#discussion_r484664112



##########
File path: registry/directory/directory.go
##########
@@ -104,72 +104,129 @@ func (dir *RegistryDirectory) subscribe(url *common.URL) {
 }
 
 // Notify monitor changes from registry,and update the cacheServices
-func (dir *RegistryDirectory) Notify(event *registry.ServiceEvent) {
-	go dir.update(event)
+func (dir *RegistryDirectory) Notify(event ...*registry.ServiceEvent) {
+	go dir.update(event...)
 }
 
 // update the cacheServices and subscribe service from registry
-func (dir *RegistryDirectory) update(res *registry.ServiceEvent) {
-	if res == nil {
-		return
+func (dir *RegistryDirectory) update(events ...*registry.ServiceEvent) {
+	for _, event := range events {
+		logger.Debugf("registry update, result{%s}", event)
+		logger.Debugf("update service name: %s!", event.Service)

Review comment:
       done.




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

[GitHub] [dubbo-go] beiwei30 commented on a change in pull request #741: Address notification batch mode

Posted by GitBox <gi...@apache.org>.
beiwei30 commented on a change in pull request #741:
URL: https://github.com/apache/dubbo-go/pull/741#discussion_r484385068



##########
File path: registry/directory/directory.go
##########
@@ -104,72 +104,129 @@ func (dir *RegistryDirectory) subscribe(url *common.URL) {
 }
 
 // Notify monitor changes from registry,and update the cacheServices
-func (dir *RegistryDirectory) Notify(event *registry.ServiceEvent) {
-	go dir.update(event)
+func (dir *RegistryDirectory) Notify(event ...*registry.ServiceEvent) {
+	go dir.update(event...)
 }
 
 // update the cacheServices and subscribe service from registry
-func (dir *RegistryDirectory) update(res *registry.ServiceEvent) {
-	if res == nil {
-		return
+func (dir *RegistryDirectory) update(events ...*registry.ServiceEvent) {
+	for _, event := range events {
+		logger.Debugf("registry update, result{%s}", event)
+		logger.Debugf("update service name: %s!", event.Service)
 	}
-	logger.Debugf("registry update, result{%s}", res)
-	logger.Debugf("update service name: %s!", res.Service)
-	dir.refreshInvokers(res)
+
+	dir.refreshInvokers(events...)
 }
 
-func (dir *RegistryDirectory) refreshInvokers(res *registry.ServiceEvent) {
-	var (
-		url        *common.URL
-		oldInvoker protocol.Invoker = nil
-	)
-	// judge is override or others
-	if res != nil {
-		url = &res.Service
-		// 1.for override url in 2.6.x
-		if url.Protocol == constant.OVERRIDE_PROTOCOL ||
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.CONFIGURATORS_CATEGORY {
-			dir.configurators = append(dir.configurators, extension.GetDefaultConfigurator(url))
-			url = nil
-		} else if url.Protocol == constant.ROUTER_PROTOCOL || // 2.for router
-			url.GetParam(constant.CATEGORY_KEY, constant.DEFAULT_CATEGORY) == constant.ROUTER_CATEGORY {
-			url = nil
+// refreshInvokers refreshes service's events. It supports two modes: incremental mode and batch mode. If a single
+// service event is passed in, then it is incremental mode, and if an array of service events are passed in, it is
+// batch mode, in this mode, we assume the registry center have the complete list of the service events, therefore
+// in this case, we can safely assume any cached invoker not in the incoming list can be removed. It is necessary
+// since in batch mode, the register center handles the different type of events by itself, then notify the directory
+// a batch of 'Update' events, instead of omit the different type of event one by one.
+func (dir *RegistryDirectory) refreshInvokers(events ...*registry.ServiceEvent) {
+	var oldInvokers []protocol.Invoker
+
+	// in batch mode, it is safe to remove since we have the complete list of events.
+	if events != nil && len(events) > 1 {

Review comment:
       done.

##########
File path: registry/protocol/protocol.go
##########
@@ -242,7 +242,12 @@ func newOverrideSubscribeListener(overriderUrl *common.URL, invoker protocol.Inv
 }
 
 // Notify will be triggered when a service change notification is received.
-func (nl *overrideSubscribeListener) Notify(event *registry.ServiceEvent) {
+func (nl *overrideSubscribeListener) Notify(events ...*registry.ServiceEvent) {
+	if events == nil || len(events) == 0 {

Review comment:
       done.




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