From 94496c5a803713aaa45677f8e61ddf8cb62e4c2e Mon Sep 17 00:00:00 2001
From: cvictory <shenglicao2@gmail.com>
Date: Wed, 23 Sep 2020 20:41:36 +0800
Subject: [PATCH] fix review issue: add constraint of Action value of
ServiceEvent in NotifyAll func
---
registry/directory/directory.go | 59 ++++++++++++++++++++-------------
registry/registry.go | 4 +--
2 files changed, 38 insertions(+), 25 deletions(-)
diff --git a/registry/directory/directory.go b/registry/directory/directory.go
index 148483604..c78a9f1bd 100644
--- a/registry/directory/directory.go
+++ b/registry/directory/directory.go
@@ -67,6 +67,7 @@ type RegistryDirectory struct {
referenceConfigurationListener *referenceConfigurationListener
serviceKey string
forbidden atomic.Bool
+ registerLock sync.Mutex // this lock if for register
}
// NewRegistryDirectory will create a new RegistryDirectory
@@ -111,6 +112,7 @@ func (dir *RegistryDirectory) Notify(event *registry.ServiceEvent) {
go dir.refreshInvokers(event)
}
+// NotifyAll notify the events that are complete Service Event List.
func (dir *RegistryDirectory) NotifyAll(events []*registry.ServiceEvent) {
go dir.refreshAllInvokers(events)
}
@@ -129,38 +131,49 @@ func (dir *RegistryDirectory) refreshInvokers(event *registry.ServiceEvent) {
}
// refreshAllInvokers the argument is the complete list of the service events, we can safely assume any cached invoker
-// not in the incoming list can be removed. It will ignore Action of serviceEvent.
+// not in the incoming list can be removed. The Action of serviceEvent should be EventTypeUpdate.
func (dir *RegistryDirectory) refreshAllInvokers(events []*registry.ServiceEvent) {
var (
oldInvokers []protocol.Invoker
addEvents []*registry.ServiceEvent
)
-
- // get need clear invokers from original invoker list
- dir.cacheInvokersMap.Range(func(k, v interface{}) bool {
- if !dir.eventMatched(k.(string), events) {
- // delete unused invoker from cache
- if invoker := dir.uncacheInvokerWithKey(k.(string)); invoker != nil {
- oldInvokers = append(oldInvokers, invoker)
- }
- }
- return true
- })
- // get need add invokers from events
+ // loop the events to check the Action should be EventTypeUpdate.
for _, event := range events {
- // Is the key (url.Key()) of cacheInvokersMap the best way?
- if _, ok := dir.cacheInvokersMap.Load(event.Service.Key()); !ok {
- event.Action = remoting.EventTypeAdd
- addEvents = append(addEvents, event)
+ if event.Action != remoting.EventTypeUpdate {
+ panic("Your implements of register center is wrong, " +
+ "please check the Action of ServiceEvent should be EventTypeUpdate")
+ return
}
}
- // loop the addEvents
- for _, event := range addEvents {
- logger.Debugf("registry update, result{%s}", event)
- if oldInvoker, _ := dir.cacheInvokerByEvent(event); oldInvoker != nil {
- oldInvokers = append(oldInvokers, oldInvoker)
+ func() {
+ // this lock is work at batch update of InvokeCache
+ dir.registerLock.Lock()
+ defer dir.registerLock.Unlock()
+ // get need clear invokers from original invoker list
+ dir.cacheInvokersMap.Range(func(k, v interface{}) bool {
+ if !dir.eventMatched(k.(string), events) {
+ // delete unused invoker from cache
+ if invoker := dir.uncacheInvokerWithKey(k.(string)); invoker != nil {
+ oldInvokers = append(oldInvokers, invoker)
+ }
+ }
+ return true
+ })
+ // get need add invokers from events
+ for _, event := range events {
+ // Is the key (url.Key()) of cacheInvokersMap the best way?
+ if _, ok := dir.cacheInvokersMap.Load(event.Service.Key()); !ok {
+ addEvents = append(addEvents, event)
+ }
}
- }
+ // loop the updateEvents
+ for _, event := range addEvents {
+ logger.Debugf("registry update, result{%s}", event)
+ if oldInvoker, _ := dir.cacheInvokerByEvent(event); oldInvoker != nil {
+ oldInvokers = append(oldInvokers, oldInvoker)
+ }
+ }
+ }()
dir.setNewInvokers()
// destroy unused invokers
for _, invoker := range oldInvokers {
diff --git a/registry/registry.go b/registry/registry.go
index e61576ac8..97051334d 100644
--- a/registry/registry.go
+++ b/registry/registry.go
@@ -73,8 +73,8 @@ type NotifyListener interface {
// passed in, then it's a incremental event. Pls. note when a list (instead of single event) comes,
// the impl of NotifyListener may abandon the accumulated result from previous notifications.
Notify(*ServiceEvent)
- // Notify the events are complete Service Event List.
- // The argument of events []*ServiceEvent is equal to urls []*URL, because Action of ServiceEvent will be ignored.
+ // NotifyAll the events are complete Service Event List.
+ // The argument of events []*ServiceEvent is equal to urls []*URL, The Action of serviceEvent should be EventTypeUpdate.
// If your registry center can only get all urls but can't get individual event, you should use this one.
NotifyAll([]*ServiceEvent)
}
--
GitLab