[RIP-43] Support Timing Messages with Arbitrary Time Delay #4642

GenerousMan · 2022-07-20T09:42:06Z

Make sure set the target branch to develop

What is the purpose of the change

Currently, RocketMQ's delay message feature only supports delayed delivery for specific time levels. Such delay message feature(only support specific levels of delay time) is no longer enough to support the flexible usage of rocketmq. Therefore, we need a delay message feature that supports arbitrary delay time.

I have written my proposal on the links below：
Google Doc: https://docs.google.com/document/d/1D6XWwY39p531c2aVi5HQll9iwzTUNT1haUFHqMoRkT0/edit?usp=sharing
Shimo:https://shimo.im/docs/gXqme9PKKpIeD7qo/

To optimize the commit history, this is a new PR.
The old PR is :#4558

Brief changelog

Add classes to support the feature, such as TimerMessageStore, TimerWheel, TimerLog, and Slot , etc.
Modify the logic of CommitLog.AsyncPutMessage(). Add two judgement(isRolledTimerMessage(msg) / checkIfTimerMessage(msg)) for timing messages. If the message is timing message, its topic and related properties will be converted.
Modify the logic of BrokerController.start()/shutdown()/initialize(), etc. Add the TimerMessageStore into these processes.
Add some functions in related files(DefaultMessageStore, CommitLog, MappedFileQueue, MessageConst, etc).
Add unit tests for the feature.

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.

coveralls · 2022-07-20T10:38:46Z

Coverage increased (+0.5%) to 48.994% when pulling d538133 on GenerousMan:RIP-43 into 8ccffa7 on apache:develop.

RongtongJin

There are benchmarks in the current example module, but I think it would be better to add the most straightforward example to tell users how to use timing messages with arbitrary time delay. In addition, adding relevant documents is also a good choice
It would be better to add an IT test.
It is necessary to replace the origin delay level message with new timer message in the POP consume. It can also be completed in the subsequent pull requests.
The current implementation needs to adapt to the slaveActingMaster mode. If no one is willing to do it, I am willing to do this in the subsequent pull requests later.

RongtongJin · 2022-07-20T11:51:18Z

example/pom.xml

+        <dependency>
+            <groupId>commons-cli</groupId>
+            <artifactId>commons-cli</artifactId>
+            <version>1.4</version>


Delete this line, we can use the parent pom file dependency.

RongtongJin · 2022-07-20T11:55:16Z

store/pom.xml

+        <dependency>
+            <groupId>com.conversantmedia</groupId>
+            <artifactId>disruptor</artifactId>
+            <version>1.2.10</version>


It would be better if we can put the version to the parent pom file.

RongtongJin · 2022-07-20T11:58:10Z

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

    public static final String ONS_HTTP_PROXY_GROUP = "CID_ONS-HTTP-PROXY";
    public static final String CID_ONSAPI_PERMISSION_GROUP = "CID_ONSAPI_PERMISSION";
    public static final String CID_ONSAPI_OWNER_GROUP = "CID_ONSAPI_OWNER";
+    public static final String SYSTEM_TOPIC_PREFIX = "RMQ_SYS_";


