[Dubbo- support tag router feature] Add a new Router implement -- TagRouter #2228

lexburner · 2018-08-11T08:56:26Z

What is the purpose of the change

Support tag router feature for dubbo.

some pain points

fact1: some applications changed at the same time in different branch,like A,B,C.
fact2: some applications didn't change ,but deploy repeatedly，like D,E.
fact3: if a new feature need to be tested, all 5 application need to be deployed.
fact4: isolation by feature is not friendly to parallel development.

use tag

fact: changed applications are incremental deployment

some cases

usage

consumer

RpcContext.getContext().setAttachment("request.tag","red");

i suggest you can use filter or SPI to set this attachment, notice that RpcContext is bound to Thread.

provider

    @Bean
    public ApplicationConfig applicationConfig() {
        ApplicationConfig applicationConfig = new ApplicationConfig();
        applicationConfig.setName("provider-book");
        // instruct tag router
        Map<String,String> parameters = new HashMap<>();
        parameters.put(Constants.ROUTER_KEY, "tag");
        applicationConfig.setParameters(parameters);
        return applicationConfig;
    }

Brief changelog

add unit property for dubbo.xsd.
add TagRouter,TagRouterFactory.
add SPI statement for adding TagRouter automatically.
fix a small bug(may be not) in org.apache.dubbo.registry.integration.RegistryDirectory#route，exclude runtime routers, avoid to filter some runtime invokers.

Verifying this change

Here is more detail about Unit Test:
org.apache.dubbo.rpc.cluster.router.tag.TagRouterTest
I have verified that the unit test has been correct and passed.

Here is more detail about a Integration Test:
clone the repository https://github.com/lexburner/Dubbo-Sample-TagRouter.git,can see the route tag feature.

Follow this checklist to help us incorporate your contribution quickly and easily:

Make sure there is a GITHUB_issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
Format the pull request title like [Dubbo-XXX] Fix UnknownException when host config not exist #XXX. Each commit in the pull request should have a meaningful subject line and body.
Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
Run mvn clean install -DskipTests & mvn clean test-compile failsafe:integration-test to make sure unit-test and integration-test pass.
If this contribution is large, please follow the Software Donation Guide.

codecov-io · 2018-08-13T03:27:11Z

Codecov Report

❗ No coverage uploaded for pull request base (master@b2ce7b2). Click here to learn what that means.
The diff coverage is 61.36%.

@@            Coverage Diff            @@
##             master    #2228   +/-   ##
=========================================
  Coverage          ?   54.65%           
  Complexity        ?     5172           
=========================================
  Files             ?      571           
  Lines             ?    25084           
  Branches          ?     4464           
=========================================
  Hits              ?    13709           
  Misses            ?     9334           
  Partials          ?     2041

