From cb4e6075344b50cc3606e4d7946a4ad985404c4c Mon Sep 17 00:00:00 2001 From: Michal Maslanka Date: Mon, 20 Nov 2023 16:16:03 +0100 Subject: [PATCH 1/2] r/consensus: log configuration replace event Signed-off-by: Michal Maslanka --- src/v/raft/consensus.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/v/raft/consensus.cc b/src/v/raft/consensus.cc index f0696625bfe81..2f056cd038aa1 100644 --- a/src/v/raft/consensus.cc +++ b/src/v/raft/consensus.cc @@ -1158,10 +1158,16 @@ ss::future consensus::replace_configuration( model::revision_id new_revision, std::optional learner_start_offset) { return change_configuration( - [nodes = std::move(nodes), new_revision, learner_start_offset]( + [this, nodes = std::move(nodes), new_revision, learner_start_offset]( group_configuration current) mutable { + auto old = current; current.set_version(raft::group_configuration::v_5); current.replace(nodes, new_revision, learner_start_offset); + vlog( + _ctxlog.debug, + "Replacing current configuration: {} with new configuration: {}", + old, + current); return result{std::move(current)}; }); From 7bb54d3c5e747f0809e7d9fc74a36e2a8a34e25a Mon Sep 17 00:00:00 2001 From: Michal Maslanka Date: Mon, 20 Nov 2023 16:42:47 +0100 Subject: [PATCH 2/2] r/recovery_stm: stop recovery when follower was already updated Recovery and replicate stms are not synchronized. It may be the case when both of stms are active at the same time that the same batch is delivered to the follower twice. In general this batch duplication is harmless as Raft is not vulnerable for messages redelivery but it may cause unnecessary truncation and latency increase. Added a check validating expected log end offset right before sending recovery append entries request. This will prevent sending the same set of batches twice to the follower. Fixes: #14413 Signed-off-by: Michal Maslanka --- src/v/raft/recovery_stm.cc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/v/raft/recovery_stm.cc b/src/v/raft/recovery_stm.cc index c1beda8ae2ce1..8f27c802bc2c3 100644 --- a/src/v/raft/recovery_stm.cc +++ b/src/v/raft/recovery_stm.cc @@ -557,6 +557,18 @@ ss::future<> recovery_stm::replicate( _stop_requested = true; return ss::now(); } + if (meta.value()->expected_log_end_offset >= _last_batch_offset) { + vlog( + _ctxlog.trace, + "follower expected log end offset is already updated, stopping " + "recovery. Expected log end offset: {}, recovery range last offset: " + "{}", + meta.value()->expected_log_end_offset, + _last_batch_offset); + + _stop_requested = true; + return ss::now(); + } /** * Update follower expected log end. It is equal to the last batch in a set * of batches read for this recovery round.