Skip to content

Commit

Permalink
r/tests: fixed incorrect assumptions in replace_whole_group test
Browse files Browse the repository at this point in the history
Fixed `replace_whole_group` test by waiting for the whole raft group to
be up to date before executing configuration change.

The test was flaky as sometimes the configuration was changed before
propagating any records to one of the nodes (raft correctness is based
on majority). This caused the condition validating if some batches are
read to fail permanently.

Fixes: redpanda-data#342

Signed-off-by: Michal Maslanka <michal@redpanda.com>
  • Loading branch information
mmaslankaprv committed Jul 28, 2022
1 parent bb24073 commit 8e506f1
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/v/raft/tests/membership_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,12 @@ FIXTURE_TEST(replace_whole_group, raft_test_fixture) {
gr.enable_all();
info("replicating some batches");
auto res = replicate_random_batches(gr, 5).get0();
// wait for all group members to have the same log
wait_for(
10s,
[&] { return are_all_commit_indexes_the_same(gr); },
"initially all replicas have the same committed offsets");

// all nodes are replaced with new node
gr.create_new_node(model::node_id(5));
std::vector<raft::broker_revision> new_members;
Expand Down Expand Up @@ -292,7 +298,7 @@ FIXTURE_TEST(replace_whole_group, raft_test_fixture) {
}
auto last_old = old_nodes_log.begin()->second.back().last_offset();
auto last_new = new_nodes_log.begin()->second.back().last_offset();
if (last_new <= last_old) {
if (last_new < last_old) {
return false;
}
return std::all_of(
Expand Down

0 comments on commit 8e506f1

Please sign in to comment.