raft: Avoid having more than one snapshot in-flight.

Previous commit 8c2c503bdb0d ("raft: Avoid sending equal snapshots.")
took a "safe" approach to not send only exactly same snapshot
installation requests.  However, it doesn't make much sense to send
more than one snapshot at a time.  If obsolete snapshot installed,
leader will re-send the most recent one.

With this change leader will have only 1 snapshot in-flight per
connection.  This will reduce backlogs on raft connections in case
new snapshot created while 'install_snapshot_request' is in progress
or if election timer changed in that period.

Also, not tracking the exact 'install_snapshot_request' we've sent
allows to simplify the code.

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1888829
Fixes: 8c2c503bdb0d ("raft: Avoid sending equal snapshots.")
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 16 insertions, 26 deletions

diff --git a/ovsdb/raft.c b/ovsdb/raft.c
index ac85c6b67..f94a3eed8 100644
--- a/ovsdb/raft.c
+++ b/ovsdb/raft.c
@@ -1437,12 +1437,11 @@ raft_conn_run(struct raft *raft, struct raft_conn *conn)
                            && jsonrpc_session_is_connected(conn->js));
 
     if (reconnected) {
-        /* Clear 'last_install_snapshot_request' since it might not reach the
-         * destination or server was restarted. */
+        /* Clear 'install_snapshot_request_in_progress' since it might not
+         * reach the destination or server was restarted. */
         struct raft_server *server = raft_find_server(raft, &conn->sid);
         if (server) {
-            free(server->last_install_snapshot_request);
-            server->last_install_snapshot_request = NULL;
+            server->install_snapshot_request_in_progress = false;
         }
     }
 
@@ -2564,6 +2563,7 @@ raft_server_init_leader(struct raft *raft, struct raft_server *s)
     s->match_index = 0;
     s->phase = RAFT_PHASE_STABLE;
     s->replied = false;
+    s->install_snapshot_request_in_progress = false;
 }
 
 static void
@@ -3320,31 +3320,19 @@ raft_send_install_snapshot_request(struct raft *raft,
         }
     };
 
-    if (s->last_install_snapshot_request) {
-        struct raft_install_snapshot_request *old, *new;
-
-        old = s->last_install_snapshot_request;
-        new = &rpc.install_snapshot_request;
-        if (   old->term           == new->term
-            && old->last_index     == new->last_index
-            && old->last_term      == new->last_term
-            && old->last_servers   == new->last_servers
-            && old->data           == new->data
-            && old->election_timer == new->election_timer
-            && uuid_equals(&old->last_eid, &new->last_eid)) {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
+    if (s->install_snapshot_request_in_progress) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
 
-            VLOG_WARN_RL(&rl, "not sending exact same install_snapshot_request"
-                              " to server %s again", s->nickname);
-            return;
-        }
+        VLOG_INFO_RL(&rl, "not sending snapshot to server %s, "
+                          "already in progress", s->nickname);
+        return;
     }
-    free(s->last_install_snapshot_request);
-    CONST_CAST(struct raft_server *, s)->last_install_snapshot_request
-        = xmemdup(&rpc.install_snapshot_request,
-                  sizeof rpc.install_snapshot_request);
 
-    raft_send(raft, &rpc);
+    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);
+    VLOG_INFO_RL(&rl, "sending snapshot to server %s, %"PRIu64":%"PRIu64".",
+                      s->nickname, raft->term, raft->log_start - 1);
+    CONST_CAST(struct raft_server *, s)->install_snapshot_request_in_progress
+        = raft_send(raft, &rpc);
 }
 
 static void
@@ -4061,6 +4049,8 @@ raft_handle_install_snapshot_reply(
         }
     }
 
+    s->install_snapshot_request_in_progress = false;
+
     if (rpy->last_index != raft->log_start - 1 ||
         rpy->last_term != raft->snap.term) {
         static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 5);