[ISSUE #7064] [RIP-66] Support KV(RocksDB) Storage #7065

fujian-zfj · 2023-07-23T03:37:18Z

Which Issue(s) This PR Fixes

Brief Description

This proposal mainly optimizes and solves the existing perfermance problems in the million-topic scenario from two levels:

Metadata like topic, subscription, consumerOffset implement kv storage
ConsumeQueue index file implements kv storage

lizhimins · 2023-07-23T07:45:55Z

common/src/main/java/org/apache/rocketmq/common/TopicConfig.java

@@ -27,7 +29,7 @@

 import static org.apache.rocketmq.common.TopicAttributes.TOPIC_MESSAGE_TYPE_ATTRIBUTE;

-public class TopicConfig {
+public class TopicConfig implements Serializable {


不建议这么改

lizhimins · 2023-07-23T07:50:28Z

common/pom.xml

@@ -104,5 +104,10 @@
            <groupId>io.github.aliyunmq</groupId>
            <artifactId>rocketmq-logback-classic</artifactId>
        </dependency>
+        <!-- rocksdb -->
+        <dependency>
+            <groupId>io.github.aliyunmq</groupId>


Why rely on a commercial version? How to ensure the stability of this version?

It's still open source, but we need a new compactionFilter for ConsumeQueue. If you're interested, see how flink uses rocksdb.

@fujian-zfj Your purpose of building custom rocksdb is to define a compaction filter. What a custom compaction filter expects is native function. Will this way be better alternative:

Use official rocksdb-java jar;

Build a shared library(dynamically linked with rocksdb shared library) for expected custom compaction filters;

When configuring RocksDB, pass the native function to it, because it does not matter which shared library the compaction filter stays in;
As a matter of fact, this is how the native programming languages use rocksdb in case of dynamic link.

aaron-ai · 2023-07-24T01:48:41Z

pom.xml

+                <groupId>io.github.aliyunmq</groupId>
+                <artifactId>rocketmq-rocksdb</artifactId>
+                <version>${rocksdb.version}</version>
+            </dependency>


Is this feature compatible with other platforms (Windows and macOS)? If not, should we set it as optional?

support Windows, MacOS and Linux

codecov-commenter · 2023-07-25T08:43:30Z

Codecov Report

Merging #7065 (ea03a1e) into develop (90c5382) will decrease coverage by 0.08%.
Report is 4 commits behind head on develop.
The diff coverage is 40.57%.

@@              Coverage Diff              @@
##             develop    #7065      +/-   ##
=============================================
- Coverage      42.72%   42.64%   -0.08%     
- Complexity      9282     9480     +198     
=============================================
  Files           1138     1157      +19     
  Lines          81146    83192    +2046     
  Branches       10619    10850     +231     
=============================================
+ Hits           34672    35481     +809     
- Misses         42137    43297    +1160     
- Partials        4337     4414      +77

Files Changed	Coverage Δ
...he/rocketmq/broker/controller/ReplicasManager.java	`44.69% <ø> (-1.59%)`	⬇️
...mq/broker/offset/RocksDBConsumerOffsetManager.java	`0.00% <0.00%> (ø)`
...ocketmq/broker/processor/SendMessageProcessor.java	`36.29% <ø> (ø)`
.../subscription/RocksDBLmqConsumerOffsetManager.java	`0.00% <0.00%> (ø)`
...bscription/RocksDBLmqSubscriptionGroupManager.java	`0.00% <0.00%> (ø)`
.../subscription/RocksDBSubscriptionGroupManager.java	`0.00% <0.00%> (ø)`
...tmq/broker/topic/RocksDBLmqTopicConfigManager.java	`0.00% <0.00%> (ø)`
...cketmq/broker/topic/RocksDBTopicConfigManager.java	`0.00% <0.00%> (ø)`
.../consumer/store/RocksDBOffsetSerializeWrapper.java	`0.00% <0.00%> (ø)`
...java/org/apache/rocketmq/common/ConfigManager.java	`56.41% <0.00%> (-3.05%)`	⬇️
... and 41 more

... and 26 files with indirect coverage changes

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

HScarb · 2023-07-27T04:04:36Z

store/src/main/java/org/apache/rocketmq/store/queue/RocksDBConsumeQueueStore.java

+        this.flushOptions.setAllowWriteStall(false);
+
+        this.writeBatch = new WriteBatch();
+        this.bufferDRList = new ArrayList(BATCH_SIZE);


Raw use of parameterized class

lizhanhui · 2023-07-28T05:52:41Z

It's a huge pull request, containing massive changes to core parts of this project with more than 6000 lines of change. Thus, it's challenging to have a quality review. Normally, developers would break it into a series of small and verifiable ones to keep community participation.

lizhanhui · 2023-07-28T05:55:09Z

broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java

@@ -746,7 +760,12 @@ public boolean initializeMetadata() {
    public boolean initializeMessageStore() {
        boolean result = true;
        try {
-            DefaultMessageStore defaultMessageStore = new DefaultMessageStore(this.messageStoreConfig, this.brokerStatsManager, this.messageArrivingListener, this.brokerConfig, topicConfigManager.getTopicConfigTable());
+            DefaultMessageStore defaultMessageStore;
+            if (isEnableRocksDBStore()) {


Will SPI be a better choice than if...else?

lizhanhui · 2023-07-28T05:57:52Z

broker/src/main/java/org/apache/rocketmq/broker/controller/ReplicasManager.java

@@ -225,7 +225,7 @@ public void shutdown() {

    public synchronized void changeBrokerRole(final Long newMasterBrokerId, final String newMasterAddress,
        final Integer newMasterEpoch,
-        final Integer syncStateSetEpoch, final Set<Long> syncStateSet) {
+        final Integer syncStateSetEpoch, final Set<Long> syncStateSet) throws Exception {


Exception is too general...Create one within the project exception hierarchy

lizhanhui · 2023-07-28T05:58:06Z

broker/src/main/java/org/apache/rocketmq/broker/controller/ReplicasManager.java

@@ -235,7 +235,7 @@ public synchronized void changeBrokerRole(final Long newMasterBrokerId, final St
        }
    }

-    public void changeToMaster(final int newMasterEpoch, final int syncStateSetEpoch, final Set<Long> syncStateSet) {
+    public void changeToMaster(final int newMasterEpoch, final int syncStateSetEpoch, final Set<Long> syncStateSet) throws Exception {


lizhanhui

Please break this huge PR into smaller ones

fujian-zfj · 2023-07-28T08:15:34Z

Please break this huge PR into smaller ones

OK, I will split it into two PRS, metadata and consumeQueue

fujian-zfj and others added 30 commits November 30, 2021 20:19

typo int readme[ecosystem]

09c0c8c

Merge branch 'apache:develop' into develop

89713ba

Merge branch 'apache:develop' into develop

8d0c4fc

Merge branch 'apache:develop' into develop

2c9e961

Merge branch 'apache:develop' into develop

93d6669

Merge branch 'apache:develop' into develop

2dd16d6

Merge branch 'apache:develop' into develop

e02c7aa

Merge branch 'apache:develop' into develop

cb8c7b0

Merge branch 'apache:develop' into develop

03f707f

Merge branch 'apache:develop' into develop

6ef608f

Merge branch 'apache:develop' into develop

818919f

Merge branch 'apache:develop' into develop

b315381

Merge branch 'apache:develop' into develop

5d1b050

Merge branch 'apache:develop' into develop

de2238b

Merge branch 'apache:develop' into develop

d22756f

Merge branch 'apache:develop' into develop

83d8178

Merge branch 'apache:develop' into develop

a4c94a9

Merge branch 'apache:develop' into develop

7234a41

support rocksdb

c4e11cd

remove unused import and 'throws exception'

d9db620

checkstyle

171e74c

fix dirty topic

ec64dc8

fix storeCheckpoint

8401f02

fix TimerMessageStore.TIMER_TOPIC do not create problem

fc10830

default rocksdb

c7d4702

Merge branch 'apache:develop' into develop

c7708a7

merge

a082ccb

Merge branch 'apache:develop' into develop

166ebbd

Merge branch 'develop' into develop_rocksdb

300aa16

remove unused import

46979b0

fujian-zfj and others added 6 commits July 13, 2023 15:27

rocketmq-rocksdb final release

f7623cd

fix npe b ug

c92ab9a

comment

d7e022e

comment

248da07

Merge branch 'apache:develop' into develop

6b27cc7

merge current branch

ff6b744

lizhimins reviewed Jul 23, 2023

View reviewed changes

lizhimins requested changes Jul 23, 2023

View reviewed changes

aaron-ai reviewed Jul 24, 2023

View reviewed changes

fujian-zfj added 3 commits July 24, 2023 14:29

remove the seriablizale of topicConfig and subscriptionConfig

b461b57

rocksdb support windows

5d3ff5d

file separator error in windows

49505a1

fujian-zfj added 3 commits July 26, 2023 09:24

test 1.0.1 mac lib

ea03a1e

new rocksdb version 1.0.3

97124f0

skip rocksdb test in mac os

ec04e21

HScarb reviewed Jul 27, 2023

View reviewed changes

lizhanhui reviewed Jul 28, 2023

View reviewed changes

lizhanhui requested changes Jul 28, 2023

View reviewed changes

fujian-zfj closed this Jul 28, 2023

fujian-zfj reopened this Jul 28, 2023

optimize

ebd9a22

RongtongJin closed this Jan 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ISSUE #7064] [RIP-66] Support KV(RocksDB) Storage #7065

[ISSUE #7064] [RIP-66] Support KV(RocksDB) Storage #7065

fujian-zfj commented Jul 23, 2023

lizhimins Jul 23, 2023

fujian-zfj Jul 24, 2023

lizhimins Jul 23, 2023

fujian-zfj Jul 23, 2023

lizhanhui Jul 28, 2023

aaron-ai Jul 24, 2023

fujian-zfj Jul 24, 2023

codecov-commenter commented Jul 25, 2023 •

edited

Loading

HScarb Jul 27, 2023

fujian-zfj Jul 28, 2023

lizhanhui commented Jul 28, 2023

lizhanhui Jul 28, 2023

lizhanhui Jul 28, 2023

lizhanhui Jul 28, 2023

fujian-zfj Jul 28, 2023

lizhanhui left a comment

fujian-zfj commented Jul 28, 2023

[ISSUE #7064] [RIP-66] Support KV(RocksDB) Storage #7065

[ISSUE #7064] [RIP-66] Support KV(RocksDB) Storage #7065

Conversation

fujian-zfj commented Jul 23, 2023

Which Issue(s) This PR Fixes

Brief Description

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

Choose a reason for hiding this comment

codecov-commenter commented Jul 25, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lizhanhui commented Jul 28, 2023

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

lizhanhui left a comment

Choose a reason for hiding this comment

fujian-zfj commented Jul 28, 2023

codecov-commenter commented Jul 25, 2023 •

edited

Loading