You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by GitBox <gi...@apache.org> on 2022/11/03 02:19:49 UTC

[GitHub] [rocketmq] lizhimins opened a new pull request, #5457: [RIP-48] Support reset offset in server-side for pop message

lizhimins opened a new pull request, #5457:
URL: https://github.com/apache/rocketmq/pull/5457

   [RIP-48] Support reset offset in server-side for pop message


-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] lizhanhui commented on a diff in pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
lizhanhui commented on code in PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#discussion_r1015014999


##########
broker/src/main/java/org/apache/rocketmq/broker/offset/ConsumerOrderInfoManager.java:
##########
@@ -149,6 +149,13 @@ public boolean checkBlock(String topic, String group, int queueId, long invisibl
         return orderInfo.needBlock(invisibleTime);
     }
 
+    public void clearBlock(String topic, String group, int queueId) {
+        table.computeIfPresent(buildKey(topic, group), (key, val) -> {

Review Comment:
   @RongtongJin How can computeIfAbsent helps computeIfPresent?



-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] lizhanhui commented on a diff in pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
lizhanhui commented on code in PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#discussion_r1015014999


##########
broker/src/main/java/org/apache/rocketmq/broker/offset/ConsumerOrderInfoManager.java:
##########
@@ -149,6 +149,13 @@ public boolean checkBlock(String topic, String group, int queueId, long invisibl
         return orderInfo.needBlock(invisibleTime);
     }
 
+    public void clearBlock(String topic, String group, int queueId) {
+        table.computeIfPresent(buildKey(topic, group), (key, val) -> {

Review Comment:
   @RongtongJin How can computeIfAbsent help computeIfPresent?



-- 
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: commits-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 #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#issuecomment-1301583744

   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/5457?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 [#5457](https://codecov.io/gh/apache/rocketmq/pull/5457?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (910fe16) into [develop](https://codecov.io/gh/apache/rocketmq/commit/4ba8aa238d8cd7104cdc1119d0f35514fb47cfc2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ba8aa2) will **decrease** coverage by `0.00%`.
   > The diff coverage is `24.13%`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #5457      +/-   ##
   =============================================
   - Coverage      43.17%   43.17%   -0.01%     
   - Complexity      8046     8071      +25     
   =============================================
     Files           1020     1025       +5     
     Lines          71892    72207     +315     
     Branches        9519     9560      +41     
   =============================================
   + Hits           31039    31172     +133     
   - Misses         36946    37096     +150     
   - Partials        3907     3939      +32     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/5457?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...he/rocketmq/test/client/rmq/RMQNormalConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC9jbGllbnQvcm1xL1JNUU5vcm1hbENvbnN1bWVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../apache/rocketmq/test/client/rmq/RMQPopClient.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC9jbGllbnQvcm1xL1JNUVBvcENsaWVudC5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...pache/rocketmq/test/client/rmq/RMQPopConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC9jbGllbnQvcm1xL1JNUVBvcENvbnN1bWVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../apache/rocketmq/test/factory/ConsumerFactory.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC9mYWN0b3J5L0NvbnN1bWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...rg/apache/rocketmq/test/util/MQAdminTestUtils.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW5UZXN0VXRpbHMuamF2YQ==) | `0.00% <ø> (ø)` | |
   | [...org/apache/rocketmq/tools/command/CommandUtil.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-dG9vbHMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Rvb2xzL2NvbW1hbmQvQ29tbWFuZFV0aWwuamF2YQ==) | `31.16% <0.00%> (-3.12%)` | :arrow_down: |
   | [...cketmq/broker/offset/ConsumerOrderInfoManager.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvb2Zmc2V0L0NvbnN1bWVyT3JkZXJJbmZvTWFuYWdlci5qYXZh) | `76.15% <33.33%> (+3.42%)` | :arrow_up: |
   | [...rocketmq/broker/processor/PopMessageProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL1BvcE1lc3NhZ2VQcm9jZXNzb3IuamF2YQ==) | `37.39% <52.63%> (-0.11%)` | :arrow_down: |
   | [...apache/rocketmq/acl/plain/PlainAccessResource.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-YWNsL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9hY2wvcGxhaW4vUGxhaW5BY2Nlc3NSZXNvdXJjZS5qYXZh) | `57.57% <75.00%> (-0.08%)` | :arrow_down: |
   | [...apache/rocketmq/broker/longpolling/PopRequest.java](https://codecov.io/gh/apache/rocketmq/pull/5457/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbG9uZ3BvbGxpbmcvUG9wUmVxdWVzdC5qYXZh) | `31.03% <0.00%> (-13.80%)` | :arrow_down: |
   | ... and [36 more](https://codecov.io/gh/apache/rocketmq/pull/5457/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) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] RongtongJin commented on a diff in pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
RongtongJin commented on code in PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#discussion_r1015023686


##########
broker/src/main/java/org/apache/rocketmq/broker/offset/ConsumerOrderInfoManager.java:
##########
@@ -149,6 +149,13 @@ public boolean checkBlock(String topic, String group, int queueId, long invisibl
         return orderInfo.needBlock(invisibleTime);
     }
 
+    public void clearBlock(String topic, String group, int queueId) {
+        table.computeIfPresent(buildKey(topic, group), (key, val) -> {

Review Comment:
   Sorry,I misread it



-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] lizhanhui commented on pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
lizhanhui commented on PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#issuecomment-1305050605

   Rebase to make CI pass


-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] lizhanhui merged pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
lizhanhui merged PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457


-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] lizhimins commented on a diff in pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
lizhimins commented on code in PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#discussion_r1012444587


##########
test/src/main/java/org/apache/rocketmq/test/client/rmq/RMQNormalConsumer.java:
##########
@@ -25,26 +25,31 @@
 import org.apache.rocketmq.test.util.RandomUtil;
 
 public class RMQNormalConsumer extends AbstractMQConsumer {
-    private static Logger logger = Logger.getLogger(RMQNormalConsumer.class);
+
+    private static final Logger LOGGER = Logger.getLogger(RMQNormalConsumer.class);

Review Comment:
   checkstyle



-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] lizhimins commented on a diff in pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
lizhimins commented on code in PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#discussion_r1012444587


##########
test/src/main/java/org/apache/rocketmq/test/client/rmq/RMQNormalConsumer.java:
##########
@@ -25,26 +25,31 @@
 import org.apache.rocketmq.test.util.RandomUtil;
 
 public class RMQNormalConsumer extends AbstractMQConsumer {
-    private static Logger logger = Logger.getLogger(RMQNormalConsumer.class);
+
+    private static final Logger LOGGER = Logger.getLogger(RMQNormalConsumer.class);

Review Comment:
   checkstyle limit



-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] lizhimins commented on a diff in pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
lizhimins commented on code in PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#discussion_r1012445023


##########
test/src/main/java/org/apache/rocketmq/test/client/rmq/RMQPopConsumer.java:
##########
@@ -17,17 +17,84 @@
 
 package org.apache.rocketmq.test.client.rmq;
 
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeoutException;
+import org.apache.log4j.Logger;
+import org.apache.rocketmq.client.consumer.AckResult;
+import org.apache.rocketmq.client.consumer.PopResult;
+import org.apache.rocketmq.client.exception.MQBrokerException;
+import org.apache.rocketmq.client.exception.MQClientException;
+import org.apache.rocketmq.common.constant.ConsumeInitMode;
+import org.apache.rocketmq.common.filter.ExpressionType;
+import org.apache.rocketmq.common.message.MessageQueue;
+import org.apache.rocketmq.remoting.exception.RemotingException;
+import org.apache.rocketmq.test.factory.ConsumerFactory;
 import org.apache.rocketmq.test.listener.AbstractListener;
 
 public class RMQPopConsumer extends RMQNormalConsumer {
+
+    private static final Logger log = Logger.getLogger(RMQPopConsumer.class);
+
+    public static final long POP_TIMEOUT = 3000;
+    public static final long DEFAULT_INVISIBLE_TIME = 30000;
+
+    private RMQPopClient client;
+
+    private int maxNum = 16;
+
     public RMQPopConsumer(String nsAddr, String topic, String subExpression,
-        String consumerGroup, AbstractListener listner) {

Review Comment:
   fix spelling mistakes



-- 
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: commits-unsubscribe@rocketmq.apache.org

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


[GitHub] [rocketmq] RongtongJin commented on a diff in pull request #5457: [RIP-48] Support reset offset in server-side for pop message

Posted by GitBox <gi...@apache.org>.
RongtongJin commented on code in PR #5457:
URL: https://github.com/apache/rocketmq/pull/5457#discussion_r1012529581


##########
broker/src/main/java/org/apache/rocketmq/broker/offset/ConsumerOrderInfoManager.java:
##########
@@ -149,6 +149,13 @@ public boolean checkBlock(String topic, String group, int queueId, long invisibl
         return orderInfo.needBlock(invisibleTime);
     }
 
+    public void clearBlock(String topic, String group, int queueId) {
+        table.computeIfPresent(buildKey(topic, group), (key, val) -> {

Review Comment:
   How about using org.apache.rocketmq.common.utils.ConcurrentHashMapUtils#computeIfAbsent?



-- 
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: commits-unsubscribe@rocketmq.apache.org

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