[Tarantool-patches] [PATCH v2 2/5] raft: fix ev_timer.at incorrect usage

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

ev_timer.at was used as timeout. But after ev_timer_start() it
turns into the deadline - totally different value.

The patch makes sure ev_timer.at is not used in raft at all.

To test that the fakeev subsystem is patched to start its time not
from 0. Otherwise ev_timer.at often really matched the timeout
even for an active timer.
---
 src/lib/fakesys/fakeev.c | 10 +++++--
 src/lib/raft/raft.c      | 59 ++++++++++++++++++++++------------------
 src/lib/raft/raft.h      |  2 +-
 3 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/src/lib/fakesys/fakeev.c b/src/lib/fakesys/fakeev.c
index 5c3469486..333c1646f 100644
--- a/src/lib/fakesys/fakeev.c
+++ b/src/lib/fakesys/fakeev.c
@@ -37,8 +37,11 @@
 #include "say.h"
 #include <stdbool.h>
 
+/** Start not from 0 to catch bugs like incorrect usage of ev_timer members. */
+#define FAKEEV_START_TIME 10000
+
 /** Global watch, propagated by new events. */
-static double watch = 0;
+static double watch = FAKEEV_START_TIME;
 
 /**
  * Increasing event identifiers are used to preserve order of
@@ -278,6 +281,7 @@ fakeev_timer_start(struct ev_loop *loop, struct ev_timer *base)
 		return;
 	/* Create the periodic watcher and one event. */
 	fakeev_timer_event_new((struct ev_watcher *)base, base->at);
+	base->at += watch;
 }
 
 double
@@ -287,6 +291,7 @@ fakeev_timer_remaining(struct ev_loop *loop, struct ev_timer *base)
 	struct fakeev_event *e = fakeev_event_by_ev((struct ev_watcher *)base);
 	if (e == NULL)
 		return base->at;
+	assert(e->deadline == base->at);
 	return e->deadline - fakeev_time();
 }
 
@@ -298,6 +303,7 @@ fakeev_timer_again(struct ev_loop *loop, struct ev_timer *base)
 		return;
 	/* Create the periodic watcher and one event. */
 	fakeev_timer_event_new((struct ev_watcher *)base, base->repeat);
+	base->at = watch + base->repeat;
 }
 
 /** Time stop cancels the event if the timer is active. */
@@ -339,7 +345,7 @@ fakeev_reset(void)
 		fakeev_event_delete(e);
 	assert(mh_size(events_hash) == 0);
 	event_id = 0;
-	watch = 0;
+	watch = FAKEEV_START_TIME;
 }
 
 void
diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c
index 1ae8fe87f..d6a849503 100644
--- a/src/lib/raft/raft.c
+++ b/src/lib/raft/raft.c
@@ -931,22 +931,23 @@ raft_restore(struct raft *raft)
 void
 raft_cfg_election_timeout(struct raft *raft, double timeout)
 {
-	if (timeout == raft->election_timeout)
+	double old_timeout = raft->election_timeout;
+	if (timeout == old_timeout)
 		return;
 
 	raft->election_timeout = timeout;
-	if (raft->vote != 0 && raft->leader == 0 && raft->is_candidate &&
-	    !raft->is_write_in_progress) {
-		assert(raft_ev_is_active(&raft->timer));
-		struct ev_loop *loop = raft_loop();
-		double timeout = raft_ev_timer_remaining(loop, &raft->timer) -
-				 raft->timer.at + raft->election_timeout;
-		if (timeout < 0)
-			timeout = 0;
-		raft_ev_timer_stop(loop, &raft->timer);
-		raft_ev_timer_set(&raft->timer, timeout, timeout);
-		raft_ev_timer_start(loop, &raft->timer);
-	}
+	if (raft->vote == 0 || raft->leader != 0 || !raft->is_candidate ||
+	    raft->is_write_in_progress)
+		return;
+
+	assert(raft_ev_is_active(&raft->timer));
+	struct ev_loop *loop = raft_loop();
+	timeout += raft_ev_timer_remaining(loop, &raft->timer) - old_timeout;
+	if (timeout < 0)
+		timeout = 0;
+	raft_ev_timer_stop(loop, &raft->timer);
+	raft_ev_timer_set(&raft->timer, timeout, raft->election_timeout);
+	raft_ev_timer_start(loop, &raft->timer);
 }
 
 void
@@ -961,21 +962,25 @@ raft_cfg_election_quorum(struct raft *raft, int election_quorum)
 }
 
 void
-raft_cfg_death_timeout(struct raft *raft, double death_timeout)
+raft_cfg_death_timeout(struct raft *raft, double timeout)
 {
-	raft->death_timeout = death_timeout;
-	if (raft->state == RAFT_STATE_FOLLOWER && raft->is_candidate &&
-	    raft->leader != 0) {
-		assert(raft_ev_is_active(&raft->timer));
-		struct ev_loop *loop = raft_loop();
-		double timeout = raft_ev_timer_remaining(loop, &raft->timer) -
-				 raft->timer.at + raft->death_timeout;
-		if (timeout < 0)
-			timeout = 0;
-		raft_ev_timer_stop(loop, &raft->timer);
-		raft_ev_timer_set(&raft->timer, timeout, timeout);
-		raft_ev_timer_start(loop, &raft->timer);
-	}
+	double old_timeout = raft->death_timeout;
+	if (timeout == old_timeout)
+		return;
+
+	raft->death_timeout = timeout;
+	if (raft->state != RAFT_STATE_FOLLOWER || !raft->is_candidate ||
+	    raft->leader == 0)
+		return;
+
+	assert(raft_ev_is_active(&raft->timer));
+	struct ev_loop *loop = raft_loop();
+	timeout += raft_ev_timer_remaining(loop, &raft->timer) - old_timeout;
+	if (timeout < 0)
+		timeout = 0;
+	raft_ev_timer_stop(loop, &raft->timer);
+	raft_ev_timer_set(&raft->timer, timeout, raft->death_timeout);
+	raft_ev_timer_start(loop, &raft->timer);
 }
 
 void
diff --git a/src/lib/raft/raft.h b/src/lib/raft/raft.h
index 53e2c58a8..e844c2d07 100644
--- a/src/lib/raft/raft.h
+++ b/src/lib/raft/raft.h
@@ -307,7 +307,7 @@ raft_cfg_election_quorum(struct raft *raft, int election_quorum);
  * heartbeats from the leader to consider it dead.
  */
 void
-raft_cfg_death_timeout(struct raft *raft, double death_timeout);
+raft_cfg_death_timeout(struct raft *raft, double timeout);
 
 /**
  * Configure ID of the given Raft instance. The ID can't be changed after it is
-- 
2.24.3 (Apple Git-128)