AlexStocks commented on a change in pull request #708:
URL: https://github.com/apache/dubbo-go/pull/708#discussion_r470196768



##########
File path: cluster/router/chain/chain.go
##########
@@ -79,6 +105,104 @@ func (c *RouterChain) AddRouters(routers 
[]router.PriorityRouter) {
        c.routers = newRouters
 }
 
+// SetInvokers receives updated invokers from registry center. If the times of 
notification exceeds countThreshold and
+// time interval exceeds timeThreshold since last cache update, then notify to 
update the cache.
+func (c *RouterChain) SetInvokers(invokers []protocol.Invoker) {
+       c.mutex.Lock()
+       c.invokers = invokers
+       c.mutex.Unlock()
+
+       c.count++
+       now := time.Now()
+       if c.count >= countThreshold && now.Sub(c.last) >= timeThreshold {
+               c.last = now
+               c.count = 0
+               go func() {
+                       c.ch <- struct{}{}
+               }()
+       }
+}
+
+// loop listens on events to update the address cache when it's necessary, 
either when it receives notification
+// from address update, or when timeInterval exceeds.
+func (c *RouterChain) loop() {
+       for {
+               select {
+               case <-time.Tick(timeInterval):
+                       c.buildCache()
+               case <-c.ch:
+                       c.buildCache()
+               }
+       }
+}
+
+// copyRouters make a snapshot copy from RouterChain's router list.
+func (c *RouterChain) copyRouters() []router.PriorityRouter {
+       c.mutex.RLock()
+       ret := copySlice(c.routers)
+       c.mutex.RUnlock()
+       return ret.([]router.PriorityRouter)
+}
+
+// copyInvokers copies a snapshot of the received invokers.
+func (c *RouterChain) copyInvokers() []protocol.Invoker {
+       c.mutex.RLock()
+       if c.invokers == nil || len(c.invokers) == 0 {

Review comment:
       OMG, u return the func value without release the lock!

##########
File path: cluster/router/chain/chain.go
##########
@@ -79,6 +105,104 @@ func (c *RouterChain) AddRouters(routers 
[]router.PriorityRouter) {
        c.routers = newRouters
 }
 
+// SetInvokers receives updated invokers from registry center. If the times of 
notification exceeds countThreshold and
+// time interval exceeds timeThreshold since last cache update, then notify to 
update the cache.
+func (c *RouterChain) SetInvokers(invokers []protocol.Invoker) {
+       c.mutex.Lock()
+       c.invokers = invokers
+       c.mutex.Unlock()
+
+       c.count++
+       now := time.Now()
+       if c.count >= countThreshold && now.Sub(c.last) >= timeThreshold {
+               c.last = now
+               c.count = 0
+               go func() {
+                       c.ch <- struct{}{}
+               }()
+       }
+}
+
+// loop listens on events to update the address cache when it's necessary, 
either when it receives notification
+// from address update, or when timeInterval exceeds.
+func (c *RouterChain) loop() {
+       for {
+               select {
+               case <-time.Tick(timeInterval):
+                       c.buildCache()
+               case <-c.ch:
+                       c.buildCache()
+               }
+       }
+}
+
+// copyRouters make a snapshot copy from RouterChain's router list.
+func (c *RouterChain) copyRouters() []router.PriorityRouter {
+       c.mutex.RLock()

Review comment:
       hey, guy, u should using unlock in this way `defer c.mutex.RUnlock()` to 
release the lock safely.

##########
File path: cluster/router/chain/chain.go
##########
@@ -79,6 +105,104 @@ func (c *RouterChain) AddRouters(routers 
[]router.PriorityRouter) {
        c.routers = newRouters
 }
 
+// SetInvokers receives updated invokers from registry center. If the times of 
notification exceeds countThreshold and
+// time interval exceeds timeThreshold since last cache update, then notify to 
update the cache.
+func (c *RouterChain) SetInvokers(invokers []protocol.Invoker) {
+       c.mutex.Lock()
+       c.invokers = invokers
+       c.mutex.Unlock()
+
+       c.count++
+       now := time.Now()
+       if c.count >= countThreshold && now.Sub(c.last) >= timeThreshold {
+               c.last = now
+               c.count = 0
+               go func() {
+                       c.ch <- struct{}{}
+               }()
+       }
+}
+
+// loop listens on events to update the address cache when it's necessary, 
either when it receives notification
+// from address update, or when timeInterval exceeds.
+func (c *RouterChain) loop() {
+       for {
+               select {
+               case <-time.Tick(timeInterval):
+                       c.buildCache()
+               case <-c.ch:
+                       c.buildCache()
+               }
+       }
+}
+
+// copyRouters make a snapshot copy from RouterChain's router list.
+func (c *RouterChain) copyRouters() []router.PriorityRouter {
+       c.mutex.RLock()
+       ret := copySlice(c.routers)
+       c.mutex.RUnlock()
+       return ret.([]router.PriorityRouter)
+}
+
+// copyInvokers copies a snapshot of the received invokers.
+func (c *RouterChain) copyInvokers() []protocol.Invoker {
+       c.mutex.RLock()

Review comment:
       defer c.mutex.RUnlock()

##########
File path: cluster/router/chain/chain.go
##########
@@ -48,20 +57,37 @@ type RouterChain struct {
        mutex sync.RWMutex
 
        url common.URL
+
+       // The times of address notification since last update for address cache
+       count int64
+       // The timestamp of last update for address cache
+       last time.Time
+       // Channel for notify to update the address cache
+       ch chan struct{}

Review comment:
       pls change this var name to 'notify'.

##########
File path: cluster/router/chain/invoker_cache.go
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package chain
+
+import (
+       "github.com/RoaringBitmap/roaring"
+)
+
+import (
+       "github.com/apache/dubbo-go/cluster/router"
+       "github.com/apache/dubbo-go/cluster/router/utils"
+       "github.com/apache/dubbo-go/protocol"
+)
+
+// Cache caches all addresses relevant info for a snapshot of received 
invokers. It keeps a snapshot of the received
+// address list, and also keeps address pools and address metadata from 
routers based on the same address snapshot, if
+// the router implements Poolable.
+type InvokerCache struct {
+       // The snapshot of invokers
+       invokers []protocol.Invoker
+
+       // The bitmap representation for invokers snapshot
+       bitmap *roaring.Bitmap
+
+       // Address pool from routers which implement Poolable
+       pools map[string]router.AddrPool
+
+       // Address metadata from routers which implement Poolable
+       metadatas map[string]router.AddrMetadata
+}
+
+// BuildCache builds address cache from the given invokers.
+func BuildCache(invokers []protocol.Invoker) *InvokerCache {
+       return &InvokerCache{
+               invokers:  invokers,
+               bitmap:    utils.ToBitmap(invokers),
+               pools:     make(map[string]router.AddrPool),

Review comment:
       pls remember that u should set a init length for map when u init a map 
var.
   
   ```Go
   pools:     make(map[string]router.AddrPool, 8),
   metadatas: make(map[string]router.AddrMetadata, 8),
   ```

##########
File path: cluster/router/router.go
##########
@@ -50,3 +55,39 @@ type PriorityRouter interface {
        // 0 to ^int(0) is better
        Priority() int64
 }
+
+// Poolable caches address pool and address metadata for a router instance 
which will be used later in Router's Route.
+type Poolable interface {

Review comment:
       got it.




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

Reply via email to