ethervoid commented on code in PR #3357:
URL: https://github.com/apache/kvrocks/pull/3357#discussion_r2753405658


##########
src/config/config.cc:
##########
@@ -206,6 +206,8 @@ Config::Config() {
       {"replication-no-slowdown", false, new 
YesNoField(&replication_no_slowdown, true)},
       {"replication-delay-bytes", false, new 
IntField(&max_replication_delay_bytes, 16 * 1024, 1, INT_MAX)},
       {"replication-delay-updates", false, new 
IntField(&max_replication_delay_updates, 16, 1, INT_MAX)},
+      {"max-replication-lag", false, new Int64Field(&max_replication_lag, 
100000000, 1, INT64_MAX)},

Review Comment:
   Good suggestion - I've updated the code to skip the lag check when 
max_replication_lag == 0, so users can disable this feature if needed. I've set 
it to 0 by default so this feature is disabled by default



##########
src/common/io_util.cc:
##########
@@ -468,6 +471,93 @@ Status SockSend(int fd, const std::string &data, 
[[maybe_unused]] bufferevent *b
 #endif
 }
 
+Status SockSendWithTimeout(int fd, const std::string &data, int timeout_ms) {
+  ssize_t n = 0;
+  auto start = std::chrono::steady_clock::now();
+
+  while (n < static_cast<ssize_t>(data.size())) {

Review Comment:
   I considered this but I don't think it's a great fit here. WriteImpl is a 
simple blocking loop that treats any -1 from write() as fatal. The timeout 
version needs fundamentally different behavior:
   - Tracking elapsed time with steady_clock
   - Polling for writability with poll()
   - Handling EAGAIN/EWOULDBLOCK gracefully (continue waiting vs error)
   - 
   so reusing it would either lose behavior or require a bigger refactor. I’d 
rather prefer keep the explicit loop for now. What do you think?



##########
src/common/io_util.cc:
##########
@@ -468,6 +471,93 @@ Status SockSend(int fd, const std::string &data, 
[[maybe_unused]] bufferevent *b
 #endif
 }
 
+Status SockSendWithTimeout(int fd, const std::string &data, int timeout_ms) {
+  ssize_t n = 0;
+  auto start = std::chrono::steady_clock::now();
+

Review Comment:
   Good catch. I've added a guard in both SockSendWithTimeout overloads to fall 
back to the blocking SockSend when timeout_ms <= 0. This handles it defensively 
even though the config currently enforces >= 1000.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to