Skip to content

Commit

Permalink
raft: Avoid having more than one snapshot in-flight.
Browse files Browse the repository at this point in the history
Previous commit 8c2c503 ("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: 8c2c503 ("raft: Avoid sending equal snapshots.")
Acked-by: Dumitru Ceara <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
igsilya committed Nov 3, 2020
1 parent f38f98a commit 83fbd2e
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 29 deletions.
1 change: 0 additions & 1 deletion ovsdb/raft-private.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ raft_server_destroy(struct raft_server *s)
if (s) {
free(s->address);
free(s->nickname);
free(s->last_install_snapshot_request);
free(s);
}
}
Expand Down
4 changes: 2 additions & 2 deletions ovsdb/raft-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ struct raft_server {
bool replied; /* Reply to append_request was received from this
node during current election_timeout interval.
*/
/* Copy of the last install_snapshot_request sent to this server. */
struct raft_install_snapshot_request *last_install_snapshot_request;
/* install_snapshot_request has been sent, but there is no response yet. */
bool install_snapshot_request_in_progress;

/* For use in adding and removing servers: */
struct uuid requester_sid; /* Nonzero if requested via RPC. */
Expand Down
42 changes: 16 additions & 26 deletions ovsdb/raft.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 83fbd2e

Please sign in to comment.