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 2022/06/18 04:54:13 UTC

[GitHub] [rocketmq] azhsmesos opened a new pull request, #4477: implement transaction message escape

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

   docs: https://shimo.im/docs/rp3OVGvVLpFav8Am
   具体开发思路在 proposal 下
   


-- 
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 #4477: implement transaction message escape

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

   
   [![Coverage Status](https://coveralls.io/builds/50148606/badge)](https://coveralls.io/builds/50148606)
   
   Coverage increased (+0.06%) to 47.314% when pulling **f4b7c3c5617ba67191b88055ff8177e923523174 on azhsmesos:5.0.0-beta** into **cd24a244248db32819474328672868a7ff6ee1f2 on apache:5.0.0-beta**.
   


-- 
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] caigy commented on a diff in pull request #4477: implement transaction message escape

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


##########
broker/src/main/java/org/apache/rocketmq/broker/transaction/queue/TransactionalMessageServiceImpl.java:
##########
@@ -354,7 +358,13 @@ private PutMessageResult putBackToHalfQueueReturnResult(MessageExt messageExt) {
         PutMessageResult putMessageResult = null;
         try {
             MessageExtBrokerInner msgInner = transactionalMessageBridge.renewHalfMessageInner(messageExt);
-            putMessageResult = transactionalMessageBridge.putMessageReturnResult(msgInner);
+            if (this.transactionalMessageBridge.getBrokerController().isSpecialServiceRunning()
+                    && BrokerRole.SLAVE == this.transactionalMessageBridge.getBrokerController().getMessageStoreConfig()
+                    .getBrokerRole()) {
+                putMessageResult = transactionalMessageBridge.getBrokerController().getEscapeBridge().putMessage(msgInner);
+            } else {
+                putMessageResult = transactionalMessageBridge.putMessageReturnResult(msgInner);
+            }

Review Comment:
   If retry times of the half message reached maximum value, the broker where it escaped would not check it any more, and the broker it was born also had skipped it, that message would be lost.



##########
broker/src/main/java/org/apache/rocketmq/broker/transaction/queue/TransactionalMessageServiceImpl.java:
##########
@@ -354,7 +358,13 @@ private PutMessageResult putBackToHalfQueueReturnResult(MessageExt messageExt) {
         PutMessageResult putMessageResult = null;
         try {
             MessageExtBrokerInner msgInner = transactionalMessageBridge.renewHalfMessageInner(messageExt);
-            putMessageResult = transactionalMessageBridge.putMessageReturnResult(msgInner);
+            if (this.transactionalMessageBridge.getBrokerController().isSpecialServiceRunning()
+                    && BrokerRole.SLAVE == this.transactionalMessageBridge.getBrokerController().getMessageStoreConfig()
+                    .getBrokerRole()) {
+                putMessageResult = transactionalMessageBridge.getBrokerController().getEscapeBridge().putMessage(msgInner);
+            } else {
+                putMessageResult = transactionalMessageBridge.putMessageReturnResult(msgInner);
+            }

Review Comment:
   If retry times of the half message reached maximum value, the broker where it escaped would not check it any more, and the broker it was born also had skipped it, that message would be lost.



-- 
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] github-actions[bot] closed pull request #4477: implement transaction message escape

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4477: implement transaction message escape
URL: https://github.com/apache/rocketmq/pull/4477


-- 
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] github-actions[bot] commented on pull request #4477: implement transaction message escape

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4477:
URL: https://github.com/apache/rocketmq/pull/4477#issuecomment-1712350163

   This PR is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR.


-- 
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 #4477: implement transaction message escape

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

   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/4477?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 [#4477](https://codecov.io/gh/apache/rocketmq/pull/4477?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (721d0d0) into [5.0.0-beta](https://codecov.io/gh/apache/rocketmq/commit/cd24a244248db32819474328672868a7ff6ee1f2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cd24a24) will **decrease** coverage by `0.04%`.
   > The diff coverage is `15.78%`.
   
   ```diff
   @@               Coverage Diff                @@
   ##             5.0.0-beta    #4477      +/-   ##
   ================================================
   - Coverage         43.09%   43.04%   -0.05%     
   + Complexity         6051     6048       -3     
   ================================================
     Files               801      801              
     Lines             57086    57102      +16     
     Branches           7805     7814       +9     
   ================================================
   - Hits              24601    24581      -20     
   - Misses            29264    29299      +35     
   - Partials           3221     3222       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/4477?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ocketmq/broker/processor/SendMessageProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvcHJvY2Vzc29yL1NlbmRNZXNzYWdlUHJvY2Vzc29yLmphdmE=) | `45.23% <0.00%> (-0.41%)` | :arrow_down: |
   | [...ion/AbstractTransactionalMessageCheckListener.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdHJhbnNhY3Rpb24vQWJzdHJhY3RUcmFuc2FjdGlvbmFsTWVzc2FnZUNoZWNrTGlzdGVuZXIuamF2YQ==) | `64.44% <0.00%> (-6.29%)` | :arrow_down: |
   | [...ueue/DefaultTransactionalMessageCheckListener.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdHJhbnNhY3Rpb24vcXVldWUvRGVmYXVsdFRyYW5zYWN0aW9uYWxNZXNzYWdlQ2hlY2tMaXN0ZW5lci5qYXZh) | `77.14% <25.00%> (-7.24%)` | :arrow_down: |
   | [...saction/queue/TransactionalMessageServiceImpl.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvdHJhbnNhY3Rpb24vcXVldWUvVHJhbnNhY3Rpb25hbE1lc3NhZ2VTZXJ2aWNlSW1wbC5qYXZh) | `49.17% <28.57%> (-0.83%)` | :arrow_down: |
   | [.../broker/subscription/SubscriptionGroupManager.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvc3Vic2NyaXB0aW9uL1N1YnNjcmlwdGlvbkdyb3VwTWFuYWdlci5qYXZh) | `68.79% <0.00%> (-14.19%)` | :arrow_down: |
   | [...ache/rocketmq/broker/topic/TopicConfigManager.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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.03% <0.00%> (-3.06%)` | :arrow_down: |
   | [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `59.82% <0.00%> (-2.57%)` | :arrow_down: |
   | [...apache/rocketmq/store/ha/GroupTransferService.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL0dyb3VwVHJhbnNmZXJTZXJ2aWNlLmphdmE=) | `91.80% <0.00%> (-1.64%)` | :arrow_down: |
   | [...che/rocketmq/namesrv/kvconfig/KVConfigManager.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-bmFtZXNydi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbmFtZXNydi9rdmNvbmZpZy9LVkNvbmZpZ01hbmFnZXIuamF2YQ==) | `59.18% <0.00%> (-1.03%)` | :arrow_down: |
   | [.../apache/rocketmq/logging/inner/LoggingBuilder.java](https://codecov.io/gh/apache/rocketmq/pull/4477/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-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9Mb2dnaW5nQnVpbGRlci5qYXZh) | `63.76% <0.00%> (-0.95%)` | :arrow_down: |
   | ... and [12 more](https://codecov.io/gh/apache/rocketmq/pull/4477/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/4477?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/4477?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 [cd24a24...721d0d0](https://codecov.io/gh/apache/rocketmq/pull/4477?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] hzh0425 commented on a diff in pull request #4477: implement transaction message escape

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


##########
broker/src/test/java/org/apache/rocketmq/broker/transaction/queue/TransactionEscapeBridgeTest.java:
##########
@@ -0,0 +1,13 @@
+package org.apache.rocketmq.broker.transaction.queue;
+
+import org.junit.Ignore;
+
+/**
+ * @author zhaozhenhang <zh...@kuaishou.com>
+ * Created on 2022-06-18
+ */

Review Comment:
   Don't leave author info



-- 
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] azhsmesos commented on a diff in pull request #4477: implement transaction message escape

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


##########
broker/src/main/java/org/apache/rocketmq/broker/transaction/queue/TransactionalMessageServiceImpl.java:
##########
@@ -354,7 +358,13 @@ private PutMessageResult putBackToHalfQueueReturnResult(MessageExt messageExt) {
         PutMessageResult putMessageResult = null;
         try {
             MessageExtBrokerInner msgInner = transactionalMessageBridge.renewHalfMessageInner(messageExt);
-            putMessageResult = transactionalMessageBridge.putMessageReturnResult(msgInner);
+            if (this.transactionalMessageBridge.getBrokerController().isSpecialServiceRunning()
+                    && BrokerRole.SLAVE == this.transactionalMessageBridge.getBrokerController().getMessageStoreConfig()
+                    .getBrokerRole()) {
+                putMessageResult = transactionalMessageBridge.getBrokerController().getEscapeBridge().putMessage(msgInner);
+            } else {
+                putMessageResult = transactionalMessageBridge.putMessageReturnResult(msgInner);
+            }

Review Comment:
   The transactionmessageserviceimpl#putbacktohalfqueuereturnresult() method needs to rewrite the half message. Here,I think we have to rewrite the topic. I understand that this step should also be carried out by escaping to other masters



-- 
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] hzh0425 commented on a diff in pull request #4477: implement transaction message escape

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


##########
broker/src/test/java/org/apache/rocketmq/broker/transaction/queue/TransactionEscapeBridgeTest.java:
##########
@@ -0,0 +1,13 @@
+package org.apache.rocketmq.broker.transaction.queue;

Review Comment:
   need apache Licence if add new class



-- 
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] hzh0425 commented on pull request #4477: implement transaction message escape

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

   Please add some tests to prove correctness


-- 
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] azhsmesos commented on pull request #4477: implement transaction message escape

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

   > Please add some tests to prove correctness
   
   thanks！I will add it when i local test later


-- 
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] github-actions[bot] commented on pull request #4477: implement transaction message escape

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4477:
URL: https://github.com/apache/rocketmq/pull/4477#issuecomment-1714775295

   This PR was closed because it has been inactive for 3 days since being marked as stale.


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