From 779e7ba41e20e9fb1e2b07d6b17a032bcad87878 Mon Sep 17 00:00:00 2001
From: Nicolae Vartolomei <nv@redpanda.com>
Date: Wed, 19 Jun 2024 22:00:36 +0100
Subject: [PATCH] raft: fix changing raft_election_timeout_ms without restart

Previously the watch callback was set on the binding which didn't leave
past the constructor so it was never called.
---
 src/v/raft/tests/jitter_tests.cc | 42 ++++++++++++++++++++++++++++++++
 src/v/raft/timeout_jitter.h      | 17 +++++++++++--
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/src/v/raft/tests/jitter_tests.cc b/src/v/raft/tests/jitter_tests.cc
index 5540728881018..85521418110a8 100644
--- a/src/v/raft/tests/jitter_tests.cc
+++ b/src/v/raft/tests/jitter_tests.cc
@@ -7,6 +7,8 @@
 // the Business Source License, use of this software will be governed
 // by the Apache License, Version 2.0
 
+#include "config/base_property.h"
+#include "config/config_store.h"
 #include "config/property.h"
 #include "raft/timeout_jitter.h"
 
@@ -33,3 +35,43 @@ SEASTAR_THREAD_TEST_CASE(base_jitter_gurantees) {
         BOOST_CHECK(next >= now + low && next <= now + high);
     }
 }
+
+struct config_store : public config::config_store {
+    config::property<std::chrono::milliseconds> timeout;
+
+    config_store()
+      :
+
+      timeout(
+        *this,
+        "timeout",
+        "timeout for the jitter",
+        {.needs_restart = config::needs_restart::no},
+        std::chrono::milliseconds(100)) {}
+};
+
+SEASTAR_THREAD_TEST_CASE(base_jitter_update) {
+    auto cfg = config_store();
+
+    raft::timeout_jitter jit(cfg.timeout.bind());
+    BOOST_CHECK_EQUAL(
+      std::chrono::duration_cast<std::chrono::milliseconds>(jit.base_duration())
+        .count(),
+      (cfg.timeout()).count());
+
+    cfg.timeout.set_value(std::chrono::milliseconds(200));
+    BOOST_CHECK_EQUAL(
+      std::chrono::duration_cast<std::chrono::milliseconds>(jit.base_duration())
+        .count(),
+      (cfg.timeout()).count());
+
+    // Test update after using the **move constructor**.
+    auto jit2 = std::move(jit);
+
+    cfg.timeout.set_value(std::chrono::milliseconds(300));
+    BOOST_CHECK_EQUAL(
+      std::chrono::duration_cast<std::chrono::milliseconds>(
+        jit2.base_duration())
+        .count(),
+      (cfg.timeout()).count());
+}
diff --git a/src/v/raft/timeout_jitter.h b/src/v/raft/timeout_jitter.h
index c0a8111b5f74f..e1f15ddd33ca6 100644
--- a/src/v/raft/timeout_jitter.h
+++ b/src/v/raft/timeout_jitter.h
@@ -19,10 +19,23 @@ namespace raft {
 class timeout_jitter {
 public:
     explicit timeout_jitter(config::binding<std::chrono::milliseconds> timeout)
-      : _base_timeout(timeout)
+      : _base_timeout(std::move(timeout))
       , _time_jitter(_base_timeout()) {
-        timeout.watch([this] { update_base_timeout(); });
+        _base_timeout.watch([this] { update_base_timeout(); });
     }
+
+    timeout_jitter(const timeout_jitter&) = delete;
+    timeout_jitter& operator=(const timeout_jitter&) = delete;
+
+    timeout_jitter(timeout_jitter&& rhs) noexcept
+      : _base_timeout(std::move(rhs._base_timeout))
+      , _time_jitter(std::move(rhs._time_jitter)) {
+        _base_timeout.watch([this] { update_base_timeout(); });
+    };
+    timeout_jitter& operator=(timeout_jitter&& rhs) = delete;
+
+    ~timeout_jitter() = default;
+
     raft::clock_type::time_point operator()() { return _time_jitter(); }
 
     raft::clock_type::duration base_duration() const {