Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix the bug of broadcast cluster invoke #7174

Merged
merged 10 commits into from Feb 22, 2021
Merged

Conversation

xiaoheng1
Copy link
Contributor

What is the purpose of the change

fix the bug of broadcast cluster invoke #7173

Brief changelog

XXXXX

Verifying this change

XXXXX

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

  • Make sure there is a GITHUB_issue field 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 sample in dubbo samples project.
  • Run mvn clean install -DskipTests=false & 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
Copy link

codecov-io commented Feb 5, 2021

Codecov Report

Merging #7174 (cfd7dcb) into master (9cecbc3) will decrease coverage by 0.24%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7174      +/-   ##
============================================
- Coverage     59.97%   59.73%   -0.25%     
+ Complexity      288      287       -1     
============================================
  Files          1004     1004              
  Lines         39976    39993      +17     
  Branches       5905     5910       +5     
============================================
- Hits          23974    23888      -86     
- Misses        13311    13390      +79     
- Partials       2691     2715      +24     
Impacted Files Coverage Δ Complexity Δ
...o/rpc/cluster/support/BroadcastClusterInvoker.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ng/transport/dispatcher/all/AllChannelHandler.java 62.06% <0.00%> (-27.59%) 0.00% <0.00%> (ø%)
...bo/config/event/listener/LoggingEventListener.java 37.50% <0.00%> (-25.00%) 0.00% <0.00%> (ø%)
...dubbo/common/status/support/LoadStatusChecker.java 50.00% <0.00%> (-16.67%) 0.00% <0.00%> (ø%)
...ng/transport/dispatcher/WrappedChannelHandler.java 47.82% <0.00%> (-15.22%) 0.00% <0.00%> (ø%)
...va/org/apache/dubbo/remoting/TimeoutException.java 22.22% <0.00%> (-11.12%) 0.00% <0.00%> (ø%)
...ava/org/apache/dubbo/config/DubboShutdownHook.java 63.82% <0.00%> (-10.64%) 0.00% <0.00%> (ø%)
.../remoting/transport/netty4/NettyClientHandler.java 57.62% <0.00%> (-10.17%) 0.00% <0.00%> (ø%)
...dubbo/remoting/exchange/support/DefaultFuture.java 79.64% <0.00%> (-7.97%) 0.00% <0.00%> (ø%)
...apache/dubbo/common/config/ConfigurationUtils.java 53.12% <0.00%> (-6.25%) 0.00% <0.00%> (ø%)
... and 23 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 9cecbc3...cfd7dcb. Read the comment docs.

@qq664042
Copy link

qq664042 commented Feb 5, 2021

image

这个修改好像还是不太完美, 提前抛异常的话,后面可能还有结点没有执行

@xiaoheng1
Copy link
Contributor Author

广播模式下,只要任意一台报错,最终会返回报错。在这种情况下,继续调度其他节点,我认为这个是没有意义的调度,所以就提前抛出异常了。

@qq664042
Copy link

qq664042 commented Feb 5, 2021

我觉得有意义吧,其实我这还想提一个新的需求,不知道是否采纳, 广播模式调用有生产者节点, 要能获取到全部节点的返回结果信息, 通过ThreadLocal保存全部节点的信息,消息者在通过ThreadLocal获得该信息。

因为有这一个现实意义的使用场景。

@qq664042
Copy link

qq664042 commented Feb 5, 2021

假设生产者有三个节点,每个节点都只保存自己的在线用户信息, 如果消息者想查询10个用户,分别存在哪几台服务器上,这么一个场景;

@xiaoheng1
Copy link
Contributor Author

假设生产者有三个节点,每个节点都只保存自己的在线用户信息, 如果消息者想查询10个用户,分别存在哪几台服务器上,这么一个场景;

这种场景下,假设一个节点出现问题,消费者查询 10 个用户在其他两个节点上的情况,最终得到的结果还是异常。那么在第一次查询到异常的时候,是否还需要继续查询?这个可以讨论下。

@qq664042
Copy link

qq664042 commented Feb 5, 2021

假设生产者有三个节点,每个节点都只保存自己的在线用户信息, 如果消息者想查询10个用户,分别存在哪几台服务器上,这么一个场景;

这种场景下,假设一个节点出现问题,消费者查询 10 个用户在其他两个节点上的情况,最终得到的结果还是异常。那么在第一次查询到异常的时候,是否还需要继续查询?这个可以讨论下。

这个时候可以不主动抛出result.getException()异常啊,总之保证可以通过ThreadLocal获取到全部的结果信息

@xiaoheng1
Copy link
Contributor Author

