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

[ISSUE #5929] Fix the issue that broker send heartbeat to controller frequently causing thread blocking when the network partition #5930

Merged
merged 8 commits into from
Jan 30, 2023

Conversation

RongtongJin
Copy link
Contributor

Make sure set the target branch to develop

What is the purpose of the change

Close #5929
Fix the issue that broker send heartbeat to controller frequently causing thread blocking when the network partition

Brief changelog

  1. Regularly scan whether the address of controllers is connected
  2. Heartbeat is only sent to available controller addresses

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.

  • 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 [ISSUE #123] Fix UnknownException when host config not exist. 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(over 80% coverage) 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 -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

…sing thread blocking when the network partition
@RongtongJin RongtongJin marked this pull request as draft January 26, 2023 03:31
@codecov-commenter
Copy link

codecov-commenter commented Jan 26, 2023

Codecov Report

Merging #5930 (0c8c4bf) into develop (0ea9301) will increase coverage by 0.00%.
The diff coverage is 41.17%.

@@            Coverage Diff             @@
##             develop    #5930   +/-   ##
==========================================
  Coverage      43.34%   43.34%           
- Complexity      8802     8806    +4     
==========================================
  Files           1090     1090           
  Lines          76435    76467   +32     
  Branches        9987     9993    +6     
==========================================
+ Hits           33129    33148   +19     
- Misses         39166    39179   +13     
  Partials        4140     4140           
Impacted Files Coverage Δ
...a/org/apache/rocketmq/broker/BrokerController.java 43.25% <0.00%> (ø)
...org/apache/rocketmq/broker/out/BrokerOuterAPI.java 16.52% <0.00%> (-0.03%) ⬇️
...e/rocketmq/remoting/netty/NettyRemotingClient.java 40.94% <0.00%> (-0.55%) ⬇️
...he/rocketmq/broker/controller/ReplicasManager.java 47.01% <56.00%> (+3.57%) ⬆️
...pl/consumer/ConsumeMessageConcurrentlyService.java 46.60% <0.00%> (-2.27%) ⬇️
...a/org/apache/rocketmq/filter/util/BloomFilter.java 60.43% <0.00%> (-2.20%) ⬇️
...he/rocketmq/client/trace/AsyncTraceDispatcher.java 82.21% <0.00%> (-1.45%) ⬇️
...he/rocketmq/client/impl/consumer/ProcessQueue.java 59.91% <0.00%> (-1.33%) ⬇️
.../apache/rocketmq/store/ha/DefaultHAConnection.java 65.86% <0.00%> (-1.21%) ⬇️
...a/org/apache/rocketmq/store/StoreStatsService.java 34.53% <0.00%> (-0.56%) ⬇️
... and 14 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@RongtongJin RongtongJin marked this pull request as ready for review January 26, 2023 11:35
Comment on lines +536 to +538
public List<String> getAvailableControllerAddresses() {
return new ArrayList<>(availableControllerAddresses.keySet());
}
Copy link
Member

Choose a reason for hiding this comment

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

How about set ArrayList initialCapacity to prevent expansion

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This list will not expand.

odbozhou
odbozhou previously approved these changes Jan 29, 2023
mxsm
mxsm previously approved these changes Jan 30, 2023
Copy link
Member

@mxsm mxsm left a comment

Choose a reason for hiding this comment

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

LGTM~

@RongtongJin RongtongJin dismissed stale reviews from mxsm and odbozhou via 12bfb5b January 30, 2023 02:18
@MatrixHB MatrixHB merged commit 1e7c861 into apache:develop Jan 30, 2023
RongtongJin added a commit to RongtongJin/rocketmq that referenced this pull request Jan 30, 2023
…oller frequently causing thread blocking when the network partition (apache#5930)

* Fix the issue that broker send heartbeat to controller frequently causing thread blocking when the network partition

* Fix the ReplicasManagerTest can not pass

* Reslove conflict with apache#5922

* Fix code style

* Rename isAddressCanConnect to isAddressReachable

* Rename isAddressCanConnect to isAddressReachable
@RongtongJin RongtongJin added the module/ha high availably related label Feb 14, 2023
drpmma pushed a commit that referenced this pull request Feb 21, 2023
…frequently causing thread blocking when the network partition (#5930)

* Fix the issue that broker send heartbeat to controller frequently causing thread blocking when the network partition

* Fix the ReplicasManagerTest can not pass

* Reslove conflict with #5922

* Fix code style

* Rename isAddressCanConnect to isAddressReachable

* Rename isAddressCanConnect to isAddressReachable
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/ha high availably related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The broker send heartbeat to controller frequently causing thread blocking when the network partition
7 participants