Broadcast mode supports the collection of service response sent by ev… #7645

goodjava · 2021-04-27T13:10:12Z

For instance, a dubbo consumer broadcasts a call to every dubbo provider's health check api. The BroadcastClusterInvoker will collect the response of every one of these providers.

AlbumenJ · 2021-04-28T08:29:42Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java

        for (Invoker<T> invoker : invokers) {
+            BroadcastResult br = new BroadcastResult();


How about only build such result when enable aggerate result?

AlbumenJ · 2021-04-28T08:30:38Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java

@@ -100,7 +134,7 @@ public Result doInvoke(final Invocation invocation, List<Invoker<T>> invokers, L
            }
            throw exception;
        }
-
+        result.setAttachment(BROADCAST_RESULTS_KEY, new Gson().toJson(resultList));


was this necessary?

AlbumenJ · 2021-04-28T08:30:49Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastResult.java

+package org.apache.dubbo.rpc.cluster.support;
+
+/**
+ * @Author [email protected]


pls remove author tag, thx

AlbumenJ · 2021-04-28T08:32:32Z

also, pls add some ut for this pr

codecov-commenter · 2021-04-28T11:13:47Z

Codecov Report

Merging #7645 (1fac10f) into master (936c662) will decrease coverage by 0.06%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #7645      +/-   ##
============================================
- Coverage     59.13%   59.06%   -0.07%     
+ Complexity      530      529       -1     
============================================
  Files          1076     1079       +3     
  Lines         43439    43627     +188     
  Branches       6351     6369      +18     
============================================
+ Hits          25689    25770      +81     
- Misses        14913    14996      +83     
- Partials       2837     2861      +24

Impacted Files	Coverage Δ	Complexity Δ
...e/dubbo/rpc/cluster/support/BroadcastCluster2.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
.../rpc/cluster/support/BroadcastCluster2Invoker.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...che/dubbo/rpc/cluster/support/BroadcastResult.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (?)`
...tadata/DynamicConfigurationServiceNameMapping.java	`47.36% <0.00%> (-36.85%)`	`0.00% <0.00%> (ø%)`
...ache/dubbo/cache/support/AbstractCacheFactory.java	`87.50% <0.00%> (-12.50%)`	`0.00% <0.00%> (ø%)`
...nfig/configcenter/nop/NopDynamicConfiguration.java	`75.00% <0.00%> (-12.50%)`	`0.00% <0.00%> (ø%)`
...in/java/org/apache/dubbo/common/utils/JVMUtil.java	`81.13% <0.00%> (-11.33%)`	`0.00% <0.00%> (ø%)`
...che/dubbo/remoting/transport/mina/MinaChannel.java	`35.52% <0.00%> (-10.53%)`	`14.00% <0.00%> (-1.00%)`
...dubbo/remoting/exchange/support/DefaultFuture.java	`80.34% <0.00%> (-6.84%)`	`0.00% <0.00%> (ø%)`
...apache/dubbo/registry/retry/AbstractRetryTask.java	`47.22% <0.00%> (-5.56%)`	`0.00% <0.00%> (ø%)`
... and 53 more

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 936c662...1fac10f. Read the comment docs.

AlbumenJ

LGTM.

AlbumenJ · 2021-05-01T04:59:08Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastClusterInvoker.java

@@ -55,6 +62,7 @@ public Result doInvoke(final Invocation invocation, List<Invoker<T>> invokers, L
        // 100 means that an exception will be thrown last, and 0 means that as long as an exception occurs, it will be thrown.
        // see https://github.com/apache/dubbo/pull/7174
        int broadcastFailPercent = url.getParameter(BROADCAST_FAIL_PERCENT_KEY, MAX_BROADCAST_FAIL_PERCENT);
+        boolean collectAllResults = url.getParameter(BROADCAST_COLLECT_ALL_RESULT_KEY, false);


Please add some comment about this parameter

AlbumenJ · 2021-05-01T05:01:58Z

Please improve the related document ( about usage and scenes to be used ) in dubbo-website after this PR merged.

xiaoheng1

LGTM

goodjava · 2021-05-06T06:00:29Z

Please improve the related document ( about usage and scenes to be used ) in dubbo-website after this PR merged.

get

AlbumenJ · 2021-05-11T03:01:54Z

dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/support/BroadcastCluster2Invoker.java

+        if (hasException(res.exception)) {
+            return createResult(invocation, res.exception, res.resultList);
+        }
+        Result result = res.resultList.get(0).getResult();


why not use getter
also will this produce npe?

…ery dubbo provider.

goodjava force-pushed the feat_27 branch 2 times, most recently from 915d08f to 64cda74 Compare April 28, 2021 02:21

AlbumenJ reviewed Apr 28, 2021

View reviewed changes

goodjava force-pushed the feat_27 branch from 64cda74 to 26d69c7 Compare April 28, 2021 10:33

AlbumenJ approved these changes Apr 30, 2021

View reviewed changes

AlbumenJ reviewed May 1, 2021

View reviewed changes

xiaoheng1 approved these changes May 3, 2021

View reviewed changes

goodjava force-pushed the feat_27 branch 2 times, most recently from baba5f3 to 1fac10f Compare May 11, 2021 02:01

AlbumenJ reviewed May 11, 2021

View reviewed changes

Broadcast mode supports the collection of service response sent by ev…

f524202

…ery dubbo provider.

goodjava force-pushed the feat_27 branch from 1fac10f to f524202 Compare May 11, 2021 05:57

AlbumenJ approved these changes May 13, 2021

View reviewed changes

AlbumenJ merged commit b7419d4 into apache:master May 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Broadcast mode supports the collection of service response sent by ev… #7645

Broadcast mode supports the collection of service response sent by ev… #7645

goodjava commented Apr 27, 2021 •

edited

Loading

AlbumenJ Apr 28, 2021

goodjava Apr 28, 2021

AlbumenJ Apr 28, 2021

goodjava Apr 28, 2021

AlbumenJ Apr 28, 2021

goodjava Apr 28, 2021

AlbumenJ commented Apr 28, 2021

codecov-commenter commented Apr 28, 2021 •

edited

Loading

AlbumenJ left a comment

AlbumenJ May 1, 2021

goodjava May 8, 2021

AlbumenJ commented May 1, 2021

xiaoheng1 left a comment

goodjava commented May 6, 2021

AlbumenJ May 11, 2021

		for (Invoker<T> invoker : invokers) {
		BroadcastResult br = new BroadcastResult();

Broadcast mode supports the collection of service response sent by ev… #7645

Broadcast mode supports the collection of service response sent by ev… #7645

Conversation

goodjava commented Apr 27, 2021 • 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

AlbumenJ commented Apr 28, 2021

codecov-commenter commented Apr 28, 2021 • edited Loading

Codecov Report

AlbumenJ left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlbumenJ commented May 1, 2021

xiaoheng1 left a comment

Choose a reason for hiding this comment

goodjava commented May 6, 2021

Choose a reason for hiding this comment

goodjava commented Apr 27, 2021 •

edited

Loading

codecov-commenter commented Apr 28, 2021 •

edited

Loading