[GitHub] [rocketmq] GenerousMan commented on a diff in pull request #6729: [ISSUE #6728] Compute the confirmOffset without considering new connections

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

GenerousMan commented on code in PR #6729:
URL: https://github.com/apache/rocketmq/pull/6729#discussion_r1189421737


##########
store/src/main/java/org/apache/rocketmq/store/ha/autoswitch/AutoSwitchHAService.java:
##########
@@ -421,14 +421,21 @@ public long computeConfirmOffset() {
         for (Long syncId : currentSyncStateSet) {
             if (!idList.contains(syncId) && this.brokerControllerId != null && !Objects.equals(syncId, this.brokerControllerId)) {
                 LOGGER.warn("Slave {} is still in syncStateSet, but has lost its connection. So new offset can't be compute.", syncId);
-                return this.defaultMessageStore.getConfirmOffset();
+                // Without check and re-compute, return the confirmOffset's value directly.
+                return this.defaultMessageStore.getConfirmOffsetDirectly();
             }
         }
 
         for (HAConnection connection : this.connectionList) {
             final Long slaveId = ((AutoSwitchHAConnection) connection).getSlaveId();
             if (currentSyncStateSet.contains(slaveId)) {
-                newConfirmOffset = Math.min(newConfirmOffset, connection.getSlaveAckOffset());
+                long slaveAckOffset = connection.getSlaveAckOffset();
+                if (slaveAckOffset <= 0) {
+                    // Slave's connection is just inited, the ack hasn't been calculated.
+                    // So skip this ackOffset.
+                    continue;
+                }

Review Comment:
   Yes, I have optimized this logic as you said.



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