RIP-7 Multiple Directories Storage Support #3357

Jason918 · 2021-09-15T14:43:46Z

Make sure set the target branch to develop

What is the purpose of the change

Follow up PR of #3348
Update according to comments, see change log

Brief changelog

remove isMultiCommitlogPathEnable
In org.apache.rocketmq.store.DefaultMessageStore.CleanCommitLogService#isSpaceToDelete, use minPhysicRatio to determin when the broker's disk is full. And mark all store path when it's full.
In org.apache.rocketmq.store.MultiPathMappedFileQueue#tryCreateMappedFile, we ignore readonly paths and paths marked full in step 2.

Verifying this change

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.

broker/src/main/java/org/apache/rocketmq/broker/processor/SendMessageProcessor.java

store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java

…cessor.SendMessageProcessor#diskUtil 2. use minPhysicsUsedRatio instead of max in org.apache.rocketmq.store.DefaultMessageStore#getRuntimeInfo

francisoliverlee

checked and muti commit log dir work well. but some errors in DiskCheckScheduledThread and AdminBrokerThread measuring Disk usage.it seems AdminBrokerThread error when no any topics

2021-09-17 02:24:50 ERROR DiskCheckScheduledThread1 - Error when measuring disk space usage, file doesn't exist on this path: /opt/rocketmq/master-data1/commitlog:/opt/rocketmq/master-data2/commitlog
2021-09-17 02:25:00 ERROR AdminBrokerThread_7 - Error when measuring disk space usage, file doesn't exist on this path: /opt/rocketmq/master-data1/consumequeue
2021-09-17 02:25:00 ERROR DiskCheckScheduledThread1 - Error when measuring disk space usage, file doesn't exist on this path: /opt/rocketmq/master-data1/commitlog:/opt/rocketmq/master-data2/commitlog
2021-09-17 02:25:03 ERROR AdminBrokerThread_8 - Error when measuring disk space usage, file doesn't exist on this path: /opt/rocketmq/master-data1/consumequeue
2021-09-17 02:25:10 ERROR DiskCheckScheduledThread1 - Error when measuring disk space usage, file doesn't exist on this path: /opt/rocketmq/master-data1/commitlog:/opt/rocketmq/master-data2/commitlog

…tLogService#isSpaceFull

Jason918 · 2021-09-17T03:21:01Z

DiskCheckScheduledThread1

@francisoliverlee Thank you for the notice.
Error in DiskCheckScheduledThread1 is due to storePathCommitLog usage in "DefaultMessageStore.CleanCommitLogService#isSpaceFull". I fixed it and added test in last commit.
Sorry for missing that. I have checked all the usage of storePathCommitLog.

Jason918 · 2021-09-17T03:25:05Z

AdminBrokerThread error when no any topics

@francisoliverlee
The log shows that the error in AdminBrokerThread is due to the directory for storing CQs is not created.
It's a little bit strange, this PR should have nothing to do with CQs.
I would try to reproduce it.

…DefaultMessageStore#getRuntimeInfo

Jason918 · 2021-09-18T09:03:02Z

@francisoliverlee Added path empty check to avoid error log. Checked with bin/mqadmin brokerStatus -n 127.1:9876 -b 127.1:10911, no error logs appears in broker.log.
Please take a look.

RongtongJin

LGTM~, but only one disk is written at the same time, which does not take advantage of the increased write bandwidth after multi-disk deployment.

store/src/main/java/org/apache/rocketmq/store/config/MessageStoreConfig.java

duhenglucky · 2021-09-20T17:25:28Z

DiskCheckScheduledThread1

@francisoliverlee Thank you for the notice.
Error in DiskCheckScheduledThread1 is due to storePathCommitLog usage in "DefaultMessageStore.CleanCommitLogService#isSpaceFull". I fixed it and added test in last commit.
Sorry for missing that. I have checked all the usage of storePathCommitLog.

It seems this error still exists ，and maybe you discarded this method org.apache.rocketmq.common.UtilAll#getDiskPartitionSpaceUsedPercent, or should we check the existence of a directory at the start of the broker?

Jason918 · 2021-09-21T10:10:16Z

It seems this error still exists ，and maybe you discarded this method org.apache.rocketmq.common.UtilAll#getDiskPartitionSpaceUsedPercent, or should we check the existence of a directory at the start of the broker?

Yes, for now, user have to put existing file paths in storePathCommitLog.

Added isPathExists check in 'DiskCheckScheduledThread'.

PTAL

…Config#MULTI_PATH_SPLITTER to ',' and can be changed by System.getProperty("rocketmq.broker.multiPathSplitter")

Jason918 · 2021-09-21T10:24:19Z

LGTM~, but only one disk is written at the same time, which does not take advantage of the increased write bandwidth after multi-disk deployment.