There is an same constant in the TopicValidator class (org.apache.rocketmq.common.topic.TopicValidator#SYSTEM_TOPIC_PREFIX). I don't think it's necessary to re create one here

fuyou001 · 2022-07-21T02:16:13Z

Conflicting files @GenerousMan

rushsky518 · 2022-07-21T03:32:08Z

broker/src/main/java/org/apache/rocketmq/broker/util/HookUtils.java

+        long deliverMs;
+        try {
+            if (msg.getProperty(MessageConst.PROPERTY_TIMER_DELAY_SEC) != null) {
+                deliverMs = System.currentTimeMillis() + Integer.parseInt(msg.getProperty(MessageConst.PROPERTY_TIMER_DELAY_SEC)) * 1000;


when set setDelayTimeSec(24 * 3600 * 3), integer overflows

thx, it has been resolved.

# Conflicts: # broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java # store/src/main/java/org/apache/rocketmq/store/config/MessageStoreConfig.java

RongtongJin · 2022-07-21T08:00:39Z

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

+    TimerMessageStore getTimerMessageStore();
+
+    void setTimerMessageStore(TimerMessageStore timerMessageStore);


It would be better. to add some comments for the interface.

AuroraTwinkle · 2024-07-27T10:12:47Z

store/src/main/java/org/apache/rocketmq/store/timer/TimerWheel.java

+    public long checkPhyPos(long timeStartMs, long maxOffset) {
+        long minFirst = Long.MAX_VALUE;
+        int firstSlotIndex = getSlotIndex(timeStartMs);
+        for (int i = 0; i < slotsTotal * 2; i++) {


I noticed that slotsTotal * 2 appears multiple times. Why do we need to multiply it by 2? I don't understand, could you please help me? Thanks. @GenerousMan

[RIP-43] Support Timing Messages with Arbitrary Time Delay

ccead04

lizhanhui approved these changes Jul 20, 2022

View reviewed changes

RongtongJin reviewed Jul 20, 2022

View reviewed changes

rushsky518 reviewed Jul 21, 2022

View reviewed changes

fuyou001 and others added 3 commits July 21, 2022 15:19

refactor #RIP-43

8f99df3

Merge remote-tracking branch 'open-core/develop' into RIP-43

fcdbc30

# Conflicts: # broker/src/main/java/org/apache/rocketmq/broker/BrokerController.java # store/src/main/java/org/apache/rocketmq/store/config/MessageStoreConfig.java

[RIP-43] Resolve some review problems.

d538133

RongtongJin approved these changes Jul 21, 2022

View reviewed changes

duhenglucky approved these changes Jul 21, 2022

View reviewed changes

duhenglucky merged commit 8336b49 into apache:develop Jul 21, 2022

duhenglucky added this to the 5.0.0 milestone Jul 21, 2022

AuroraTwinkle reviewed Jul 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RIP-43] Support Timing Messages with Arbitrary Time Delay #4642

[RIP-43] Support Timing Messages with Arbitrary Time Delay #4642

Uh oh!

GenerousMan commented Jul 20, 2022

Uh oh!

coveralls commented Jul 20, 2022 •

edited

Loading

Uh oh!

RongtongJin left a comment •

edited

Loading

Uh oh!

RongtongJin Jul 20, 2022

Uh oh!

RongtongJin Jul 20, 2022

Uh oh!

RongtongJin Jul 20, 2022

Uh oh!

fuyou001 commented Jul 21, 2022

Uh oh!

rushsky518 Jul 21, 2022

Uh oh!

GenerousMan Jul 21, 2022

Uh oh!

RongtongJin Jul 21, 2022

Uh oh!

AuroraTwinkle Jul 27, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

		TimerMessageStore getTimerMessageStore();

		void setTimerMessageStore(TimerMessageStore timerMessageStore);

[RIP-43] Support Timing Messages with Arbitrary Time Delay #4642

[RIP-43] Support Timing Messages with Arbitrary Time Delay #4642

Uh oh!

Conversation

GenerousMan commented Jul 20, 2022

What is the purpose of the change

Brief changelog

Uh oh!

coveralls commented Jul 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RongtongJin left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RongtongJin Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

RongtongJin Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

RongtongJin Jul 20, 2022

Choose a reason for hiding this comment

Uh oh!

fuyou001 commented Jul 21, 2022

Uh oh!

rushsky518 Jul 21, 2022

Choose a reason for hiding this comment

Uh oh!

GenerousMan Jul 21, 2022

Choose a reason for hiding this comment

Uh oh!

RongtongJin Jul 21, 2022

Choose a reason for hiding this comment

Uh oh!

AuroraTwinkle Jul 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

coveralls commented Jul 20, 2022 •

edited

Loading

RongtongJin left a comment •

edited

Loading

AuroraTwinkle Jul 27, 2024 •

edited

Loading