Impacted Files	Coverage Δ	Complexity Δ
...c/main/java/org/apache/dubbo/common/Constants.java	`88.88% <ø> (ø)`	`1 <0> (?)`
...g/apache/dubbo/config/AbstractInterfaceConfig.java	`80.78% <ø> (ø)`	`81 <0> (?)`
...g/apache/dubbo/config/AbstractReferenceConfig.java	`93.75% <ø> (ø)`	`27 <0> (?)`
.../dubbo/registry/integration/RegistryDirectory.java	`0% <0%> (ø)`	`0 <0> (?)`
...dubbo/rpc/cluster/router/tag/TagRouterFactory.java	`100% <100%> (ø)`	`2 <2> (?)`
...org/apache/dubbo/config/AbstractServiceConfig.java	`95.58% <33.33%> (ø)`	`42 <1> (?)`
...apache/dubbo/rpc/cluster/router/tag/TagRouter.java	`66.66% <66.66%> (ø)`	`11 <11> (?)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2ce7b2...bf3915c. Read the comment docs.

diecui1202 · 2018-08-17T02:07:29Z

@lexburner Thanks for your contributing. I think this PR is a big feature, so I suggest discussing it on the dev mail list. BTW, pls. solve the conflicts.

beiwei30 · 2018-08-17T16:31:02Z

very nice feature, will look into it.

kimmking · 2018-08-20T13:43:25Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouter.java

+import java.util.List;
+
+/**
+ * @author [email protected]


remove author plz

lack of ASF license in the head:

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

got it, i will add licenses for all new files.

kimmking · 2018-08-20T13:46:25Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouter.java

+            // Tag request
+            if (!StringUtils.isEmpty(tag)) {
+                // Select tag invokers first
+                for (Invoker<T> invoker : invokers) {


three for statement can be combined one and check conditions in it.

These three for statements have their own logic, depend on different conditions, can you give me more details, thanks a lot.

@lexburner @kimmking i think three for statement can be combined too,my impl:

public <T> List<Invoker<T>> route(List<Invoker<T>> invokers, URL url, Invocation invocation) throws RpcException { // filter List<Invoker<T>> result = new ArrayList<>(); // all invokers that don't have "tag" parameter in url is a normal invoker List<Invoker<T>> normalResult = new ArrayList<>(); try { // Dynamic param String tag = RpcContext.getContext().getAttachment(REQUEST_TAG_KEY); for (Invoker<T> invoker : invokers) { if (StringUtils.isEmpty(invoker.getUrl().getParameter(TAG_KEY))) { // all invokers that don't have "tag" parameter in url is a normal invoker normalResult.add(invoker); } else { if (invoker.getUrl().getParameter(TAG_KEY).equals(tag)) { result.add(invoker); } } } // If no invoker be selected, downgrade to normal invokers if (result.isEmpty()) { return normalResult; } return result; } catch (Exception e) { logger.error("Route by tag error,return all invokers.", e); } // Downgrade to all invokers return invokers; }

beiwei30 · 2018-08-21T03:13:22Z

dubbo-config/dubbo-config-spring/src/main/resources/META-INF/dubbo.xsd

@@ -307,6 +307,12 @@
                        <xsd:documentation><![CDATA[ The serialization protocol of service. ]]></xsd:documentation>
                    </xsd:annotation>
                </xsd:attribute>
+                <xsd:attribute name="tag" type="xsd:string">


pls. also change dubbo-config-spring/src/main/resources/META-INF/compat/dubbo.xsd

beiwei30 · 2018-08-21T03:27:59Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouter.java

+    private final URL url;
+
+    public static final URL ROUTER_URL =
+            new URL("tag"


pls. change this to one liner, or complete remove it since I notice TagRouter() is redundant.

beiwei30 · 2018-08-21T03:30:01Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouter.java

+                }
+                // Normal request
+            } else {
+                for (Invoker<T> invoker : invokers) {


for normal request, I guess we should include all invokers, no matter the service is published with or without tag, for example: what if all services are published with tag, then is it expected that there's no valid service to invoke?

This is intentional, if a service published with tag, we assume it as a new feature, and won't normal request or other feature's request hit it. Only the tag request hit the tag service.

In another case, tag request can hit normal service if (and only if) there is no tag service, i call it downgrade.

I know that no valid service to invoke was sound terrible , but if users want to use TagRouter , i think it is their duty to know what will happen to their projects.

By default, if we do not config like:

Map<String,String> parameters = new HashMap<>(); parameters.put(Constants.ROUTER_KEY, "tag"); applicationConfig.setParameters(parameters);

There will no TagRouter feature exist in our project. Users who don't care about the TagRouter feature are not affected by it.

beiwei30 · 2018-08-21T03:30:34Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouterFactory.java

+import org.apache.dubbo.rpc.cluster.RouterFactory;
+
+/**
+ * @author [email protected]


again, remove author and add ASF license header.

beiwei30 · 2018-08-21T03:40:54Z

dubbo-cluster/src/test/java/org/apache/dubbo/rpc/cluster/router/tag/TagRouterTest.java

+import java.util.List;
+
+/**
+ * @author [email protected]


again, remove author info and add ASF license header.

beiwei30 · 2018-08-22T08:17:37Z

I think it's good to go, I will merge it.

@lexburner pls. add corresponding doc at https://github.com/apache/incubator-dubbo-website, and pls. consider to write a tech blog too. Thx.

hupuxiaojun · 2018-09-03T10:05:25Z

TagRouter feature is very good, but i think an LoadBalance Wrapper can get same result ,and has smaller changes with source code:

import com.alibaba.dubbo.common.URL;
import com.alibaba.dubbo.common.logger.Logger;
import com.alibaba.dubbo.common.logger.LoggerFactory;
import com.alibaba.dubbo.common.utils.StringUtils;
import com.alibaba.dubbo.rpc.Invocation;
import com.alibaba.dubbo.rpc.Invoker;
import com.alibaba.dubbo.rpc.RpcContext;
import com.alibaba.dubbo.rpc.RpcException;
import com.alibaba.dubbo.rpc.cluster.LoadBalance;

import java.util.ArrayList;
import java.util.List;


public class LoadbalanceTagFilterWrapper implements LoadBalance {
    private static final Logger logger = LoggerFactory.getLogger(LoadbalanceTagFilterWrapper.class);
    private LoadBalance loadBalance;

    public LoadbalanceTagFilterWrapper(LoadBalance loadBalance) {
        this.loadBalance = loadBalance;
    }

    @Override
    public <T> Invoker<T> select(List<Invoker<T>> invokers, URL url, Invocation invocation) throws RpcException {
        List<Invoker<T>> filteredInvokers = filterInvokers(invokers);
        return loadBalance.select(filteredInvokers, url, invocation);
    }

    private <T> List<Invoker<T>> filterInvokers(List<Invoker<T>> invokers) {
        // filter
        List<Invoker<T>> result = new ArrayList<>();
        // all invokers that don't have "tag" parameter in url is a normal invoker
        List<Invoker<T>> normalResult = new ArrayList<>();
        try {
            // Dynamic param
            String tag = RpcContext.getContext().getAttachment(Constants.REQUEST_TAG_KEY);

            for (Invoker<T> invoker : invokers) {
                if (StringUtils.isEmpty(invoker.getUrl().getParameter(Constants.TAG_KEY))) {
                    // all invokers that don't have "tag" parameter in url is a normal invoker
                    normalResult.add(invoker);
                } else {
                    String[] tags = invoker.getUrl().getParameter(Constants.TAG_KEY).split(",");
                    for (String s : tags) {
                        if (s.equals(tag)) {
                            result.add(invoker);
                            break;
                        }
                    }
                }
            }
            // If no invoker be selected, downgrade to normal invokers
            if (result.isEmpty()) {
                return normalResult;
            }
            return result;
        } catch (Exception e) {
            logger.error("Route by tag error,return all invokers.", e);
        }
        // Downgrade to all invokers
        return invokers;
    }
}

qixiaobo · 2020-07-20T07:42:51Z

TagRouter feature is very good, but i think an LoadBalance Wrapper can get same result ,and has smaller changes with source code:

import com.alibaba.dubbo.common.URL;
import com.alibaba.dubbo.common.logger.Logger;
import com.alibaba.dubbo.common.logger.LoggerFactory;
import com.alibaba.dubbo.common.utils.StringUtils;
import com.alibaba.dubbo.rpc.Invocation;
import com.alibaba.dubbo.rpc.Invoker;
import com.alibaba.dubbo.rpc.RpcContext;
import com.alibaba.dubbo.rpc.RpcException;
import com.alibaba.dubbo.rpc.cluster.LoadBalance;

import java.util.ArrayList;
import java.util.List;


public class LoadbalanceTagFilterWrapper implements LoadBalance {
    private static final Logger logger = LoggerFactory.getLogger(LoadbalanceTagFilterWrapper.class);
    private LoadBalance loadBalance;

    public LoadbalanceTagFilterWrapper(LoadBalance loadBalance) {
        this.loadBalance = loadBalance;
    }

    @Override
    public <T> Invoker<T> select(List<Invoker<T>> invokers, URL url, Invocation invocation) throws RpcException {
        List<Invoker<T>> filteredInvokers = filterInvokers(invokers);
        return loadBalance.select(filteredInvokers, url, invocation);
    }

    private <T> List<Invoker<T>> filterInvokers(List<Invoker<T>> invokers) {
        // filter
        List<Invoker<T>> result = new ArrayList<>();
        // all invokers that don't have "tag" parameter in url is a normal invoker
        List<Invoker<T>> normalResult = new ArrayList<>();
        try {
            // Dynamic param
            String tag = RpcContext.getContext().getAttachment(Constants.REQUEST_TAG_KEY);

            for (Invoker<T> invoker : invokers) {
                if (StringUtils.isEmpty(invoker.getUrl().getParameter(Constants.TAG_KEY))) {
                    // all invokers that don't have "tag" parameter in url is a normal invoker
                    normalResult.add(invoker);
                } else {
                    String[] tags = invoker.getUrl().getParameter(Constants.TAG_KEY).split(",");
                    for (String s : tags) {
                        if (s.equals(tag)) {
                            result.add(invoker);
                            break;
                        }
                    }
                }
            }
            // If no invoker be selected, downgrade to normal invokers
            if (result.isEmpty()) {
                return normalResult;
            }
            return result;
        } catch (Exception e) {
            logger.error("Route by tag error,return all invokers.", e);
        }
        // Downgrade to all invokers
        return invokers;
    }
}

Very interesting , so I should say route and loadBalance maybe a little redundancy of functions;
But in fact, load balance may just do something like aim of making their overall processing more efficient.
So router maybe the better choice in semantic.

qixiaobo · 2020-07-20T07:45:30Z

How about providing a filter to set tag attachment at same time ?
I think we will invoke not only one service but the attachments will be cleared after invoking provider every time

lexburner added 10 commits August 8, 2018 17:00

tagRouter feature

04ce01d

update dubbo.xsd

c3c9d48

remove reference router param

307dcd2

add Unit Test

fc4ef14

rollback pom.xml for merge

4f73d77

rollback pom.xml for merge

5d6677c

fix checkstyle

b3cc038

fix checkstyle

c887624

fix unit test

be39988

format import style

b70c32c

diecui1202 added the status/waiting-for-feedback Need reporters to triage label Aug 17, 2018

resolve conflict from master

bf3915c

kimmking requested changes Aug 20, 2018

View reviewed changes

Merge remote-tracking branch 'upstream/master' into tag_route_feature

b93fbfc

beiwei30 reviewed Aug 21, 2018

View reviewed changes

lexburner added 2 commits August 21, 2018 14:01

add license&remove author info

8a20925

trigger again

376c0ca

beiwei30 merged commit fcd1af8 into apache:master Aug 22, 2018

lexburner mentioned this pull request Aug 23, 2018

[DISCUSS] Introduce TagRouter, a new route feature. #2341

Closed

lexburner mentioned this pull request Sep 11, 2018

add TagRouter Document apache/dubbo-website#150

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Dubbo- support tag router feature] Add a new Router implement -- TagRouter #2228

[Dubbo- support tag router feature] Add a new Router implement -- TagRouter #2228

lexburner commented Aug 11, 2018 •

edited

Loading

codecov-io commented Aug 13, 2018 •

edited

Loading

diecui1202 commented Aug 17, 2018

beiwei30 commented Aug 17, 2018

kimmking Aug 20, 2018

beiwei30 Aug 21, 2018

lexburner Aug 21, 2018

kimmking Aug 20, 2018

lexburner Aug 21, 2018

hupuxiaojun Sep 3, 2018 •

edited

Loading

beiwei30 Aug 21, 2018

beiwei30 Aug 21, 2018

beiwei30 Aug 21, 2018

lexburner Aug 21, 2018

beiwei30 Aug 21, 2018

beiwei30 Aug 21, 2018

beiwei30 commented Aug 22, 2018

hupuxiaojun commented Sep 3, 2018 •

edited

Loading

qixiaobo commented Jul 20, 2020

qixiaobo commented Jul 20, 2020

[Dubbo- support tag router feature] Add a new Router implement -- TagRouter #2228

[Dubbo- support tag router feature] Add a new Router implement -- TagRouter #2228

Conversation

lexburner commented Aug 11, 2018 • edited Loading

What is the purpose of the change

some pain points

use tag

some cases

usage

Brief changelog

Verifying this change

codecov-io commented Aug 13, 2018 • edited Loading

Codecov Report

diecui1202 commented Aug 17, 2018

beiwei30 commented Aug 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hupuxiaojun Sep 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beiwei30 commented Aug 22, 2018

hupuxiaojun commented Sep 3, 2018 • edited Loading

qixiaobo commented Jul 20, 2020

qixiaobo commented Jul 20, 2020

lexburner commented Aug 11, 2018 •

edited

Loading

codecov-io commented Aug 13, 2018 •

edited

Loading

hupuxiaojun Sep 3, 2018 •

edited

Loading

hupuxiaojun commented Sep 3, 2018 •

edited

Loading