From ccd20243e4d698c393cb7623874e1fa6bf142faa Mon Sep 17 00:00:00 2001
From: Alexander Indenbaum <aindenba@redhat.com>
Date: Mon, 20 Oct 2025 12:00:04 +0300
Subject: [PATCH] nvmeof: refactor timer for exact frequency timing with drift
 correction

- Replace relative timing with absolute time calculation to prevent drift
- Implement drift detection and correction mechanism
- Add comprehensive logging for timer firing, execution duration, drift
  and rescheduling

This refactoring ensures the nvmeof monitor client maintains exact frequency
timing over long periods, even when individual tick executions are delayed.

Signed-off-by: Alexander Indenbaum <aindenba@redhat.com>
---
 src/nvmeof/NVMeofGwMonitorClient.cc | 71 +++++++++++++++++++++++++----
 src/nvmeof/NVMeofGwMonitorClient.h  |  8 ++++
 2 files changed, 71 insertions(+), 8 deletions(-)

diff --git a/src/nvmeof/NVMeofGwMonitorClient.cc b/src/nvmeof/NVMeofGwMonitorClient.cc
index c362bf21ca2..5d37924573a 100644
--- a/src/nvmeof/NVMeofGwMonitorClient.cc
+++ b/src/nvmeof/NVMeofGwMonitorClient.cc
@@ -184,7 +184,10 @@ int NVMeofGwMonitorClient::init()
 
   {
     std::lock_guard bl(beacon_lock);
-    tick();
+    // Initialize timer state for exact frequency timing
+    next_tick_time = ceph::mono_clock::now();
+    // Start the timer after full initialization is complete
+    schedule_next_tick();
   }
 
   dout(10) << "Complete." << dendl;
@@ -283,19 +286,55 @@ void NVMeofGwMonitorClient::connect_panic()
 void NVMeofGwMonitorClient::tick()
 {
   dout(10) << dendl;
-
+  auto start_time = ceph::mono_clock::now();
   connect_panic();
   disconnect_panic();
   send_beacon();
   first_beacon = false;
-  timer.add_event_after(
-      g_conf().get_val<std::chrono::seconds>("nvmeof_mon_client_tick_period").count(),
-      new LambdaContext([this](int r){
-          tick();
-      }
-  ));
+
+  // Log tick execution duration
+  log_tick_execution_duration(start_time);
+
+  // Schedule next tick with exact frequency timing
+  schedule_next_tick();
 }
 
+void NVMeofGwMonitorClient::schedule_next_tick()
+{
+  ceph_assert(ceph_mutex_is_locked_by_me(beacon_lock));
+
+  // Calculate next tick time based on configured interval
+  auto tick_period = g_conf().get_val<std::chrono::seconds>("nvmeof_mon_client_tick_period");
+  next_tick_time += tick_period;
+
+  // Get current time to check for drift
+  auto now = ceph::mono_clock::now();
+
+  // If we're behind schedule, adjust next_tick_time to prevent drift accumulation
+  if (next_tick_time < now) {
+    dout(1) << "Timer drift detected, adjusting next_tick_time from "
+             << next_tick_time << " to " << now << dendl;
+    next_tick_time = now;
+  }
+
+  // Schedule the next tick
+  auto callback = new LambdaContext([this](int r) {
+    tick();
+  });
+
+  auto tick_time = next_tick_time;
+  if (!timer.add_event_at(tick_time, callback)) {
+    throw std::runtime_error("Failed to schedule timer event (timer shutting down)");
+  }
+
+  // Log scheduling information for frequency analysis
+  auto delay_ns = (next_tick_time - now).count();
+  dout(10) << "Scheduled next tick at " << next_tick_time
+          << " (delay: " << delay_ns << " ns, "
+          << (delay_ns / 1000000.0) << " ms)" << dendl;
+}
+
+
 void NVMeofGwMonitorClient::shutdown()
 {
   std::lock_guard l(lock);
@@ -475,3 +514,19 @@ int NVMeofGwMonitorClient::main(std::vector<const char *> args)
 
   return 0;
 }
+
+void NVMeofGwMonitorClient::log_tick_execution_duration(const ceph::mono_clock::time_point& start_time)
+{
+  auto execution_time = ceph::mono_clock::now() - start_time;
+  auto tick_period = g_conf().get_val<std::chrono::seconds>("nvmeof_mon_client_tick_period");
+  auto max_allowed_time = tick_period * 0.1; // 10% of tick period
+
+  dout(10) << "Tick execution took " << execution_time.count() / 1000000.0
+          << "ms" << dendl;
+
+  if (execution_time > max_allowed_time) {
+    dout(1) << "WARNING: Tick execution took " << execution_time.count() / 1000000.0
+            << "ms, exceeding 10% of tick period (" << max_allowed_time.count() / 1000000.0
+            << "ms)" << dendl;
+  }
+}
diff --git a/src/nvmeof/NVMeofGwMonitorClient.h b/src/nvmeof/NVMeofGwMonitorClient.h
index b1ca5c94deb..65ab9bb5eb7 100644
--- a/src/nvmeof/NVMeofGwMonitorClient.h
+++ b/src/nvmeof/NVMeofGwMonitorClient.h
@@ -21,6 +21,7 @@
 #include "common/Finisher.h"
 #include "common/Timer.h"
 #include "common/LogClient.h"
+#include "common/ceph_time.h"
 
 #include "mon/MonClient.h"
 #include "osdc/Objecter.h"
@@ -71,11 +72,18 @@ protected:
   ceph::mutex beacon_lock = ceph::make_mutex("NVMeofGw::beacon_lock");
   SafeTimer timer;
 
+  // Timer state for exact frequency timing
+  ceph::mono_clock::time_point next_tick_time;
+
   int orig_argc;
   const char **orig_argv;
 
   void send_config_beacon(); 
   void send_beacon();
+
+  // Timer management for exact frequency
+  void schedule_next_tick();
+  void log_tick_execution_duration(const ceph::mono_clock::time_point& start_time);
  
 public:
   NVMeofGwMonitorClient(int argc, const char **argv);
-- 
2.39.5