[GitHub] [rocketmq] lizhimins commented on a diff in pull request #5995: [RIP-46] [ISSUE #5994] add pop and timer metrics

["lizhimins (via GitHub)" <gi...@apache.org>]

lizhimins commented on code in PR #5995:
URL: https://github.com/apache/rocketmq/pull/5995#discussion_r1097121536


##########
broker/src/test/java/org/apache/rocketmq/broker/processor/PopReviveServiceTest.java:
##########
@@ -249,22 +249,24 @@ public static MessageExtBrokerInner buildCkMsg(PopCheckPoint ck) {
         return msgInner;
     }
 
-    public static MessageExtBrokerInner buildAckMsg(AckMsg ackMsg, long deliverMs, long reviveOffset, long deliverTime) {
+    public static MessageExtBrokerInner buildAckMsg(AckMsg ackMsg, long deliverMs, long reviveOffset,
+        long deliverTime) {
         MessageExtBrokerInner messageExtBrokerInner = buildAckInnerMessage(
-                REVIVE_TOPIC,
-                ackMsg,
-                REVIVE_QUEUE_ID,
-                STORE_HOST,
-                deliverMs,
-                PopMessageProcessor.genAckUniqueId(ackMsg)

Review Comment:
   code style is strange..



##########
broker/src/main/java/org/apache/rocketmq/broker/processor/PopReviveService.java:
##########
@@ -66,6 +67,7 @@ public class PopReviveService extends ServiceThread {
     private int queueId;
     private BrokerController brokerController;
     private String reviveTopic;
+    private volatile long currentReviveMessageTimestamp = -1;

Review Comment:
   Need to add the volatile modifier here?



##########
store/src/main/java/org/apache/rocketmq/store/timer/TimerMessageStore.java:
##########
@@ -1106,6 +1106,13 @@ private MessageExtBrokerInner convertMessage(MessageExt msgExt, boolean needRoll
         return msgInner;
     }
 
+    private String getRealTopic(MessageExt msgExt) {
+        if (msgExt == null) {
+            return null;
+        }
+        return msgExt.getProperty(MessageConst.PROPERTY_REAL_TOPIC);

Review Comment:
   Here may return null



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