Checking parameters (#385)

* election timeout upper bound should larger than the lower * format * A smooth way to handle invalid params * add test mode flag * fix tests
eBay · Nov 21, 2022 · 71c3340 · 71c3340
1 parent 1c2587f
commit 71c3340
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 5 deletions.
diff --git a/include/libnuraft/raft_server.hxx b/include/libnuraft/raft_server.hxx
@@ -68,6 +68,15 @@ public:
         init_options()
             : skip_initial_election_timeout_(false)
             , start_server_in_constructor_(true)
+            , test_mode_flag_(false)
+            {}
+
+        init_options(bool skip_initial_election_timeout,
+                     bool start_server_in_constructor,
+                     bool test_mode_flag)
+            : skip_initial_election_timeout_(skip_initial_election_timeout)
+            , start_server_in_constructor_(start_server_in_constructor)
+            , test_mode_flag_(test_mode_flag)
             {}
 
         /**
@@ -91,6 +100,11 @@ public:
          * in constructor. Initialize election timer.
          */
         bool start_server_in_constructor_;
+
+        /**
+          * If `true`, test mode is enabled.
+         */
+        bool test_mode_flag_;
     };
 
     struct limits {
@@ -1460,6 +1474,11 @@ protected:
      *          awaiter at a time, by the help of `lock_`.
      */
     EventAwaiter* ea_follower_log_append_;
+
+    /**
+     * If `true`, test mode is enabled.
+     */
+    std::atomic<bool> test_mode_flag_;
 };
 
 } // namespace nuraft;

diff --git a/src/raft_server.cxx b/src/raft_server.cxx
@@ -107,6 +107,7 @@ raft_server::raft_server(context* ctx, const init_options& opt)
                                                 std::placeholders::_2 ) )
     , last_snapshot_(ctx->state_machine_->last_snapshot())
     , ea_follower_log_append_(new EventAwaiter())
+    , test_mode_flag_(opt.test_mode_flag_)
 {
     char temp_buf[4096];
     std::string print_msg;
@@ -372,6 +373,22 @@ void raft_server::update_params(const raft_params& new_params) {
 
 void raft_server::apply_and_log_current_params() {
     ptr<raft_params> params = ctx_->get_params();
+
+    if (!test_mode_flag_) {
+        if (params->heart_beat_interval_ >= params->election_timeout_lower_bound_) {
+            params->election_timeout_lower_bound_ = params->heart_beat_interval_ * 2;
+            p_wn("invalid election timeout lower bound detected, adjusted to %d",
+                 params->election_timeout_lower_bound_);
+        }
+        if (params->election_timeout_lower_bound_
+            >= params->election_timeout_upper_bound_) {
+            params->election_timeout_upper_bound_ =
+                params->election_timeout_lower_bound_ * 2;
+            p_wn("invalid election timeout upper bound detected, adjusted to %d",
+                 params->election_timeout_upper_bound_);
+        }
+    }
+
     p_in( "parameters: "
           "timeout %d - %d, heartbeat %d, "
           "leadership expiry %d, "

diff --git a/tests/unit/failure_test.cxx b/tests/unit/failure_test.cxx
@@ -291,7 +291,7 @@ int force_log_compaction_test() {
     std::atomic<bool> invoked(false);
     for (size_t ii = 0; ii < pkgs.size(); ++ii) {
         RaftPkg* ff = pkgs[ii];
-        raft_server::init_options opt;
+        raft_server::init_options opt(false, true, true);
         if (ii < 2) {
             opt.raft_callback_ = cb_default;
             ff->initServer(nullptr, opt);

diff --git a/tests/unit/raft_package_fake.hxx b/tests/unit/raft_package_fake.hxx
@@ -54,7 +54,7 @@ public:
 
     void initServer(raft_params* given_params = nullptr,
                     const raft_server::init_options& opt =
-                        raft_server::init_options())
+                        raft_server::init_options(false, true, true))
     {
         fNet = cs_new<FakeNetwork>( myEndpoint, fBase );
         fBase->addNetwork(fNet);
@@ -95,7 +95,7 @@ public:
      */
     void restartServer(raft_params* given_params = nullptr,
                        const raft_server::init_options& opt =
-                           raft_server::init_options())
+                           raft_server::init_options(false, true, true))
     {
         if (!given_params) {
             params.with_election_timeout_lower(0);
@@ -234,7 +234,7 @@ static INT_UNUSED launch_servers(const std::vector<RaftPkg*>& pkgs,
     size_t num_srvs = pkgs.size();
     CHK_GT(num_srvs, 0);
 
-    raft_server::init_options opt;
+    raft_server::init_options opt(false, true, true);
     opt.raft_callback_ = cb_default;
 
     for (size_t ii = 0; ii < num_srvs; ++ii) {

diff --git a/tests/unit/raft_server_test.cxx b/tests/unit/raft_server_test.cxx
@@ -187,7 +187,7 @@ int init_options_test() {
     size_t num_srvs = pkgs.size();
     CHK_GT(num_srvs, 0);
 
-    raft_server::init_options opt;
+    raft_server::init_options opt(false, true, true);
 
     for (size_t ii = 0; ii < num_srvs; ++ii) {
         RaftPkg* ff = pkgs[ii];