On 12/13/21 18:44, Han Zhou wrote: > > > On Mon, Dec 13, 2021 at 3:03 AM Dumitru Ceara <[email protected] > <mailto:[email protected]>> wrote: >> >> Commit 3c2d6274bcee ("raft: Transfer leadership before creating >> snapshots.") made it such that raft leaders transfer leadership before >> snapshotting. However, there's still the case when the next leader to >> be is in the process of snapshotting. To avoid delays in that case too, >> we now explicitly allow snapshots only on followers. Cluster members >> will have to wait until the current election is settled before >> snapshotting. >> >> Given the following logs taken from an OVN_Southbound 3-server cluster >> during a scale test: >> >> S1 (old leader): >> 2021-12-10T19:07:51.226Z|00823|raft|INFO|Transferring leadership to write >> a snapshot. >> 2021-12-10T19:08:03.830Z|00824|ovsdb|INFO|OVN_Southbound: Database >> compaction took 12601ms >> 2021-12-10T19:08:03.833Z|00825|timeval|WARN|Unreasonably long 12604ms poll >> interval (10632ms user, 1924ms system) >> 2021-12-10T19:08:03.940Z|00838|raft|INFO|server 8b8d is leader for term 43 >> >> S2 (follower): >> 2021-12-10T19:08:00.870Z|00481|raft|INFO|server 8b8d is leader for term 43 >> >> S3 (new leader): >> 2021-12-10T19:07:51.242Z|01083|raft|INFO|received leadership transfer from >> f5c9 in term 42 >> 2021-12-10T19:07:51.244Z|01084|raft|INFO|term 43: starting election >> 2021-12-10T19:08:00.805Z|01085|ovsdb|INFO|OVN_Southbound: Database >> compaction took 9559ms >> 2021-12-10T19:08:00.869Z|01100|raft|INFO|term 43: elected leader by 2+ of >> 3 servers >> >> We see that the leader to be (S3) receives the leadership transfer, >> initiates the election and immediately after starts a snapshot that >> takes ~9.5 seconds. During this time, S2 votes for S3 electing it >> as cluster leader but S3 doesn't effectively become leader until it >> finishes snapshotting, essentially keeping the cluster without a >> leader for up to ~9.5 seconds. >> >> With the current change, S3 will delay compaction and snapshotting until >> the election is finished. >> >> Signed-off-by: Dumitru Ceara <[email protected] <mailto:[email protected]>> >> --- >> ovsdb/raft.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/ovsdb/raft.c b/ovsdb/raft.c >> index ce40c5bc075c..6ffcb21db1e2 100644 >> --- a/ovsdb/raft.c >> +++ b/ovsdb/raft.c >> @@ -4226,7 +4226,7 @@ raft_may_snapshot(const struct raft *raft) >> && !raft->leaving >> && !raft->left >> && !raft->failed >> - && raft->role != RAFT_LEADER >> + && raft->role == RAFT_FOLLOWER >> && raft->last_applied >= raft->log_start); >> } >> >> -- >> 2.27.0 >> > > Thanks Dumitru! This change looks good to me. > In addition, I'd add a check for server count so that the follower role is > required only if there are more than one server. i.e.: > && (raft->role == RAFT_FOLLOWER || hmap_count(&raft->servers) == 1) > It is not a problem for real production deployments but since 1 node cluster > is supported by ovsdb RAFT, it is better to allow snapshot for that scenario, > too. Of course this is not a problem of this patch, but existed already before > this change.
I didn't test this, but I think that cluster of one server should be able to snapshot even with the current implementation. This is because 'raft_notify_snapshot_recommended' will "transfer" the leadership to no-one and force-switch the role to RAFT_FOLLOWER in the 'raft_become_follower'. And this server will stay in this state waiting for the election timer. Once election timer expired, the server will re-elect itself as a leader. So, there is at least 1 second time frame where conditions are sufficient for the compaction. Though, some non-raft wakeup needs to occur before the election timer expired in order to trigger the compaction. And I agree that it's not a very clean way to do things, so it's definitely better to avoid the transfer for the clusters of one server. Thanks for noticing this! > > Acked-by: Han Zhou <[email protected] <mailto:[email protected]>> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
