(kvrocks) branch unstable updated: fix(replication): send ack on ping even if no data written (#3220)

[twice]

This is an automated email from the ASF dual-hosted git repository.

twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git


The following commit(s) were added to refs/heads/unstable by this push:
     new 0bd1b081a fix(replication): send ack on ping even if no data written 
(#3220)
0bd1b081a is described below

commit 0bd1b081a54b6db442499c328f14cca70c503a0e
Author: Zhixin Wen <z...@roblox.com>
AuthorDate: Tue Oct 7 21:28:35 2025 -0700

    fix(replication): send ack on ping even if no data written (#3220)
    
    In the old code, we only ack when there is data written and
    `sendReplConfAck` would only send ack if not ack was sent in the last 1
    sec. This works fine for continuous traffic. However, if write stops and
    the last write did not trigger an ack due to 1s rate limit, no more ack
    would be sent.
    
    If user runs `INFO replication` on master, the user would see a
    replication lag that never get catch up. In this PR we send ack back
    whenever replication sees ping, because ping is only sent when master
    has no data, this should not add extra load.
    
    ---------
    
    Co-authored-by: Twice <tw...@apache.org>
---
 src/cluster/replication.cc | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/cluster/replication.cc b/src/cluster/replication.cc
index a54aaa600..c66406ddb 100644
--- a/src/cluster/replication.cc
+++ b/src/cluster/replication.cc
@@ -703,9 +703,10 @@ ReplicationThread::CBState 
ReplicationThread::incrementBatchLoopCB(bufferevent *
         if (bulk_string == "ping") {
           // master would send the ping heartbeat packet to check whether the 
slave was alive or not,
           // don't write ping to db here.
-          if (data_written) {
-            sendReplConfAck(bev, force_ack);
-          }
+          // We should not check data_written here because sendReplConfAck 
only send ack if it has been 1s from last ack
+          // when force_ack is false. As a result, if the last write did not 
trigger ack, the replication would not send
+          // ack forever and the info command on master would report incorrect 
lag.
+          sendReplConfAck(bev, force_ack);
           return CBState::AGAIN;
         }