nanjiek commented on code in PR #777:
URL: https://github.com/apache/dubbo-go-pixiu/pull/777#discussion_r2571402414


##########
pkg/common/router/router.go:
##########
@@ -37,136 +38,213 @@ import (
        "github.com/apache/dubbo-go-pixiu/pkg/server"
 )
 
-type (
-       // RouterCoordinator the router coordinator for http connection manager
-       RouterCoordinator struct {
-               activeConfig *model.RouteConfiguration
-               rw           sync.RWMutex
-       }
-)
+// RouterCoordinator the router coordinator for http connection manager
+type RouterCoordinator struct {
+       mainSnapshot atomic.Pointer[model.RouteSnapshot] // atomic snapshot
+       mu           sync.Mutex
+
+       nextSnapshot []*model.Router // temp store for dynamic update, DO NOT 
read directly
+
+       timer    *time.Timer   // debounce timer
+       debounce time.Duration // merge window, default 50ms
+}
 
 // CreateRouterCoordinator create coordinator for http connection manager
 func CreateRouterCoordinator(routeConfig *model.RouteConfiguration) 
*RouterCoordinator {
-       rc := &RouterCoordinator{activeConfig: routeConfig}
+       rc := &RouterCoordinator{
+               nextSnapshot: make([]*model.Router, 0, len(routeConfig.Routes)),
+               debounce:     50 * time.Millisecond, // merge window
+       }
        if routeConfig.Dynamic {
                server.GetRouterManager().AddRouterListener(rc)
        }
-       rc.initTrie()
-       rc.initRegex()
+       // build initial config and store snapshot
+       
rc.mainSnapshot.Store(model.ToSnapshot(buildRouteConfiguration(routeConfig.Routes).Routes))
+       // copy initial routes to store, keep origin order
+       rc.nextSnapshot = append(rc.nextSnapshot, routeConfig.Routes...)
        return rc
 }
 
-// Route find routeAction for request
 func (rm *RouterCoordinator) Route(hc *http.HttpContext) (*model.RouteAction, 
error) {
-       rm.rw.RLock()
-       defer rm.rw.RUnlock()
-
        return rm.route(hc.Request)
 }
 
 func (rm *RouterCoordinator) RouteByPathAndName(path, method string) 
(*model.RouteAction, error) {
-       rm.rw.RLock()
-       defer rm.rw.RUnlock()
-
-       return rm.activeConfig.RouteByPathAndMethod(path, method)
+       s := rm.mainSnapshot.Load()
+       if s == nil {
+               return nil, errors.New("router configuration is empty")
+       }
+       t := s.MethodTries[method]
+       if t == nil {
+               return nil, errors.Errorf("route failed for %s, no rules 
matched", stringutil.GetTrieKey(method, path))
+       }
+       node, _, ok := t.Match(stringutil.GetTrieKey(method, path))
+       if !ok || node == nil || node.GetBizInfo() == nil {
+               return nil, errors.Errorf("route failed for %s, no rules 
matched", stringutil.GetTrieKey(method, path))
+       }
+       act, ok := node.GetBizInfo().(model.RouteAction)
+       if !ok {
+               return nil, errors.Errorf("route failed for %s, invalid route 
action type", stringutil.GetTrieKey(method, path))
+       }
+       return &act, nil
 }
 
 func (rm *RouterCoordinator) route(req *stdHttp.Request) (*model.RouteAction, 
error) {
-       // match those route that only contains headers first
-       var matched []*model.Router
-       for _, route := range rm.activeConfig.Routes {
-               if len(route.Match.Prefix) > 0 {
+       s := rm.mainSnapshot.Load()
+       if s == nil {
+               return nil, errors.New("router configuration is empty")
+       }
+
+       // header-only first
+       for _, hr := range s.HeaderOnly {
+               if !model.MethodAllowed(hr.Methods, req.Method) {
                        continue
                }
-               if route.Match.MatchHeader(req) {
-                       matched = append(matched, route)
+               if matchHeaders(hr.Headers, req) {
+                       return &hr.Action, nil

Review Comment:
   Add the check for Action.Cluster
   
   ```
                if matchHeaders(hr.Headers, req) {
                        if len(hr.Action.Cluster) == 0 {
                                return nil, errors.New(action is nil. please 
check your configuration.)
                        }
                        return &hr.Action, nil
                }
   
   ```



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

To unsubscribe, e-mail: [email protected]

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