(kudu) branch master updated: [tserver] disable KUDU-3486 behavior by default

[al...@apache.org]

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

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 0ddaac556 [tserver] disable KUDU-3486 behavior by default
0ddaac556 is described below

commit 0ddaac556f7bc7aeb47db740300921d10eabd856
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Jan 14 12:14:58 2025 -0800

    [tserver] disable KUDU-3486 behavior by default
    
    This is a quick-and-dirty fix to mitigate KUDU-3638.
    
    This patch isn't focusing on properly addressing the issues that
    KUDU-3486 has introduced apart from fixing the obvious bug of missing
    updates of the Heartbeater::Thread::last_tombstoned_report_time_ field.
    Also, with this patch, the functionality introduced with KUDU-3486
    is now disabled by default.  To re-enable it back, customize the setting
    for the --tserver_send_tombstoned_tablets_report_inteval_secs flag,
    if needed.
    
    Properly implementing the functionality that KUDU-3486 attempted to add
    would be a much more involved patch because there are several items
    to address from both the design and implementation standpoints.
    
    Change-Id: I8e32aafab99c74f0ead3ba65aea58ce91d40297c
    Reviewed-on: http://gerrit.cloudera.org:8080/22341
    Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
    Tested-by: Alexey Serbin <al...@apache.org>
---
 src/kudu/tserver/heartbeater.cc | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/kudu/tserver/heartbeater.cc b/src/kudu/tserver/heartbeater.cc
index 913c1aa96..0d8ed4c0a 100644
--- a/src/kudu/tserver/heartbeater.cc
+++ b/src/kudu/tserver/heartbeater.cc
@@ -106,7 +106,7 @@ DEFINE_uint32(heartbeat_inject_required_feature_flag, 0,
 TAG_FLAG(heartbeat_inject_required_feature_flag, runtime);
 TAG_FLAG(heartbeat_inject_required_feature_flag, unsafe);
 
-DEFINE_int32(tserver_send_tombstoned_tablets_report_inteval_secs, 1800,
+DEFINE_int32(tserver_send_tombstoned_tablets_report_inteval_secs, -1,
              "Time interval in seconds of sending a incremental tablets report "
              "to delete the tombstoned replicas whose tablets had already been "
              "deleted. Turn off this by setting it to a value less than 0. If "
@@ -497,7 +497,6 @@ Status Heartbeater::Thread::DoHeartbeat(MasterErrorPB* error,
   // Send the most recently known TSK sequence number so that the master can
   // send us knew ones if they exist.
   req.set_latest_tsk_seq_num(server_->token_verifier().GetMaxKnownKeySequenceNumber());
-  bool including_tombstoned = false;
   if (send_full_tablet_report_) {
     LOG(INFO) << Substitute(
         "Master $0 was elected leader, sending a full tablet report...",
@@ -515,10 +514,14 @@ Status Heartbeater::Thread::DoHeartbeat(MasterErrorPB* error,
     VLOG(2) << Substitute("Sending an incremental tablet report to master $0...",
                           master_address_.ToString());
     // Check if it is time to send a report with tombstoned replicas.
-    including_tombstoned = FLAGS_tserver_send_tombstoned_tablets_report_inteval_secs >= 0 &&
-        (MonoTime::Now() - last_tombstoned_report_time_).ToSeconds() >=
+    const auto now = MonoTime::Now();
+    const auto tombstoned_report_interval =
         FLAGS_tserver_send_tombstoned_tablets_report_inteval_secs;
-    GenerateIncrementalTabletReport(req.mutable_tablet_report(), including_tombstoned);
+    const auto include_tombstoned = tombstoned_report_interval >= 0 &&
+        (now - last_tombstoned_report_time_).ToSeconds() >= tombstoned_report_interval;
+    GenerateIncrementalTabletReport(req.mutable_tablet_report(), include_tombstoned);
+    // Update the timestamp of last report on tombstoned tablet replicas.
+    last_tombstoned_report_time_ = now;
   }
 
   req.set_num_live_tablets(server_->tablet_manager()->GetNumLiveTablets());