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 #8912] Rebalancing logic shift in POP consumption mode #8923

Open
wants to merge 14 commits into
base: develop
Choose a base branch
from

Conversation

3424672656
Copy link
Contributor

Which Issue(s) This PR Fixes

Fixes #8912

Brief Description

https://shimo.im/docs/Ee32mZB85xcK87A2

How Did You Test This Change?

@codecov-commenter
Copy link

codecov-commenter commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 13.46154% with 180 lines in your changes missing coverage. Please review.

Project coverage is 47.48%. Comparing base (66ba456) to head (4007355).
Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
...e/rocketmq/client/impl/consumer/RebalanceImpl.java 3.00% 95 Missing and 2 partials ⚠️
...cketmq/client/impl/consumer/RebalancePushImpl.java 16.27% 35 Missing and 1 partial ⚠️
...rocketmq/client/impl/factory/MQClientInstance.java 0.00% 23 Missing ⚠️
.../rocketmq/client/impl/ClientRemotingProcessor.java 0.00% 11 Missing ⚠️
...g/apache/rocketmq/client/impl/MQClientAPIImpl.java 0.00% 6 Missing ⚠️
...ache/rocketmq/broker/client/net/Broker2Client.java 83.33% 2 Missing ⚠️
.../java/org/apache/rocketmq/client/ClientConfig.java 50.00% 2 Missing ⚠️
...tmq/broker/processor/QueryAssignmentProcessor.java 85.71% 0 Missing and 1 partial ⚠️
...apache/rocketmq/tools/admin/DefaultMQAdminExt.java 0.00% 1 Missing ⚠️
...he/rocketmq/tools/admin/DefaultMQAdminExtImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #8923      +/-   ##
=============================================
- Coverage      47.63%   47.48%   -0.16%     
+ Complexity     11756    11736      -20     
=============================================
  Files           1304     1304              
  Lines          91046    91263     +217     
  Branches       11672    11715      +43     
=============================================
- Hits           43370    43336      -34     
- Misses         42341    42576     +235     
- Partials        5335     5351      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qianye1001
Copy link
Contributor

Why not reduce the computational load on the server side by adding a cache to the broker, instead of adopting a client-side computation approach?

@3424672656
Copy link
Contributor Author

Why not reduce the computational load on the server side by adding a cache to the broker, instead of adopting a client-side computation approach?

Do you mean the results of the distribution cached on the broker side? When POP consumption mode is enabled, I think there are also a large number of network requests, and each topic subscribed by the corresponding client is a network request, which does not seem to solve the problem completely. Transferring to the client side can unify the rebalancing logic. Because it doesn't seem easy to completely isolate POP and PULL rebalancing logic in the original version, and the initial client execution is ideally local rebalancing

@3424672656
Copy link
Contributor Author

3424672656 commented Nov 14, 2024

Why not reduce the computational load on the server side by adding a cache to the broker, instead of adopting a client-side computation approach?
And the server cache should also consider the timely perception and update of cache, cache update and rebalancing occurred at the same time caused by the delay and so on

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.

[Feature] A shift in the rebalancing logic
3 participants