[GitHub] [rocketmq] lizhiboo commented on a change in pull request #3838: [ISSUE #3804]Commit consumption offset with specific MessageQueue.

[GitBox <gi...@apache.org>]

lizhiboo commented on a change in pull request #3838:
URL: https://github.com/apache/rocketmq/pull/3838#discussion_r803630810



##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/consumer/DefaultLitePullConsumerImpl.java
##########
@@ -645,21 +645,39 @@ private void removePullTask(final String topic) {
     }
 
     public synchronized void commitAll() {
-        try {
-            for (MessageQueue messageQueue : assignedMessageQueue.messageQueues()) {
-                long consumerOffset = assignedMessageQueue.getConsumerOffset(messageQueue);
-                if (consumerOffset != -1) {
-                    ProcessQueue processQueue = assignedMessageQueue.getProcessQueue(messageQueue);
-                    if (processQueue != null && !processQueue.isDropped()) {
-                        updateConsumeOffset(messageQueue, consumerOffset);
-                    }
-                }
+        for (MessageQueue messageQueue : assignedMessageQueue.messageQueues()) {
+            try {
+                commit(messageQueue);
+            } catch (MQClientException e) {
+                log.error("commit messageQueue [" + messageQueue+ "] consume offset error.", e);
             }
-            if (defaultLitePullConsumer.getMessageModel() == MessageModel.BROADCASTING) {
-                offsetStore.persistAll(assignedMessageQueue.messageQueues());
+        }
+    }
+
+    public synchronized void commit(final HashSet<MessageQueue> messageQueues, boolean persist) throws MQClientException {
+        if (messageQueues == null || messageQueues.size() == 0) {
+            return;
+        }
+
+        for (MessageQueue messageQueue : messageQueues) {
+            commit(messageQueue);
+        }
+
+        if (persist) {
+            this.offsetStore.persistAll(messageQueues);
+        }
+    }
+
+    private synchronized void commit(MessageQueue messageQueue) throws MQClientException {
+        long consumerOffset = assignedMessageQueue.getConsumerOffset(messageQueue);
+
+        if (consumerOffset != -1) {
+            ProcessQueue processQueue = assignedMessageQueue.getProcessQueue(messageQueue);
+            if (processQueue != null && !processQueue.isDropped()) {
+                updateConsumeOffset(messageQueue, consumerOffset);
             }
-        } catch (Exception e) {
-            log.error("An error occurred when update consume offset Automatically.");
+        } else {
+            throw new MQClientException("messageQueue ["+ messageQueue +"] does not exist in assignedMessageQueue, please assign it first.", null);

Review comment:
       if consumerOffset equals -1, commit will throw execption. IMO, consumerOffset equlas -1 means this consumequeue is empty, commit offset can be ignored.




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