You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2021/08/07 14:03:08 UTC

[GitHub] [rocketmq] makabakaboom opened a new pull request #3232: [ISSUE #3231]add topic expire time policy

makabakaboom opened a new pull request #3232:
URL: https://github.com/apache/rocketmq/pull/3232

**Make sure set the target branch to `develop`**

## What is the purpose of the change

#3231

## Brief changelog

#3231

## Verifying this change

#3231

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`.

- [x] Make sure there is a [Github issue](https://github.com/apache/rocketmq/issues) 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.
- [x] 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.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] 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](https://github.com/apache/rocketmq/tree/master/test).
- [x] 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](http://www.apache.org/licenses/#clas).

--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] odbozhou commented on pull request #3232: [ISSUE #3231]add topic expire time policy

Posted by GitBox <gi...@apache.org>.

odbozhou commented on pull request #3232:
URL: https://github.com/apache/rocketmq/pull/3232#issuecomment-1016120093


   Please resolve the branch conflict


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] makabakaboom commented on a change in pull request #3232: [ISSUE #3231]add topic expire time policy

Posted by GitBox <gi...@apache.org>.

makabakaboom commented on a change in pull request #3232:
URL: https://github.com/apache/rocketmq/pull/3232#discussion_r686447791



##########
File path: common/src/main/java/org/apache/rocketmq/common/TopicConfig.java
##########
@@ -29,6 +29,7 @@
     private TopicFilterType topicFilterType = TopicFilterType.SINGLE_TAG;
     private int topicSysFlag = 0;
     private boolean order = false;
+    private Integer expirationTime;
 

Review comment:
       nice  suggestion




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] codecov-commenter commented on pull request #3232: [ISSUE #3231]add topic expire time policy

Posted by GitBox <gi...@apache.org>.

codecov-commenter commented on pull request #3232:
URL: https://github.com/apache/rocketmq/pull/3232#issuecomment-894662062


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3232?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3232](https://codecov.io/gh/apache/rocketmq/pull/3232?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9c24c3a) into [develop](https://codecov.io/gh/apache/rocketmq/commit/44bdeedad2d0e28717f5012d375d12af76305bb5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (44bdeed) will **decrease** coverage by `0.01%`.
   > The diff coverage is `35.48%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3232/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3232?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3232      +/-   ##
   =============================================
   - Coverage      47.95%   47.93%   -0.02%     
   - Complexity      4565     4568       +3     
   =============================================
     Files            552      553       +1     
     Lines          36521    36569      +48     
     Branches        4818     4822       +4     
   =============================================
   + Hits           17514    17531      +17     
   - Misses         16795    16815      +20     
   - Partials        2212     2223      +11     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3232?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...mmon/protocol/header/CreateTopicRequestHeader.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vcHJvdG9jb2wvaGVhZGVyL0NyZWF0ZVRvcGljUmVxdWVzdEhlYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...n/java/org/apache/rocketmq/store/MessageStore.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL01lc3NhZ2VTdG9yZS5qYXZh) | `0.00% <ø> (ø)` | |
   | [...apache/rocketmq/store/config/TopicStorePolicy.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2NvbmZpZy9Ub3BpY1N0b3JlUG9saWN5LmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...n/java/org/apache/rocketmq/common/TopicConfig.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vVG9waWNDb25maWcuamF2YQ==) | `39.79% <10.00%> (-3.39%)` | :arrow_down: |
   | [...n/java/org/apache/rocketmq/store/ConsumeQueue.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0NvbnN1bWVRdWV1ZS5qYXZh) | `63.75% <25.00%> (-0.50%)` | :arrow_down: |
   | [...ocketmq/broker/processor/AdminBrokerProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL0FkbWluQnJva2VyUHJvY2Vzc29yLmphdmE=) | `8.08% <33.33%> (+0.16%)` | :arrow_up: |
   | [...org/apache/rocketmq/store/DefaultMessageStore.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0RlZmF1bHRNZXNzYWdlU3RvcmUuamF2YQ==) | `55.47% <50.00%> (-0.07%)` | :arrow_down: |
   | [...ache/rocketmq/broker/topic/TopicConfigManager.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdG9waWMvVG9waWNDb25maWdNYW5hZ2VyLmphdmE=) | `59.46% <80.00%> (+0.39%)` | :arrow_up: |
   | [...a/org/apache/rocketmq/broker/BrokerController.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvQnJva2VyQ29udHJvbGxlci5qYXZh) | `45.00% <100.00%> (+0.09%)` | :arrow_up: |
   | [...rg/apache/rocketmq/common/stats/StatsSnapshot.java](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vc3RhdHMvU3RhdHNTbmFwc2hvdC5qYXZh) | `84.61% <0.00%> (-15.39%)` | :arrow_down: |
   | ... and [17 more](https://codecov.io/gh/apache/rocketmq/pull/3232/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3232?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3232?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [44bdeed...9c24c3a](https://codecov.io/gh/apache/rocketmq/pull/3232?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] odbozhou commented on a change in pull request #3232: [ISSUE #3231]add topic expire time policy

Posted by GitBox <gi...@apache.org>.

odbozhou commented on a change in pull request #3232:
URL: https://github.com/apache/rocketmq/pull/3232#discussion_r787377174



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
##########
@@ -267,9 +268,16 @@ private synchronized RemotingCommand updateAndCreateTopic(ChannelHandlerContext
         topicConfig.setTopicFilterType(requestHeader.getTopicFilterTypeEnum());
         topicConfig.setPerm(requestHeader.getPerm());
         topicConfig.setTopicSysFlag(requestHeader.getTopicSysFlag() == null ? 0 : requestHeader.getTopicSysFlag());
+        topicConfig.setExpirationTime(requestHeader.getExpireTime());

Review comment:
       Should topic expireTime limit the maximum time? If expireTime is greater than the maximum time for commitlog storage, expireTime is meaningless?

##########
File path: store/src/main/java/org/apache/rocketmq/store/ConsumeQueue.java
##########
@@ -330,6 +336,17 @@ public boolean flush(final int flushLeastPages) {
         return result;
     }
 
+    public int deleteExpiredFile(

Review comment:
       Does the expired topic only delete the consumeQueue, and the commitLog still exists?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls commented on pull request #3232: [ISSUE #3231]add topic expire time policy

Posted by GitBox <gi...@apache.org>.

coveralls commented on pull request #3232:
URL: https://github.com/apache/rocketmq/pull/3232#issuecomment-894662036


   
   [![Coverage Status](https://coveralls.io/builds/41978315/badge)](https://coveralls.io/builds/41978315)
   
   Coverage increased (+0.07%) to 54.158% when pulling **9c24c3ae7b8aa44c2b23416a17057739fce41543 on makabakaboom:topic-expire** into **44bdeedad2d0e28717f5012d375d12af76305bb5 on apache:develop**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] francisoliverlee commented on a change in pull request #3232: [ISSUE #3231]add topic expire time policy

Posted by GitBox <gi...@apache.org>.

francisoliverlee commented on a change in pull request #3232:
URL: https://github.com/apache/rocketmq/pull/3232#discussion_r685650699



##########
File path: common/src/main/java/org/apache/rocketmq/common/TopicConfig.java
##########
@@ -29,6 +29,7 @@
     private TopicFilterType topicFilterType = TopicFilterType.SINGLE_TAG;
     private int topicSysFlag = 0;
     private boolean order = false;
+    private Integer expirationTime;
 

Review comment:
       “expirationTime” get me confused, expirationTimeInMs can be better




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org