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 #8829] feat: provide ConfigManagerV2 to make best uses of RocksDB #8856

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

lizhanhui
Copy link
Contributor

Which Issue(s) This PR Fixes

To #8829

Brief Description

How Did You Test This Change?

Signed-off-by: Li Zhanhui <lizhanhui@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2024

Codecov Report

Attention: Patch coverage is 2.12389% with 553 lines in your changes missing coverage. Please review.

Project coverage is 47.05%. Comparing base (7f8667a) to head (b128882).
Report is 66 commits behind head on develop.

Files with missing lines Patch % Lines
...etmq/broker/config/v2/ConsumerOffsetManagerV2.java 0.00% 229 Missing ⚠️
...ocketmq/broker/config/v2/TopicConfigManagerV2.java 0.00% 87 Missing ⚠️
...q/broker/config/v2/SubscriptionGroupManagerV2.java 0.00% 86 Missing ⚠️
...apache/rocketmq/broker/config/v2/ConfigHelper.java 0.00% 57 Missing ⚠️
...pache/rocketmq/broker/config/v2/ConfigStorage.java 0.00% 42 Missing ⚠️
...e/rocketmq/broker/config/v2/SerializationType.java 0.00% 13 Missing ⚠️
...a/org/apache/rocketmq/broker/BrokerController.java 8.33% 6 Missing and 5 partials ⚠️
.../org/apache/rocketmq/broker/config/v2/TableId.java 0.00% 10 Missing ⚠️
...apache/rocketmq/broker/config/v2/RecordPrefix.java 0.00% 8 Missing ⚠️
.../apache/rocketmq/broker/config/v2/TablePrefix.java 0.00% 7 Missing ⚠️
... and 1 more
Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #8856      +/-   ##
=============================================
- Coverage      47.31%   47.05%   -0.27%     
- Complexity     11612    11639      +27     
=============================================
  Files           1290     1304      +14     
  Lines          90293    90984     +691     
  Branches       11609    11668      +59     
=============================================
+ Hits           42724    42812      +88     
- Misses         42292    42884     +592     
- Partials        5277     5288      +11     

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

@lizhanhui lizhanhui changed the title feat: provide ConfigManagerV2 to make best uses of RocksDB [ISSUE #8829] feat: provide ConfigManagerV2 to make best uses of RocksDB Oct 23, 2024
Signed-off-by: Li Zhanhui <lizhanhui@gmail.com>
Copy link
Contributor

@RongtongJin RongtongJin left a comment

Choose a reason for hiding this comment

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

Hi, @lizhanhui , we have been using version v1 in the production environment, and it's likely that some community users are doing the same. From a code perspective, a compatibility upgrade from v1 to v2 is not feasible. Given the need for compatibility upgrade, I think it's essential to add support for persisting lmq consumer offsets in version v1.

@lizhanhui
Copy link
Contributor Author

Hi, @lizhanhui , we have been using version v1 in the production environment, and it's likely that some community users are doing the same. From a code perspective, a compatibility upgrade from v1 to v2 is not feasible. Given the need for compatibility upgrade, I think it's essential to add support for persisting lmq consumer offsets in version v1.

Let's do this in the next pull request. This pull request has been kind of too large.

@tianliuliu
Copy link
Contributor

LGTM

@lizhanhui lizhanhui merged commit c8938e0 into apache:develop Oct 28, 2024
11 checks passed
@lizhanhui lizhanhui deleted the config-manager branch October 28, 2024 02:00
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.

6 participants