Skip to content

Commit

Permalink
Merge pull request redpanda-data#19899 from nvartolomei/nv/raft-timeo…
Browse files Browse the repository at this point in the history
…ut-lr

raft: fix changing raft_election_timeout_ms without restart
  • Loading branch information
piyushredpanda authored Jun 24, 2024
2 parents 30c6c7f + 447cabd commit e25d9f0
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 e25d9f0

Please sign in to comment.