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 2018/11/19 08:43:03 UTC

[GitHub] walking98 commented on a change in pull request #548: [ISSUE 544]Fixed Transactional message will be lost under extreme condition

walking98 commented on a change in pull request #548: [ISSUE 544]Fixed Transactional message will be lost under extreme condition
URL: https://github.com/apache/rocketmq/pull/548#discussion_r234528241
 
 

 ##########
 File path: broker/src/main/java/org/apache/rocketmq/broker/transaction/queue/TransactionalMessageServiceImpl.java
 ##########
 @@ -315,33 +315,26 @@ private PullResult fillOpRemoveMap(HashMap<Long, Long> removeMap,
      * @param removeMap Op message map to determine whether a half message was responded by producer.
      * @param doneOpOffset Op Message which has been checked.
      * @param msgExt Half message
-     * @param checkImmunityTime User defined time to avoid being detected early.
      * @return Return true if put success, otherwise return false.
      */
-    private boolean checkPrepareQueueOffset(HashMap<Long, Long> removeMap, List<Long> doneOpOffset, MessageExt msgExt,
-        long checkImmunityTime) {
-        if (System.currentTimeMillis() - msgExt.getBornTimestamp() < checkImmunityTime) {
-            String prepareQueueOffsetStr = msgExt.getUserProperty(MessageConst.PROPERTY_TRANSACTION_PREPARED_QUEUE_OFFSET);
-            if (null == prepareQueueOffsetStr) {
-                return putImmunityMsgBackToHalfQueue(msgExt);
+    private boolean checkPrepareQueueOffset(HashMap<Long, Long> removeMap, List<Long> doneOpOffset,
+        MessageExt msgExt) {
+        String prepareQueueOffsetStr = msgExt.getUserProperty(MessageConst.PROPERTY_TRANSACTION_PREPARED_QUEUE_OFFSET);
+        if (null == prepareQueueOffsetStr) {
+            return putImmunityMsgBackToHalfQueue(msgExt);
+        } else {
+            long prepareQueueOffset = getLong(prepareQueueOffsetStr);
+            if (-1 == prepareQueueOffset) {
+                return false;
             } else {
-                long prepareQueueOffset = getLong(prepareQueueOffsetStr);
-                if (-1 == prepareQueueOffset) {
-                    return false;
+                if (removeMap.containsKey(prepareQueueOffset)) {
+                    long tmpOpOffset = removeMap.remove(prepareQueueOffset);
+                    doneOpOffset.add(tmpOpOffset);
+                    return true;
                 } else {
-                    if (removeMap.containsKey(prepareQueueOffset)) {
-                        long tmpOpOffset = removeMap.remove(prepareQueueOffset);
-                        doneOpOffset.add(tmpOpOffset);
-                        return true;
-                    } else {
-                        return putImmunityMsgBackToHalfQueue(msgExt);
-                    }
+                    return putImmunityMsgBackToHalfQueue(msgExt);
                 }
-
             }
-
-        } else {
-            return true;
 
 Review comment:
   LGTM

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services