After this multi-disk deployment, read bandwidth will be fully utilized.
In current one big commit log writing pattern, writing concurrently requires much more work. Maybe soft raid is more suitable for now, if write bandwidth is the physical bottleneck.

ni-ze

seems like muti-commit log dir make sence.

ni-ze · 2021-09-21T15:01:12Z

LGTM~, but only one disk is written at the same time, which does not take advantage of the increased write bandwidth after multi-disk deployment.

yes, it is. Appending messages write last MappedFile only, other files can not be written at all.

francisoliverlee · 2021-09-22T02:24:36Z

DiskCheckScheduledThread1

@francisoliverlee Thank you for the notice.
Error in DiskCheckScheduledThread1 is due to storePathCommitLog usage in "DefaultMessageStore.CleanCommitLogService#isSpaceFull". I fixed it and added test in last commit.
Sorry for missing that. I have checked all the usage of storePathCommitLog.

It seems this error still exists ，and maybe you discarded this method org.apache.rocketmq.common.UtilAll#getDiskPartitionSpaceUsedPercent, or should we check the existence of a directory at the start of the broker?

error stills.

when doing getDiskPartitionSpaceUsedPercent(), how about a WARNING for NO consume queue dir ?
@Jason918 @duhenglucky

broker/src/main/java/org/apache/rocketmq/broker/processor/SendMessageProcessor.java

store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java

store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java

… commitlog path in storePathCommitLog.

store/src/main/java/org/apache/rocketmq/store/MultiPathMappedFileQueue.java

lollipopjin · 2021-09-23T02:30:14Z

If we add a cloud disk to the running ecs, can the new cloud disk be recognized by updating broker.conf dynamically?

Jason918 · 2021-09-23T06:07:07Z

If we add a cloud disk to the running ecs, can the new cloud disk be recognized by updating broker.conf dynamically?

Yes, It should work. You can add path after storePathCommitLog.
But, one thing to be noticed, if your server is started with only one commit log path, you have to restart the server to change it to multi-path-commit-log mode. Because mappedFileQueue in class Commitlog is initialized during start up.
One way to avoid this restart is to add a MessageStoreConfig#MULTI_PATH_SPLITTER(default is ',' and can be changed by system property "rocketmq.broker.multiPathSplitter") after origin single storePathCommitLog, e.g. "~/store/commitlog,".

francisoliverlee

OK

cserwen · 2022-02-24T06:52:23Z

readOnlyCommitLogStorePaths: I have a question to ask. What is the meaning of this configuration, is it used for smooth upgrade? @Jason918

Jason918 · 2022-02-25T02:25:40Z

readOnlyCommitLogStorePaths: I have a question to ask. What is the meaning of this configuration, is it used for smooth upgrade? @Jason918

Yep, it can be used to decommission some broken disks.

RIP-7 Multiple Directories Storage Support

RIP-7 Multiple Directories Storage Suppor

a9a7dfa

duhenglucky mentioned this pull request Sep 16, 2021

