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/07/19 14:44:35 UTC
[GitHub] [dubbo-go] watermelo opened a new pull request #665: Ftr: dynamic tag router
watermelo opened a new pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665
<!-- 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] pantianying commented on a change in pull request #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
pantianying commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r465451983
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
Review comment:
The definition can be moved to the top.
And can you break this pile of code into several parts,it is hard for me 😂
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -100,3 +263,163 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
}
return false
}
+
+type filter func(protocol.Invoker) bool
+
+func filterInvoker(invokers []protocol.Invoker, filters ...filter) []protocol.Invoker {
+ var res []protocol.Invoker
+OUTER:
+ for _, invoker := range invokers {
+ for _, filter := range filters {
+ if !filter(invoker) {
+ continue OUTER
+ }
+ }
+ res = append(res, invoker)
+ }
+ return res
+}
+
+// TODO: need move to dubbogo/gost
+func checkAddressMatch(addresses []string, host, port string) bool {
+ for _, address := range addresses {
+ if matchIp(address, host, port) {
+ return true
+ }
+ if address == net.JoinHostPort(constant.ANYHOST_VALUE, port) {
+ return true
+ }
+ }
+ return false
+}
+
+func matchIp(pattern, host, port string) bool {
+ // if the pattern is subnet format, it will not be allowed to config port param in pattern.
+ if strings.Contains(pattern, "/") {
+ _, subnet, _ := net.ParseCIDR(pattern)
+ if subnet != nil && subnet.Contains(net.ParseIP(host)) {
+ return true
+ }
+ return false
+ }
+ return matchIpRange(pattern, host, port)
+}
+
+func matchIpRange(pattern, host, port string) bool {
+ if pattern == "" || host == "" {
+ logger.Error("Illegal Argument pattern or hostName. Pattern:" + pattern + ", Host:" + host)
+ return false
+ }
+
+ pattern = strings.TrimSpace(pattern)
+ if "*.*.*.*" == pattern || "*" == pattern {
+ return true
+ }
+
+ isIpv4 := true
+ ip4 := net.ParseIP(host).To4()
+
+ if ip4 == nil {
+ isIpv4 = false
+ }
+
+ hostAndPort := getPatternHostAndPort(pattern, isIpv4)
+ if hostAndPort[1] != "" && hostAndPort[1] != port {
+ return false
+ }
+
+ pattern = hostAndPort[0]
+ // TODO 常量化
Review comment:
has some easy todo
##########
File path: cluster/router/tag/tag_router_test.go
##########
@@ -19,32 +19,59 @@ package tag
import (
"context"
+ "fmt"
+ "github.com/stretchr/testify/suite"
Review comment:
pkg import problem
##########
File path: cluster/router/tag/tag_router_test.go
##########
@@ -19,32 +19,59 @@ package tag
import (
"context"
+ "fmt"
+ "github.com/stretchr/testify/suite"
Review comment:
pkg import problem
----------------------------------------------------------------
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] watermelo commented on a change in pull request #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
watermelo commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r464087572
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -22,9 +22,27 @@ import (
"github.com/apache/dubbo-go/common/yaml"
)
+/**
+ * %YAML1.2
+ * ---
+ * force: true
+ * runtime: false
+ * enabled: true
+ * priority: 1
+ * key: demo-provider
+ * tags:
+ * - name: tag1
+ * addresses: [ip1, ip2]
+ * - name: tag2
+ * addresses: [ip3, ip4]
+ * ...
+ */
// RouterRule RouterRule config read from config file or config center
type RouterRule struct {
router.BaseRouterRule `yaml:",inline""`
+ Tags []Tag
Review comment:
tags can't be yaml parse.
----------------------------------------------------------------
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 #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r464077311
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -22,9 +22,27 @@ import (
"github.com/apache/dubbo-go/common/yaml"
)
+/**
+ * %YAML1.2
+ * ---
+ * force: true
+ * runtime: false
+ * enabled: true
+ * priority: 1
+ * key: demo-provider
+ * tags:
+ * - name: tag1
+ * addresses: [ip1, ip2]
+ * - name: tag2
+ * addresses: [ip3, ip4]
+ * ...
+ */
// RouterRule RouterRule config read from config file or config center
type RouterRule struct {
router.BaseRouterRule `yaml:",inline""`
+ Tags []Tag
Review comment:
```suggestion
tags []Tag
```
you mean `tags`?
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +83,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ if tag != "" {
Review comment:
```suggestion
if len(tag)>0 {
```
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -100,3 +265,163 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
}
return false
}
+
+type filter func(protocol.Invoker) bool
+
+func filterInvoker(invokers []protocol.Invoker, filters ...filter) []protocol.Invoker {
+ var res []protocol.Invoker
+OUTER:
+ for _, invoker := range invokers {
+ for _, filter := range filters {
+ if !filter(invoker) {
+ continue OUTER
+ }
+ }
+ res = append(res, invoker)
+ }
+ return res
+}
+
+// TODO 需要搬到 dubbogo/gost, 可以先 review
Review comment:
Use english?
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -55,6 +65,16 @@ func (c *tagRouter) isEnabled() bool {
return c.enabled
}
+func (c *tagRouter) SetApplication(app string) {
+ c.application = app
+}
+
+func (c *tagRouter) tagRouterRuleCopy() RouterRule {
+ fmt.Println(c.tagRouterRule, "fuck")
Review comment:
```suggestion
fmt.Println(c.tagRouterRule, "fuck")
```
i think this feature has made you crazy ：）
##########
File path: cluster/router/tag/tag.go
##########
@@ -0,0 +1,39 @@
+/*
+ * 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 tag
+
+type Tag struct {
Review comment:
you mean `tag`?
```suggestion
type tag struct {
```
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +83,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ if tag != "" {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressMatches)
+ if len(result) > 0 || tagRouterRuleCopy.Force {
+ return result
+ }
+ } else {
+ // dynamic tag group doesn't have any item about the requested app OR it's null after filtered by
+ // dynamic tag group but force=false. check static tag
+ filter := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == tag {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filter)
+ }
+ // If there's no tagged providers that can match the current tagged request. force.tag is set by default
+ // to false, which means it will invoke any providers without a tag unless it's explicitly disallowed.
+ if len(result) > 0 || isForceUseTag(url, invocation) {
+ return result
+ } else {
+ // FAILOVER: return all Providers without any tags.
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(tagRouterRuleCopy.getAddresses(), url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ filterTagIsEmpty := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == "" {
+ return true
+ }
+ return false
+ }
+ return filterInvoker(invokers, filterAddressNotMatches, filterTagIsEmpty)
+ }
+ } else {
+ // return all addresses in dynamic tag group.
+ addresses = tagRouterRuleCopy.getAddresses()
+ if len(addresses) > 0 {
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressNotMatches)
+ // 1. all addresses are in dynamic tag group, return empty list.
+ if len(result) == 0 {
+ return result
+ }
+ // 2. if there are some addresses that are not in any dynamic tag group, continue to filter using the
+ // static tag group.
+ }
+ filter := func(invoker protocol.Invoker) bool {
+ localTag := invoker.GetUrl().GetParam(constant.Tagkey, "")
+ return localTag == "" || !(tagRouterRuleCopy.hasTag(localTag))
+ }
+ return filterInvoker(result, filter)
+ }
+}
+
+func (c *tagRouter) Process(event *config_center.ConfigChangeEvent) {
+ logger.Infof("Notification of tag rule, change type is:[%s] , raw rule is:[%v]", event.ConfigType, event.Value)
+ if remoting.EventTypeDel == event.ConfigType {
+ c.tagRouterRule = nil
+ return
+ } else {
+ content, ok := event.Value.(string)
+ if !ok {
+ msg := fmt.Sprintf("Convert event content fail,raw content:[%s] ", event.Value)
+ logger.Error(msg)
+ return
+ }
+
+ routerRule, err := getRule(content)
+ if err != nil {
+ logger.Errorf("Parse dynamic tag router rule fail,error:[%s] ", err)
+ return
+ }
+ c.tagRouterRule = routerRule
+ return
+ }
+}
+
+func (c *tagRouter) Notify(invokers []protocol.Invoker) {
+ if len(invokers) == 0 {
+ return
+ }
+ invoker := invokers[0]
+ url := invoker.GetUrl()
+ providerApplication := url.GetParam(constant.RemoteApplicationKey, "")
+ if providerApplication == "" {
+ logger.Error("TagRouter must getConfig from or subscribe to a specific application, but the application " +
+ "in this TagRouter is not specified.")
+ return
+ }
+ dynamicConfiguration := config.GetEnvInstance().GetDynamicConfiguration()
+ if dynamicConfiguration == nil {
+ logger.Error("Get dynamicConfiguration fail, dynamicConfiguration is nil, init config center plugin please")
+ return
+ }
+
+ if providerApplication != c.application {
+ dynamicConfiguration.RemoveListener(c.application+constant.TagRouterRuleSuffix, c)
+ }
+
+ routerKey := providerApplication + constant.TagRouterRuleSuffix
+ dynamicConfiguration.AddListener(routerKey, c)
+ //get rule
+ rule, err := dynamicConfiguration.GetRule(routerKey, config_center.WithGroup(config_center.DEFAULT_GROUP))
+ if len(rule) == 0 || err != nil {
+ logger.Errorf("Get rule fail, config rule{%s}, error{%v}", rule, err)
+ return
+ }
+ if rule != "" {
Review comment:
len>0
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +83,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ if tag != "" {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressMatches)
+ if len(result) > 0 || tagRouterRuleCopy.Force {
+ return result
+ }
+ } else {
+ // dynamic tag group doesn't have any item about the requested app OR it's null after filtered by
+ // dynamic tag group but force=false. check static tag
+ filter := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == tag {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filter)
+ }
+ // If there's no tagged providers that can match the current tagged request. force.tag is set by default
+ // to false, which means it will invoke any providers without a tag unless it's explicitly disallowed.
+ if len(result) > 0 || isForceUseTag(url, invocation) {
+ return result
+ } else {
+ // FAILOVER: return all Providers without any tags.
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(tagRouterRuleCopy.getAddresses(), url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ filterTagIsEmpty := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == "" {
+ return true
+ }
+ return false
+ }
+ return filterInvoker(invokers, filterAddressNotMatches, filterTagIsEmpty)
+ }
+ } else {
+ // return all addresses in dynamic tag group.
+ addresses = tagRouterRuleCopy.getAddresses()
+ if len(addresses) > 0 {
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressNotMatches)
+ // 1. all addresses are in dynamic tag group, return empty list.
+ if len(result) == 0 {
+ return result
+ }
+ // 2. if there are some addresses that are not in any dynamic tag group, continue to filter using the
+ // static tag group.
+ }
+ filter := func(invoker protocol.Invoker) bool {
+ localTag := invoker.GetUrl().GetParam(constant.Tagkey, "")
+ return localTag == "" || !(tagRouterRuleCopy.hasTag(localTag))
+ }
+ return filterInvoker(result, filter)
+ }
+}
+
+func (c *tagRouter) Process(event *config_center.ConfigChangeEvent) {
+ logger.Infof("Notification of tag rule, change type is:[%s] , raw rule is:[%v]", event.ConfigType, event.Value)
+ if remoting.EventTypeDel == event.ConfigType {
+ c.tagRouterRule = nil
+ return
+ } else {
+ content, ok := event.Value.(string)
+ if !ok {
+ msg := fmt.Sprintf("Convert event content fail,raw content:[%s] ", event.Value)
+ logger.Error(msg)
Review comment:
Use logger.Errorf ?
----------------------------------------------------------------
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 #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r462737890
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -52,18 +52,44 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
- // TODO init tags
+ r.init()
return r, nil
}
+func (t *RouterRule) init() {
+ t.addressToTagNames = make(map[string][]string)
+ t.tagNameToAddresses = make(map[string][]string)
Review comment:
t.addressToTagNames = make(map[string][]string, 8)
t.tagNameToAddresses = make(map[string][]string, 8)
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -52,18 +52,44 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
- // TODO init tags
+ r.init()
return r, nil
}
+func (t *RouterRule) init() {
+ t.addressToTagNames = make(map[string][]string)
+ t.tagNameToAddresses = make(map[string][]string)
+ for _, tag := range t.Tags {
+ for _, address := range tag.Addresses {
+ t.addressToTagNames[address] = append(t.addressToTagNames[address], tag.Name)
+ }
+ t.tagNameToAddresses[tag.Name] = tag.Addresses
+ }
+}
+
func (t *RouterRule) getAddresses() []string {
- // TODO get all tag addresses
- return nil
+ var result []string
Review comment:
var result = make([]string, 0, 8 * len(t.Tags))
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -172,18 +185,51 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
return false
}
-func addressMatches(url *common.URL, addresses []string) bool {
- return len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port)
+func filterAddressMatches(invokers []protocol.Invoker, addresses []string) []protocol.Invoker {
+ var idx int
+ for _, invoker := range invokers {
+ url := invoker.GetUrl()
+ if !(len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port)) {
+ continue
+ }
+ invokers[idx] = invoker
+ idx++
+ }
+ return invokers[:idx]
+}
+
+func filterAddressNotMatches(invokers []protocol.Invoker, addresses []string) []protocol.Invoker {
+ var idx int
+ for _, invoker := range invokers {
+ url := invoker.GetUrl()
+ if !(len(addresses) == 0 || !checkAddressMatch(addresses, url.Ip, url.Port)) {
+ continue
+ }
+ invokers[idx] = invoker
+ idx++
+ }
+ return invokers[:idx]
}
-func addressNotMatches(url *common.URL, addresses []string) bool {
- return len(addresses) == 0 || !checkAddressMatch(addresses, url.Ip, url.Port)
+func filterCondition(invokers []protocol.Invoker, condition func(protocol.Invoker) bool) []protocol.Invoker {
+ var idx int
+ for _, invoker := range invokers {
+ if !condition(invoker) {
+ continue
+ }
+ invokers[idx] = invoker
+ idx++
+ }
+ return invokers[:idx]
}
func checkAddressMatch(addresses []string, host, port string) bool {
for _, address := range addresses {
- // TODO address parse
- if address == (host + port) {
+ // TODO ip match
+ if address == host+":"+port {
Review comment:
delete "host+":"+port", using net.JoinHostPort(host, port) instead.
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -52,18 +52,44 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
- // TODO init tags
+ r.init()
return r, nil
}
+func (t *RouterRule) init() {
+ t.addressToTagNames = make(map[string][]string)
+ t.tagNameToAddresses = make(map[string][]string)
+ for _, tag := range t.Tags {
+ for _, address := range tag.Addresses {
+ t.addressToTagNames[address] = append(t.addressToTagNames[address], tag.Name)
+ }
+ t.tagNameToAddresses[tag.Name] = tag.Addresses
+ }
+}
+
func (t *RouterRule) getAddresses() []string {
- // TODO get all tag addresses
- return nil
+ var result []string
+ for _, tag := range t.Tags {
+ result = append(result, tag.Addresses...)
+ }
+ return result
}
func (t *RouterRule) getTagNames() []string {
- // TODO get all tag names
- return nil
+ var result []string
Review comment:
var result = make([]string, 0, len(t.Tags))
----------------------------------------------------------------
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] codecov-commenter commented on pull request #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#issuecomment-667842286
# [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/665?src=pr&el=h1) Report
> Merging [#665](https://codecov.io/gh/apache/dubbo-go/pull/665?src=pr&el=desc) into [develop](https://codecov.io/gh/apache/dubbo-go/commit/272ddd598d036d1b85cf0a6279e7877549a48c32&el=desc) will **decrease** coverage by `0.34%`.
> The diff coverage is `54.91%`.
[](https://codecov.io/gh/apache/dubbo-go/pull/665?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #665 +/- ##
===========================================
- Coverage 63.72% 63.37% -0.35%
===========================================
Files 237 239 +2
Lines 12295 12598 +303
===========================================
+ Hits 7835 7984 +149
- Misses 3706 3829 +123
- Partials 754 785 +31
```
| [Impacted Files](https://codecov.io/gh/apache/dubbo-go/pull/665?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [cluster/router/condition/listenable\_router.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvY29uZGl0aW9uL2xpc3RlbmFibGVfcm91dGVyLmdv) | `54.83% <ø> (ø)` | |
| [cluster/router/tag/file.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL2ZpbGUuZ28=) | `76.92% <ø> (ø)` | |
| [cluster/router/tag/tag.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3RhZy5nbw==) | `0.00% <0.00%> (ø)` | |
| [cluster/router/tag/tag\_router.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3RhZ19yb3V0ZXIuZ28=) | `54.62% <52.19%> (-15.97%)` | :arrow_down: |
| [cluster/router/tag/router\_rule.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-Y2x1c3Rlci9yb3V0ZXIvdGFnL3JvdXRlcl9ydWxlLmdv) | `84.21% <87.09%> (+12.78%)` | :arrow_up: |
| [filter/filter\_impl/metrics\_filter.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-ZmlsdGVyL2ZpbHRlcl9pbXBsL21ldHJpY3NfZmlsdGVyLmdv) | `86.36% <0.00%> (-13.64%)` | :arrow_down: |
| [registry/kubernetes/listener.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-cmVnaXN0cnkva3ViZXJuZXRlcy9saXN0ZW5lci5nbw==) | `65.95% <0.00%> (-6.39%)` | :arrow_down: |
| [filter/filter\_impl/hystrix\_filter.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-ZmlsdGVyL2ZpbHRlcl9pbXBsL2h5c3RyaXhfZmlsdGVyLmdv) | `68.64% <0.00%> (-3.39%)` | :arrow_down: |
| [protocol/dubbo/client.go](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree#diff-cHJvdG9jb2wvZHViYm8vY2xpZW50Lmdv) | `67.87% <0.00%> (-1.22%)` | :arrow_down: |
| ... and [1 more](https://codecov.io/gh/apache/dubbo-go/pull/665/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/dubbo-go/pull/665?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/dubbo-go/pull/665?src=pr&el=footer). Last update [272ddd5...a31a4e2](https://codecov.io/gh/apache/dubbo-go/pull/665?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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 merged pull request #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
AlexStocks merged pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665
----------------------------------------------------------------
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 #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
AlexStocks commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r465645927
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -100,3 +263,163 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
}
return false
}
+
+type filter func(protocol.Invoker) bool
+
+func filterInvoker(invokers []protocol.Invoker, filters ...filter) []protocol.Invoker {
+ var res []protocol.Invoker
+OUTER:
+ for _, invoker := range invokers {
+ for _, filter := range filters {
+ if !filter(invoker) {
+ continue OUTER
+ }
+ }
+ res = append(res, invoker)
+ }
+ return res
+}
+
+// TODO: need move to dubbogo/gost
+func checkAddressMatch(addresses []string, host, port string) bool {
+ for _, address := range addresses {
+ if matchIp(address, host, port) {
+ return true
+ }
+ if address == net.JoinHostPort(constant.ANYHOST_VALUE, port) {
+ return true
+ }
+ }
+ return false
+}
+
+func matchIp(pattern, host, port string) bool {
+ // if the pattern is subnet format, it will not be allowed to config port param in pattern.
+ if strings.Contains(pattern, "/") {
+ _, subnet, _ := net.ParseCIDR(pattern)
+ if subnet != nil && subnet.Contains(net.ParseIP(host)) {
+ return true
+ }
+ return false
+ }
+ return matchIpRange(pattern, host, port)
+}
+
+func matchIpRange(pattern, host, port string) bool {
+ if pattern == "" || host == "" {
+ logger.Error("Illegal Argument pattern or hostName. Pattern:" + pattern + ", Host:" + host)
+ return false
+ }
+
+ pattern = strings.TrimSpace(pattern)
+ if "*.*.*.*" == pattern || "*" == pattern {
+ return true
+ }
+
+ isIpv4 := true
+ ip4 := net.ParseIP(host).To4()
+
+ if ip4 == nil {
+ isIpv4 = false
+ }
+
+ hostAndPort := getPatternHostAndPort(pattern, isIpv4)
+ if hostAndPort[1] != "" && hostAndPort[1] != port {
+ return false
+ }
+
+ pattern = hostAndPort[0]
+ // TODO 常量化
Review comment:
we should not use Chinese comment in our codes.
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -100,3 +263,163 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
}
return false
}
+
+type filter func(protocol.Invoker) bool
+
+func filterInvoker(invokers []protocol.Invoker, filters ...filter) []protocol.Invoker {
+ var res []protocol.Invoker
+OUTER:
+ for _, invoker := range invokers {
+ for _, filter := range filters {
+ if !filter(invoker) {
+ continue OUTER
+ }
+ }
+ res = append(res, invoker)
+ }
+ return res
+}
+
+// TODO: need move to dubbogo/gost
+func checkAddressMatch(addresses []string, host, port string) bool {
+ for _, address := range addresses {
+ if matchIp(address, host, port) {
+ return true
+ }
+ if address == net.JoinHostPort(constant.ANYHOST_VALUE, port) {
+ return true
+ }
+ }
+ return false
+}
+
+func matchIp(pattern, host, port string) bool {
+ // if the pattern is subnet format, it will not be allowed to config port param in pattern.
+ if strings.Contains(pattern, "/") {
+ _, subnet, _ := net.ParseCIDR(pattern)
+ if subnet != nil && subnet.Contains(net.ParseIP(host)) {
+ return true
+ }
+ return false
+ }
+ return matchIpRange(pattern, host, port)
+}
+
+func matchIpRange(pattern, host, port string) bool {
+ if pattern == "" || host == "" {
+ logger.Error("Illegal Argument pattern or hostName. Pattern:" + pattern + ", Host:" + host)
+ return false
+ }
+
+ pattern = strings.TrimSpace(pattern)
+ if "*.*.*.*" == pattern || "*" == pattern {
+ return true
+ }
+
+ isIpv4 := true
+ ip4 := net.ParseIP(host).To4()
+
+ if ip4 == nil {
+ isIpv4 = false
+ }
+
+ hostAndPort := getPatternHostAndPort(pattern, isIpv4)
+ if hostAndPort[1] != "" && hostAndPort[1] != port {
+ return false
+ }
+
+ pattern = hostAndPort[0]
+ // TODO 常量化
+ splitCharacter := "."
+ if !isIpv4 {
+ splitCharacter = ":"
+ }
+
+ mask := strings.Split(pattern, splitCharacter)
+ // check format of pattern
+ if err := checkHostPattern(pattern, mask, isIpv4); err != nil {
+ logger.Error(err)
+ return false
+ }
+
+ if pattern == host {
+ return true
+ }
+
+ // short name condition
+ if !ipPatternContains(pattern) {
+ return pattern == host
+ }
+
+ ipAddress := strings.Split(host, splitCharacter)
+ for i := 0; i < len(mask); i++ {
+ if "*" == mask[i] || mask[i] == ipAddress[i] {
+ continue
+ } else if strings.Contains(mask[i], "-") {
+ rangeNumStrs := strings.Split(mask[i], "-")
+ if len(rangeNumStrs) != 2 {
+ logger.Error("There is wrong format of ip Address: " + mask[i])
+ return false
+ }
+ min := getNumOfIpSegment(rangeNumStrs[0], isIpv4)
+ max := getNumOfIpSegment(rangeNumStrs[1], isIpv4)
+ ip := getNumOfIpSegment(ipAddress[i], isIpv4)
+ if ip < min || ip > max {
+ return false
+ }
+ } else if "0" == ipAddress[i] && "0" == mask[i] || "00" == mask[i] || "000" == mask[i] || "0000" == mask[i] {
+ continue
+ } else if mask[i] != ipAddress[i] {
+ return false
+ }
+ }
+ return true
+}
+
+func ipPatternContains(pattern string) bool {
+ return strings.Contains(pattern, "*") || strings.Contains(pattern, "-")
+}
+
+func checkHostPattern(pattern string, mask []string, isIpv4 bool) error {
+ if !isIpv4 {
+ if len(mask) != 8 && ipPatternContains(pattern) {
+ return errors.New("If you config ip expression that contains '*' or '-', please fill qualified ip pattern like 234e:0:4567:0:0:0:3d:*. ")
+ }
+ if len(mask) != 8 && !strings.Contains(pattern, "::") {
+ return errors.New("The host is ipv6, but the pattern is not ipv6 pattern : " + pattern)
+ }
+ } else {
+ if len(mask) != 4 {
+ return errors.New("The host is ipv4, but the pattern is not ipv4 pattern : " + pattern)
+ }
+ }
+ return nil
+}
+
+func getPatternHostAndPort(pattern string, isIpv4 bool) []string {
+ result := make([]string, 2)
+ if strings.HasPrefix(pattern, "[") && strings.Contains(pattern, "]:") {
Review comment:
add some comments for every if/else branch, pls.
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
Review comment:
agree
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -100,3 +263,163 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
}
return false
}
+
+type filter func(protocol.Invoker) bool
+
+func filterInvoker(invokers []protocol.Invoker, filters ...filter) []protocol.Invoker {
+ var res []protocol.Invoker
+OUTER:
+ for _, invoker := range invokers {
+ for _, filter := range filters {
+ if !filter(invoker) {
+ continue OUTER
Review comment:
break OUTER？
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -100,3 +263,163 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
}
return false
}
+
+type filter func(protocol.Invoker) bool
+
+func filterInvoker(invokers []protocol.Invoker, filters ...filter) []protocol.Invoker {
+ var res []protocol.Invoker
+OUTER:
+ for _, invoker := range invokers {
+ for _, filter := range filters {
+ if !filter(invoker) {
+ continue OUTER
+ }
+ }
+ res = append(res, invoker)
+ }
+ return res
+}
+
+// TODO: need move to dubbogo/gost
+func checkAddressMatch(addresses []string, host, port string) bool {
+ for _, address := range addresses {
+ if matchIp(address, host, port) {
+ return true
+ }
+ if address == net.JoinHostPort(constant.ANYHOST_VALUE, port) {
+ return true
+ }
+ }
+ return false
+}
+
+func matchIp(pattern, host, port string) bool {
+ // if the pattern is subnet format, it will not be allowed to config port param in pattern.
+ if strings.Contains(pattern, "/") {
+ _, subnet, _ := net.ParseCIDR(pattern)
+ if subnet != nil && subnet.Contains(net.ParseIP(host)) {
+ return true
+ }
+ return false
+ }
+ return matchIpRange(pattern, host, port)
+}
+
+func matchIpRange(pattern, host, port string) bool {
+ if pattern == "" || host == "" {
+ logger.Error("Illegal Argument pattern or hostName. Pattern:" + pattern + ", Host:" + host)
+ return false
+ }
+
+ pattern = strings.TrimSpace(pattern)
+ if "*.*.*.*" == pattern || "*" == pattern {
+ return true
+ }
+
+ isIpv4 := true
+ ip4 := net.ParseIP(host).To4()
+
+ if ip4 == nil {
+ isIpv4 = false
+ }
+
+ hostAndPort := getPatternHostAndPort(pattern, isIpv4)
+ if hostAndPort[1] != "" && hostAndPort[1] != port {
+ return false
+ }
+
+ pattern = hostAndPort[0]
+ // TODO 常量化
+ splitCharacter := "."
+ if !isIpv4 {
+ splitCharacter = ":"
+ }
+
+ mask := strings.Split(pattern, splitCharacter)
+ // check format of pattern
+ if err := checkHostPattern(pattern, mask, isIpv4); err != nil {
+ logger.Error(err)
+ return false
+ }
+
+ if pattern == host {
+ return true
+ }
+
+ // short name condition
+ if !ipPatternContains(pattern) {
+ return pattern == host
+ }
+
+ ipAddress := strings.Split(host, splitCharacter)
+ for i := 0; i < len(mask); i++ {
+ if "*" == mask[i] || mask[i] == ipAddress[i] {
Review comment:
u should add comment for every if/else branch to make it easy to understand.
----------------------------------------------------------------
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] hxmhlt commented on a change in pull request #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
hxmhlt commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r465770000
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -34,5 +52,58 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
+ r.init()
return r, nil
}
+
+func (t *RouterRule) init() {
+ t.addressToTagNames = make(map[string][]string, 8)
+ t.tagNameToAddresses = make(map[string][]string, 8)
+ for _, tag := range t.Tags {
+ for _, address := range tag.Addresses {
+ t.addressToTagNames[address] = append(t.addressToTagNames[address], tag.Name)
+ }
+ t.tagNameToAddresses[tag.Name] = tag.Addresses
+ }
+}
+
+func (t *RouterRule) getAddresses() []string {
+ var result = make([]string, 0, 8*len(t.Tags))
Review comment:
Pls add func comment for RouterRule
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -34,5 +52,58 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
+ r.init()
return r, nil
}
+
+func (t *RouterRule) init() {
Review comment:
Why create a new func named init? If you create the func ,pls add the comment for 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:
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] watermelo commented on a change in pull request #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
watermelo commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r464087744
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -22,9 +22,27 @@ import (
"github.com/apache/dubbo-go/common/yaml"
)
+/**
+ * %YAML1.2
+ * ---
+ * force: true
+ * runtime: false
+ * enabled: true
+ * priority: 1
+ * key: demo-provider
+ * tags:
+ * - name: tag1
+ * addresses: [ip1, ip2]
+ * - name: tag2
+ * addresses: [ip3, ip4]
+ * ...
+ */
// RouterRule RouterRule config read from config file or config center
type RouterRule struct {
router.BaseRouterRule `yaml:",inline""`
+ Tags []Tag
Review comment:
tags is unexport, so can't be yaml parsed
----------------------------------------------------------------
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 #665: Ftr: dynamic tag router
Posted by GitBox <gi...@apache.org>.
zouyx commented on a change in pull request #665:
URL: https://github.com/apache/dubbo-go/pull/665#discussion_r465813490
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ // if we are requesting for a Provider with a specific tag
+ if len(tag) > 0 {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressMatches)
+ if len(result) > 0 || tagRouterRuleCopy.Force {
+ return result
+ }
+ } else {
+ // dynamic tag group doesn't have any item about the requested app OR it's null after filtered by
+ // dynamic tag group but force=false. check static tag
+ filter := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == tag {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filter)
+ }
+ // If there's no tagged providers that can match the current tagged request. force.tag is set by default
+ // to false, which means it will invoke any providers without a tag unless it's explicitly disallowed.
+ if len(result) > 0 || isForceUseTag(url, invocation) {
+ return result
+ } else {
+ // FAILOVER: return all Providers without any tags.
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(tagRouterRuleCopy.getAddresses(), url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ filterTagIsEmpty := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == "" {
+ return true
+ }
+ return false
Review comment:
```suggestion
return invoker.GetUrl().GetParam(constant.Tagkey, "") == ""
```
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -34,5 +52,58 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
+ r.init()
return r, nil
}
+
+func (t *RouterRule) init() {
+ t.addressToTagNames = make(map[string][]string, 8)
+ t.tagNameToAddresses = make(map[string][]string, 8)
Review comment:
```suggestion
l:=len(t.Tags)
t.addressToTagNames = make(map[string][]string, l*2)
t.tagNameToAddresses = make(map[string][]string, l)
```
##########
File path: cluster/router/tag/tag_router_test.go
##########
@@ -19,32 +19,59 @@ package tag
import (
"context"
+ "fmt"
+ "github.com/stretchr/testify/suite"
"testing"
+ "time"
)
import (
+ "github.com/dubbogo/go-zookeeper/zk"
Review comment:
split package
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ // if we are requesting for a Provider with a specific tag
+ if len(tag) > 0 {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressMatches)
+ if len(result) > 0 || tagRouterRuleCopy.Force {
+ return result
+ }
+ } else {
+ // dynamic tag group doesn't have any item about the requested app OR it's null after filtered by
+ // dynamic tag group but force=false. check static tag
+ filter := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == tag {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filter)
+ }
+ // If there's no tagged providers that can match the current tagged request. force.tag is set by default
+ // to false, which means it will invoke any providers without a tag unless it's explicitly disallowed.
+ if len(result) > 0 || isForceUseTag(url, invocation) {
+ return result
+ } else {
+ // FAILOVER: return all Providers without any tags.
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(tagRouterRuleCopy.getAddresses(), url.Ip, url.Port) {
+ return true
+ }
+ return false
Review comment:
```suggestion
return len(addresses) == 0 || !checkAddressMatch(tagRouterRuleCopy.getAddresses(), url.Ip, url.Port)
```
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -100,3 +263,163 @@ func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
}
return false
}
+
+type filter func(protocol.Invoker) bool
+
+func filterInvoker(invokers []protocol.Invoker, filters ...filter) []protocol.Invoker {
+ var res []protocol.Invoker
+OUTER:
+ for _, invoker := range invokers {
+ for _, filter := range filters {
+ if !filter(invoker) {
+ continue OUTER
+ }
+ }
+ res = append(res, invoker)
+ }
+ return res
+}
+
+// TODO: need move to dubbogo/gost
+func checkAddressMatch(addresses []string, host, port string) bool {
+ for _, address := range addresses {
+ if matchIp(address, host, port) {
+ return true
+ }
+ if address == net.JoinHostPort(constant.ANYHOST_VALUE, port) {
+ return true
+ }
+ }
+ return false
+}
+
+func matchIp(pattern, host, port string) bool {
+ // if the pattern is subnet format, it will not be allowed to config port param in pattern.
+ if strings.Contains(pattern, "/") {
+ _, subnet, _ := net.ParseCIDR(pattern)
+ if subnet != nil && subnet.Contains(net.ParseIP(host)) {
+ return true
+ }
+ return false
Review comment:
```suggestion
return subnet != nil && subnet.Contains(net.ParseIP(host))
```
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -34,5 +52,58 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
+ r.init()
return r, nil
}
+
+func (t *RouterRule) init() {
+ t.addressToTagNames = make(map[string][]string, 8)
+ t.tagNameToAddresses = make(map[string][]string, 8)
+ for _, tag := range t.Tags {
+ for _, address := range tag.Addresses {
+ t.addressToTagNames[address] = append(t.addressToTagNames[address], tag.Name)
+ }
+ t.tagNameToAddresses[tag.Name] = tag.Addresses
+ }
+}
+
+func (t *RouterRule) getAddresses() []string {
+ var result = make([]string, 0, 8*len(t.Tags))
Review comment:
```suggestion
var result = make([]string, 0, 2*len(t.Tags))
```
i think * 2 enough
##########
File path: cluster/router/tag/router_rule.go
##########
@@ -34,5 +52,58 @@ func getRule(rawRule string) (*RouterRule, error) {
return r, err
}
r.RawRule = rawRule
+ r.init()
return r, nil
}
+
+func (t *RouterRule) init() {
+ t.addressToTagNames = make(map[string][]string, 8)
+ t.tagNameToAddresses = make(map[string][]string, 8)
+ for _, tag := range t.Tags {
+ for _, address := range tag.Addresses {
+ t.addressToTagNames[address] = append(t.addressToTagNames[address], tag.Name)
+ }
+ t.tagNameToAddresses[tag.Name] = tag.Addresses
+ }
+}
+
+func (t *RouterRule) getAddresses() []string {
+ var result = make([]string, 0, 8*len(t.Tags))
+ for _, tag := range t.Tags {
+ result = append(result, tag.Addresses...)
+ }
+ return result
+}
+
+func (t *RouterRule) getTagNames() []string {
+ var result = make([]string, 0, len(t.Tags))
+ for _, tag := range t.Tags {
+ result = append(result, tag.Name)
+ }
+ return result
+}
+
+func (t *RouterRule) hasTag(tag string) bool {
+ for _, t := range t.Tags {
+ if tag == t.Name {
+ return true
+ }
+ }
+ return false
Review comment:
```suggestion
return len(tagNameToAddresses[tag])>0
```
Why dont you use `tagNameToAddresses map[string][]string`?
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ // if we are requesting for a Provider with a specific tag
+ if len(tag) > 0 {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressMatches)
+ if len(result) > 0 || tagRouterRuleCopy.Force {
+ return result
+ }
+ } else {
+ // dynamic tag group doesn't have any item about the requested app OR it's null after filtered by
+ // dynamic tag group but force=false. check static tag
+ filter := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == tag {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filter)
+ }
+ // If there's no tagged providers that can match the current tagged request. force.tag is set by default
+ // to false, which means it will invoke any providers without a tag unless it's explicitly disallowed.
+ if len(result) > 0 || isForceUseTag(url, invocation) {
+ return result
+ } else {
+ // FAILOVER: return all Providers without any tags.
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(tagRouterRuleCopy.getAddresses(), url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ filterTagIsEmpty := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == "" {
+ return true
+ }
+ return false
+ }
+ return filterInvoker(invokers, filterAddressNotMatches, filterTagIsEmpty)
+ }
+ } else {
+ // return all addresses in dynamic tag group.
+ addresses = tagRouterRuleCopy.getAddresses()
+ if len(addresses) > 0 {
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressNotMatches)
+ // 1. all addresses are in dynamic tag group, return empty list.
+ if len(result) == 0 {
+ return result
+ }
+ }
+ // 2. if there are some addresses that are not in any dynamic tag group, continue to filter using the
+ // static tag group.
+ filter := func(invoker protocol.Invoker) bool {
+ localTag := invoker.GetUrl().GetParam(constant.Tagkey, "")
+ return localTag == "" || !(tagRouterRuleCopy.hasTag(localTag))
+ }
+ return filterInvoker(result, filter)
+ }
+}
+
+func (c *tagRouter) Process(event *config_center.ConfigChangeEvent) {
+ logger.Infof("Notification of tag rule, change type is:[%s] , raw rule is:[%v]", event.ConfigType, event.Value)
+ if remoting.EventTypeDel == event.ConfigType {
+ c.tagRouterRule = nil
+ return
+ } else {
Review comment:
delete else
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -55,6 +64,15 @@ func (c *tagRouter) isEnabled() bool {
return c.enabled
}
+func (c *tagRouter) SetApplication(app string) {
+ c.application = app
+}
+
+func (c *tagRouter) tagRouterRuleCopy() RouterRule {
+ routerRule := *c.tagRouterRule
+ return routerRule
Review comment:
```suggestion
return *c.tagRouterRule
```
enough? and i think no need to extract a new function for single scene
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ // if we are requesting for a Provider with a specific tag
+ if len(tag) > 0 {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressMatches)
+ if len(result) > 0 || tagRouterRuleCopy.Force {
+ return result
+ }
+ } else {
+ // dynamic tag group doesn't have any item about the requested app OR it's null after filtered by
+ // dynamic tag group but force=false. check static tag
+ filter := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == tag {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filter)
+ }
+ // If there's no tagged providers that can match the current tagged request. force.tag is set by default
+ // to false, which means it will invoke any providers without a tag unless it's explicitly disallowed.
+ if len(result) > 0 || isForceUseTag(url, invocation) {
+ return result
+ } else {
+ // FAILOVER: return all Providers without any tags.
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(tagRouterRuleCopy.getAddresses(), url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ filterTagIsEmpty := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == "" {
+ return true
+ }
+ return false
+ }
+ return filterInvoker(invokers, filterAddressNotMatches, filterTagIsEmpty)
+ }
+ } else {
+ // return all addresses in dynamic tag group.
+ addresses = tagRouterRuleCopy.getAddresses()
+ if len(addresses) > 0 {
+ filterAddressNotMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) == 0 || !checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
Review comment:
```suggestion
return len(addresses) == 0 || !checkAddressMatch(addresses, url.Ip, url.Port)
```
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ // if we are requesting for a Provider with a specific tag
+ if len(tag) > 0 {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
+ }
+ result = filterInvoker(invokers, filterAddressMatches)
+ if len(result) > 0 || tagRouterRuleCopy.Force {
+ return result
+ }
+ } else {
+ // dynamic tag group doesn't have any item about the requested app OR it's null after filtered by
+ // dynamic tag group but force=false. check static tag
+ filter := func(invoker protocol.Invoker) bool {
+ if invoker.GetUrl().GetParam(constant.Tagkey, "") == tag {
+ return true
+ }
+ return false
Review comment:
```suggestion
return invoker.GetUrl().GetParam(constant.Tagkey, "") == tag
```
##########
File path: cluster/router/tag/tag_router.go
##########
@@ -63,7 +81,152 @@ func (c *tagRouter) Route(invokers []protocol.Invoker, url *common.URL, invocati
if len(invokers) == 0 {
return invokers
}
- return filterUsingStaticTag(invokers, url, invocation)
+ if c.tagRouterRule == nil || !c.tagRouterRule.Valid || !c.tagRouterRule.Enabled {
+ return filterUsingStaticTag(invokers, url, invocation)
+ }
+ // since the rule can be changed by config center, we should copy one to use.
+ tagRouterRuleCopy := c.tagRouterRuleCopy()
+ tag, ok := invocation.Attachments()[constant.Tagkey]
+ if !ok {
+ tag = url.GetParam(constant.Tagkey, "")
+ }
+ var (
+ result []protocol.Invoker
+ addresses []string
+ )
+ // if we are requesting for a Provider with a specific tag
+ if len(tag) > 0 {
+ addresses, _ = tagRouterRuleCopy.getTagNameToAddresses()[tag]
+ // filter by dynamic tag group first
+ if len(addresses) > 0 {
+ filterAddressMatches := func(invoker protocol.Invoker) bool {
+ url := invoker.GetUrl()
+ if len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port) {
+ return true
+ }
+ return false
Review comment:
```suggestion
return len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port)
```
----------------------------------------------------------------
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