假设生产者有三个节点,每个节点都只保存自己的在线用户信息, 如果消息者想查询10个用户,分别存在哪几台服务器上,这么一个场景;

这种场景下,假设一个节点出现问题,消费者查询 10 个用户在其他两个节点上的情况,最终得到的结果还是异常。那么在第一次查询到异常的时候,是否还需要继续查询?这个可以讨论下。

这个时候可以不主动抛出result.getException()异常啊,总之保证可以通过ThreadLocal获取到全部的结果信息

通过 ThreadLocal 如何获取全部数据了?我的理解是一个节点异常,获取不到完整结果吧。

@qq664042
Copy link

qq664042 commented Feb 5, 2021

假设生产者有三个节点,每个节点都只保存自己的在线用户信息, 如果消息者想查询10个用户,分别存在哪几台服务器上,这么一个场景;

这种场景下,假设一个节点出现问题,消费者查询 10 个用户在其他两个节点上的情况,最终得到的结果还是异常。那么在第一次查询到异常的时候,是否还需要继续查询?这个可以讨论下。

这个时候可以不主动抛出result.getException()异常啊,总之保证可以通过ThreadLocal获取到全部的结果信息

通过 ThreadLocal 如何获取全部数据了?我的理解是一个节点异常,获取不到完整结果吧。

如果节点异常之后,可以不抛出去啊,让他顺序调用完所有的节点,然后将所有节点的结果信息保存在ThreadLocal里面

@xiaoheng1
Copy link
Contributor Author

假设生产者有三个节点,每个节点都只保存自己的在线用户信息, 如果消息者想查询10个用户,分别存在哪几台服务器上,这么一个场景;

这种场景下,假设一个节点出现问题,消费者查询 10 个用户在其他两个节点上的情况,最终得到的结果还是异常。那么在第一次查询到异常的时候,是否还需要继续查询?这个可以讨论下。

这个时候可以不主动抛出result.getException()异常啊,总之保证可以通过ThreadLocal获取到全部的结果信息

通过 ThreadLocal 如何获取全部数据了?我的理解是一个节点异常,获取不到完整结果吧。

如果节点异常之后,可以不抛出去啊,让他顺序调用完所有的节点,然后将所有节点的结果信息保存在ThreadLocal里面

这样所有节点都调用完了,得到的数据也是不准确的啊。比如 K 在节点 A,B 出现过了,在 C 没有出现。那么调用结果就算 A 异常,B 1, C 0. 这样的数据放到 ThreadLocal 如何处理?

并且这个广播的设计摘自官方文档:

广播调用所有提供者,逐个调用,任意一台报错则报错。通常用于通知所有提供者更新缓存或日志等本地资源信息。

@qq664042
Copy link

qq664042 commented Feb 5, 2021

用于通知所有提供者更新缓存或日志等本地资源信息。

假设生产者有三个节点,每个节点都只保存自己的在线用户信息, 如果消息者想查询10个用户,分别存在哪几台服务器上,这么一个场景;

这种场景下,假设一个节点出现问题,消费者查询 10 个用户在其他两个节点上的情况,最终得到的结果还是异常。那么在第一次查询到异常的时候,是否还需要继续查询?这个可以讨论下。

这个时候可以不主动抛出result.getException()异常啊,总之保证可以通过ThreadLocal获取到全部的结果信息

通过 ThreadLocal 如何获取全部数据了?我的理解是一个节点异常,获取不到完整结果吧。

如果节点异常之后,可以不抛出去啊,让他顺序调用完所有的节点,然后将所有节点的结果信息保存在ThreadLocal里面

这样所有节点都调用完了,得到的数据也是不准确的啊。比如 K 在节点 A,B 出现过了,在 C 没有出现。那么调用结果就算 A 异常,B 1, C 0. 这样的数据放到 ThreadLocal 如何处理?

并且这个广播的设计摘自官方文档:

广播调用所有提供者,逐个调用,任意一台报错则报错。通常用于通知所有提供者更新缓存或日志等本地资源信息。

呃,那我使用不对了,就现在就是这么改的,解决我刚才说的那个场景问题, 保存的大数据大概是,K|null|Exception B|server1|null

@CalvinKirs
Copy link
Member

For better communication, please use English. Thx

@xiaoheng1
Copy link
Contributor Author

Ok, follow up communication in english.

@AlbumenJ
Copy link
Member

Please add some usage description for this pr in https://github.com/apache/dubbo-website repo. After you create pr here, please make it link to this pr.

Copy link
Member

@AlbumenJ AlbumenJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants