A command forwarded to the leader by a follower, or appended locally by
a leader that later loses leadership, is completed with an error after
twice the election timer. The intent, as the comment says, is that the
command survives a leader election and completes once the new leader
commits the entry.
Twice the election timer is not enough for that. A follower starts an
election one election timer plus up to ELECTION_RANGE_MSEC (1000 ms) of
random jitter after the last heartbeat it received. After that the new
leader still has to complete the election and commit the entry before
the command can finish. With the default 1000 ms election timer and
unlucky jitter this leaves almost no time for the election itself, so
the command can time out just before the new leader commits its entry.
With election timers shorter than ELECTION_RANGE_MSEC the timeout can
even expire before the election starts at all.
The command then fails with a timeout, which ovsdb-server treats as a
temporary error and retries the transaction internally, even though the
original entry is about to be applied. For a non-idempotent
transaction, such as a row insert, the retry duplicates the data.
This was seen as a failure of the "OVSDB cluster - txn on follower-2,
leader crash before sending execRep, follower-3 becomes leader" test,
where the retried transaction inserted a second QoS row:
./ovsdb-cluster.at:819: ovs-vsctl --db="$db" --no-leader-only \
--no-wait --columns=type --bare list QoS
@@ -1,2 +1,4 @@
x
+x
+
Add the random part of the election timeout to the command timeout so
that the command cannot expire before an election it is supposed to
survive has had a chance to complete.
Reported-at: https://issues.redhat.com/browse/FDP-4210
Fixes: 5a9b53a51ec9 ("ovsdb raft: Fix duplicated transaction execution when
leader failover.")
Signed-off-by: Timothy Redaelli <[email protected]>
---
ovsdb/raft.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/ovsdb/raft.c b/ovsdb/raft.c
index b1355d41c..92686a9f7 100644
--- a/ovsdb/raft.c
+++ b/ovsdb/raft.c
@@ -2188,16 +2188,18 @@ raft_run(struct raft *raft)
if (raft->role == RAFT_LEADER) {
raft_send_heartbeats(raft);
}
- /* Check if any commands timeout. Timeout is set to twice the time of
- * election base time so that commands can complete properly during
- * leader election. E.g. a leader crashed and current node with pending
- * commands becomes new leader: the pending commands can still complete
+ /* Check if any commands timeout. Timeout is set to twice the
+ * election base time plus the election random range so that
+ * commands can complete properly during leader election.
+ * E.g. a leader crashed and current node with pending commands
+ * becomes new leader: the pending commands can still complete
* if the crashed leader has replicated the transactions to majority of
* followers before it crashed. */
struct raft_command *cmd;
HMAP_FOR_EACH_SAFE (cmd, hmap_node, &raft->commands) {
if (cmd->timestamp
- && now - cmd->timestamp > raft->election_timer * 2) {
+ && now - cmd->timestamp > (raft->election_timer * 2
+ + ELECTION_RANGE_MSEC)) {
if (cmd->index && raft->role != RAFT_LEADER) {
/* This server lost leadership and command didn't complete
* in time. Likely, it wasn't replicated to the majority
--
2.55.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