[RIP-7] Multiple Directories Storage Support (#751) #3348

Closed

caigy requested changes Sep 16, 2021

View reviewed changes

broker/src/main/java/org/apache/rocketmq/broker/processor/SendMessageProcessor.java Outdated Show resolved Hide resolved

store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java Outdated Show resolved Hide resolved

Jiang Haiting added 2 commits September 16, 2021 10:55

fix test error

8e19069

1. fix initial value of physicRatio in org.apache.rocketmq.broker.pro…

9b0e31e

…cessor.SendMessageProcessor#diskUtil 2. use minPhysicsUsedRatio instead of max in org.apache.rocketmq.store.DefaultMessageStore#getRuntimeInfo

zongtanghu added this to the 4.9.2 milestone Sep 16, 2021

zongtanghu added the type/enhancement label Sep 16, 2021

zongtanghu requested review from duhenglucky and RongtongJin September 16, 2021 11:07

francisoliverlee self-requested a review September 16, 2021 15:25

francisoliverlee requested changes Sep 16, 2021

View reviewed changes

fix error in org.apache.rocketmq.store.DefaultMessageStore.CleanCommi…

466bb98

…tLogService#isSpaceFull

Jason918 requested a review from francisoliverlee September 18, 2021 01:21

francisoliverlee changed the title ~~RIP-7 Multiple Directories Storage Suppor~~ RIP-7 Multiple Directories Storage Support Sep 18, 2021

add path empty check to avoid error log in org.apache.rocketmq.store.…

255a202

…DefaultMessageStore#getRuntimeInfo

RongtongJin approved these changes Sep 20, 2021

View reviewed changes

duhenglucky reviewed Sep 20, 2021

View reviewed changes

store/src/main/java/org/apache/rocketmq/store/config/MessageStoreConfig.java Outdated Show resolved Hide resolved

change default value of org.apache.rocketmq.store.config.MessageStore…

f32406d

…Config#MULTI_PATH_SPLITTER to ',' and can be changed by System.getProperty("rocketmq.broker.multiPathSplitter")

ni-ze reviewed Sep 21, 2021

View reviewed changes

yuz10 reviewed Sep 22, 2021

View reviewed changes

broker/src/main/java/org/apache/rocketmq/broker/processor/SendMessageProcessor.java Outdated Show resolved Hide resolved

yuz10 reviewed Sep 22, 2021

View reviewed changes

store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java Outdated Show resolved Hide resolved

yuz10 reviewed Sep 22, 2021

View reviewed changes

store/src/main/java/org/apache/rocketmq/store/DefaultMessageStore.java Outdated Show resolved Hide resolved

yuz10 reviewed Sep 22, 2021

View reviewed changes

store/src/main/java/org/apache/rocketmq/store/MappedFileQueue.java Outdated Show resolved Hide resolved

vongosling added the progress/wip Work in progress. Accept or refused to be continue. label Sep 22, 2021

Jiang Haiting added 2 commits September 22, 2021 19:46

Fix bug for multi path file loading. Opt code checking for multi-path…

474c4eb

… commitlog path in storePathCommitLog.

fix compile error

7d4e5cd

yuz10 approved these changes Sep 22, 2021

View reviewed changes

lollipopjin reviewed Sep 23, 2021

View reviewed changes

store/src/main/java/org/apache/rocketmq/store/MultiPathMappedFileQueue.java Outdated Show resolved Hide resolved

fix a typo

14d50f9

francisoliverlee approved these changes Sep 25, 2021

View reviewed changes

francisoliverlee merged commit 4dbdbf0 into apache:develop Sep 25, 2021

This was referenced Nov 7, 2021

[ISSUE #3467] Fail fast on loading files with error mappedFileSize #3466

Merged

[RIP-7] Fix some issue and Optimization code for multiple_disk_dir #1144

Closed

This was referenced Feb 28, 2022

Wrong dir is created when using mult-dirs storage. #3900

Closed

[RIP-7] Support multi-directory storage in DLedger. #3914

Closed

[Feature] Support multi-directory storage openmessaging/dledger#118

Closed

GenerousMan pushed a commit to GenerousMan/rocketmq that referenced this pull request Aug 12, 2022

Merge pull request apache#3357 from Jason918/RIP-7

2422100

RIP-7 Multiple Directories Storage Support

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RIP-7 Multiple Directories Storage Support #3357

RIP-7 Multiple Directories Storage Support #3357

Jason918 commented Sep 15, 2021 •

edited

francisoliverlee left a comment •

edited

Jason918 commented Sep 17, 2021 •

edited

Jason918 commented Sep 17, 2021 •

edited

Jason918 commented Sep 18, 2021

RongtongJin left a comment •

edited

duhenglucky commented Sep 20, 2021 •

edited

Jason918 commented Sep 21, 2021 •

edited

Jason918 commented Sep 21, 2021

ni-ze left a comment

ni-ze commented Sep 21, 2021

francisoliverlee commented Sep 22, 2021 •

edited

lollipopjin commented Sep 23, 2021

Jason918 commented Sep 23, 2021

francisoliverlee left a comment

cserwen commented Feb 24, 2022

Jason918 commented Feb 25, 2022

RIP-7 Multiple Directories Storage Support #3357

RIP-7 Multiple Directories Storage Support #3357

Conversation

Jason918 commented Sep 15, 2021 • edited

What is the purpose of the change

Brief changelog

Verifying this change

francisoliverlee left a comment • edited

Choose a reason for hiding this comment

Jason918 commented Sep 17, 2021 • edited

Jason918 commented Sep 17, 2021 • edited

Jason918 commented Sep 18, 2021

RongtongJin left a comment • edited

Choose a reason for hiding this comment

duhenglucky commented Sep 20, 2021 • edited

Jason918 commented Sep 21, 2021 • edited

Jason918 commented Sep 21, 2021

ni-ze left a comment

Choose a reason for hiding this comment

ni-ze commented Sep 21, 2021

francisoliverlee commented Sep 22, 2021 • edited

lollipopjin commented Sep 23, 2021

Jason918 commented Sep 23, 2021

francisoliverlee left a comment

Choose a reason for hiding this comment

cserwen commented Feb 24, 2022

Jason918 commented Feb 25, 2022

Jason918 commented Sep 15, 2021 •

edited

francisoliverlee left a comment •

edited

Jason918 commented Sep 17, 2021 •

edited

Jason918 commented Sep 17, 2021 •

edited

RongtongJin left a comment •

edited

duhenglucky commented Sep 20, 2021 •

edited

Jason918 commented Sep 21, 2021 •

edited

francisoliverlee commented Sep 22, 2021 •

edited