Skip to content

Commit

Permalink
raft: fix changing raft_election_timeout_ms without restart
Browse files Browse the repository at this point in the history
Previously the watch callback was set on the binding which didn't leave
past the constructor so it was never called.
  • Loading branch information
nvartolomei committed Jun 20, 2024
1 parent f94b94a commit 447cabd
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 2 deletions.
42 changes: 42 additions & 0 deletions src/v/raft/tests/jitter_tests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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());
}
17 changes: 15 additions & 2 deletions src/v/raft/timeout_jitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 447cabd

Please sign in to comment.