-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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 #8589] Support file format CQ and json format offset in-place upgrade to rocksdb management #8600
Conversation
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java
Outdated
Show resolved
Hide resolved
...s/src/main/java/org/apache/rocketmq/tools/command/export/ExportMetadataInRocksDBCommand.java
Outdated
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/offset/RocksDBConsumerOffsetManager.java
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
Outdated
Show resolved
Hide resolved
...r/src/main/java/org/apache/rocketmq/broker/subscription/RocksDBSubscriptionGroupManager.java
Outdated
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/topic/TopicConfigManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving offset management to RocksDB is very good to have. but decisions and rational should be explained.
broker/src/main/java/org/apache/rocketmq/broker/offset/RocksDBConsumerOffsetManager.java
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
Outdated
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
Outdated
Show resolved
Hide resolved
common/src/main/java/org/apache/rocketmq/common/config/AbstractRocksDBStorage.java
Outdated
Show resolved
Hide resolved
tools/src/main/java/org/apache/rocketmq/tools/command/queue/DiffConsumeQueueCommand.java
Outdated
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/subscription/SubscriptionGroupManager.java
Outdated
Show resolved
Hide resolved
store/src/main/java/org/apache/rocketmq/store/queue/RocksDBConsumeQueue.java
Show resolved
Hide resolved
...s/src/main/java/org/apache/rocketmq/tools/command/export/ExportMetadataInRocksDBCommand.java
Outdated
Show resolved
Hide resolved
store/src/main/java/org/apache/rocketmq/store/config/MessageStoreConfig.java
Outdated
Show resolved
Hide resolved
tools/src/main/java/org/apache/rocketmq/tools/command/queue/DiffConsumeQueueCommand.java
Outdated
Show resolved
Hide resolved
...s/src/main/java/org/apache/rocketmq/tools/command/export/ExportMetadataInRocksDBCommand.java
Outdated
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java
Outdated
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/offset/ConsumerOffsetManager.java
Show resolved
Hide resolved
我们rocksdb方案中,topic,group,offset,cq都是存储在rocksdb中,前三者我们认为都是元数据 |
broker/src/test/java/org/apache/rocketmq/broker/offset/RocksdbTransferOffsetAndCqTest.java
Show resolved
Hide resolved
store/src/main/java/org/apache/rocketmq/store/config/MessageStoreConfig.java
Outdated
Show resolved
Hide resolved
我们的rocksdb方案 offset已经是rocksdb存储的了,不是这里引入的 |
broker/src/main/java/org/apache/rocketmq/broker/offset/ConsumerOffsetManager.java
Outdated
Show resolved
Hide resolved
store/src/main/java/org/apache/rocketmq/store/queue/RocksDBConsumeQueue.java
Show resolved
Hide resolved
store/src/main/java/org/apache/rocketmq/store/queue/RocksDBConsumeQueueStore.java
Show resolved
Hide resolved
broker/src/main/java/org/apache/rocketmq/broker/offset/RocksDBConsumerOffsetManager.java
Show resolved
Hide resolved
There is another issue to address: file backed ConsumeQueue now unconditionally preserve the last file in the file queue. If configured to RocksDB only, need to clean them up once the file-based CQ segments are totally expired. |
As previously mentioned, overall it is good to have PR, but there are many issues to resolve yet. |
It's NOT about introducing RocksDB, it's about decisions how to migrate from file-based one to the KV counterpart. |
和元数据(topic,group)一样,在启动的时候会校验DataVersion,然后Transfer一次 Like the metadata (topic, group), DataVersion will be verified upon startup, and then transferred once. |
Got another issue, if rocksdbCQDoubleWriteEnable is on, meaning users still have chance to rollback to file-based CQ and offset management, offsets should also write in dual manner. This PR currently only write to KV. |
#8699 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look forward to see this awesome feature in prod with following issues resolved. Nice job overall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting for code review
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Which Issue(s) This PR Fixes
Fixes #8589
Brief Description
How Did You Test This Change?