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/08/10 15:21:29 UTC

[GitHub] [dubbo-go] gaoxinge commented on a change in pull request #703: Ftr: add dynamic tag router

gaoxinge commented on a change in pull request #703:
URL: https://github.com/apache/dubbo-go/pull/703#discussion_r467970835



##########
File path: before_ut.sh
##########
@@ -36,5 +36,8 @@ cp ${zkJar} cluster/router/chain/zookeeper-4unittest/contrib/fatjar
 mkdir -p cluster/router/condition/zookeeper-4unittest/contrib/fatjar
 cp ${zkJar} cluster/router/condition/zookeeper-4unittest/contrib/fatjar
 
+mkdir -p cluster/router/tag/zookeeper-4unittest/contrib/fatjar
+cp ${zkJar} cluster/router/tag/zookeeper-4unittest/contrib/fatjar

Review comment:
       Also apply this change to file https://github.com/apache/dubbo-go/blob/master/before_ut.bat.

##########
File path: cluster/router/tag/tag_router.go
##########
@@ -93,10 +205,211 @@ func filterUsingStaticTag(invokers []protocol.Invoker, url *common.URL, invocati
 	return invokers
 }
 
+// filterInvokersWithTag gets a list of invoker using dynamic route with tag
+func filterInvokersWithTag(invokers []protocol.Invoker, url *common.URL, invocation protocol.Invocation, tagRouterRule RouterRule, tag string) []protocol.Invoker {
+	var (
+		result    []protocol.Invoker
+		addresses []string
+	)
+	addresses, _ = tagRouterRule.getTagNameToAddresses()[tag]
+	// filter by dynamic tag group first
+	if len(addresses) > 0 {
+		filterAddressMatches := func(invoker protocol.Invoker) bool {
+			url := invoker.GetUrl()
+			return len(addresses) > 0 && checkAddressMatch(addresses, url.Ip, url.Port)
+		}
+		result = filterInvoker(invokers, filterAddressMatches)
+		if len(result) > 0 || tagRouterRule.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 {
+			return invoker.GetUrl().GetParam(constant.Tagkey, "") == tag
+		}
+		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()
+			return len(addresses) == 0 || !checkAddressMatch(tagRouterRule.getAddresses(), url.Ip, url.Port)
+		}
+		filterTagIsEmpty := func(invoker protocol.Invoker) bool {
+			return invoker.GetUrl().GetParam(constant.Tagkey, "") == ""
+		}
+		return filterInvoker(invokers, filterAddressNotMatches, filterTagIsEmpty)
+	}
+}
+
 // isForceUseTag returns whether force use tag
 func isForceUseTag(url *common.URL, invocation protocol.Invocation) bool {
 	if b, e := strconv.ParseBool(invocation.AttachmentsByKey(constant.ForceUseTag, url.GetParam(constant.ForceUseTag, "false"))); e == nil {
 		return b
 	}
 	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
+}
+
+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) {

Review comment:
       `net.JoinHostPort(constant.ANYHOST_VALUE, port)` is independent of the cycle, so it may be better to assign it to a value before cycle starts.

##########
File path: cluster/router/tag/tag_router_test.go
##########
@@ -160,3 +187,187 @@ func TestTagRouterRouteNoForce(t *testing.T) {
 	invRst2 := tagRouter.Route(invokers, &u1, inv)
 	assert.Equal(t, 3, len(invRst2))
 }
+
+func TestFilterInvoker(t *testing.T) {
+	u2, e2 := common.NewURL(tagRouterTestHangZhouUrl)
+	u3, e3 := common.NewURL(tagRouterTestShangHaiUrl)
+	u4, e4 := common.NewURL(tagRouterTestBeijingUrl)
+	u5, e5 := common.NewURL(tagRouterTestEnabledBeijingUrl)
+	assert.Nil(t, e2)
+	assert.Nil(t, e3)
+	assert.Nil(t, e4)
+	assert.Nil(t, e5)
+	inv2 := NewMockInvoker(u2)
+	inv3 := NewMockInvoker(u3)
+	inv4 := NewMockInvoker(u4)
+	inv5 := NewMockInvoker(u5)
+	var invokers []protocol.Invoker
+	invokers = append(invokers, inv2, inv3, inv4, inv5)
+	filterTag := func(invoker protocol.Invoker) bool {
+		if invoker.GetUrl().GetParam(constant.Tagkey, "") == "beijing" {

Review comment:
       Directly return `invoker.GetUrl().GetParam(constant.Tagkey, "") == "beijing"`.

##########
File path: cluster/router/tag/tag_router_test.go
##########
@@ -160,3 +187,187 @@ func TestTagRouterRouteNoForce(t *testing.T) {
 	invRst2 := tagRouter.Route(invokers, &u1, inv)
 	assert.Equal(t, 3, len(invRst2))
 }
+
+func TestFilterInvoker(t *testing.T) {
+	u2, e2 := common.NewURL(tagRouterTestHangZhouUrl)
+	u3, e3 := common.NewURL(tagRouterTestShangHaiUrl)
+	u4, e4 := common.NewURL(tagRouterTestBeijingUrl)
+	u5, e5 := common.NewURL(tagRouterTestEnabledBeijingUrl)
+	assert.Nil(t, e2)
+	assert.Nil(t, e3)
+	assert.Nil(t, e4)
+	assert.Nil(t, e5)
+	inv2 := NewMockInvoker(u2)
+	inv3 := NewMockInvoker(u3)
+	inv4 := NewMockInvoker(u4)
+	inv5 := NewMockInvoker(u5)
+	var invokers []protocol.Invoker
+	invokers = append(invokers, inv2, inv3, inv4, inv5)
+	filterTag := func(invoker protocol.Invoker) bool {
+		if invoker.GetUrl().GetParam(constant.Tagkey, "") == "beijing" {
+			return true
+		}
+		return false
+	}
+	res := filterInvoker(invokers, filterTag)
+	assert.Equal(t, []protocol.Invoker{inv4, inv5}, res)
+	flag := true
+	filterEnabled := func(invoker protocol.Invoker) bool {
+		if invoker.GetUrl().GetParamBool(constant.RouterEnabled, false) == flag {

Review comment:
       As above.




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