lib/wsrep-lib
1
0
Fork 0
mirror of https://github.com/codership/wsrep-lib.git synced 2026-10-23 09:48:57 +03:00
Code Activity

Cover remove_fragments() failures and commit_fragment()'s apply_err

Browse Source 
branch

The existing regression tests for the missing after_apply() cleanup
did not exercise every failing call site: apply_fragment()'s
fragment-removal block only had a test for its trailing commit()
failing, leaving remove_fragments() itself untested, and
commit_fragment()'s apply_err branch (a separate "ret ||
(after_apply(), 0)" site, reached when applying the last fragment
itself fails) had no coverage at all. Add tests for both, plus a
passing coverage check that rollback_fragment()'s own fragment-removal
block -- which already calls after_apply() unconditionally -- doesn't
regress into the same short-circuit idiom.

Add a fail_next_remove_fragments_ hook to mock_high_priority_service
to allow simulating a storage-engine level failure while removing
streaming fragments.
This commit is contained in:
Teemu Ollakka
2026-07-06 17:37:29 +03:00
parent 7a180cb227
commit fbadbdd2a7
2 changed files with 112 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
test/mock_high_priority_service.hpp
View File

@@ -40,6 +40,7 @@ namespace wsrep
, fail_next_toi_()
, fail_next_rollback_()
, fail_next_append_fragment_()
, fail_next_remove_fragments_()
, after_apply_calls_()
, client_state_(client_state)
, replaying_(replaying)
@@ -66,7 +67,7 @@ namespace wsrep
const wsrep::xid&) WSREP_OVERRIDE
{ return fail_next_append_fragment_ ? 1 : 0; }
int remove_fragments(const wsrep::ws_meta&) WSREP_OVERRIDE
{ return 0; }
{ return fail_next_remove_fragments_ ? 1 : 0; }
int commit(const wsrep::ws_handle&, const wsrep::ws_meta&)
WSREP_OVERRIDE;
int rollback(const wsrep::ws_handle&, const wsrep::ws_meta&) WSREP_OVERRIDE;
@@ -101,6 +102,9 @@ namespace wsrep
// state machine.
bool fail_next_rollback_;
bool fail_next_append_fragment_;
// Force remove_fragments() to report failure, e.g. to simulate a
// storage-engine level failure while removing streaming fragments.
bool fail_next_remove_fragments_;
// Number of times after_apply() has been called on this instance,
// so tests can verify it was (or was not) invoked after a failure.
size_t after_apply_calls_;

107
test/server_state_applier_cleanup_test.cpp
View File

@@ -42,10 +42,19 @@
// - apply_write_set(): commit() fails (FIXED)
// - apply_fragment(): rollback() fails after a failed apply
// - apply_fragment(): commit() fails while removing fragments
// - apply_fragment(): remove_fragments() itself fails
// - apply_fragment(): append_fragment_and_commit() fails
// - commit_fragment(): apply of the last fragment fails and the
// resulting rollback also fails
// - commit_fragment(): commit() fails on the last fragment
// - rollback_fragment(): rollback() fails
//
// rollback_fragment()'s own fragment-removal block does *not* share the
// anti-pattern -- it calls after_apply() unconditionally rather than via
// "ret || (after_apply(), 0)". A passing test for its remove_fragments()
// failure is included below as a coverage/non-regression check, so that
// spot is not accidentally downgraded to the buggy idiom later.
//
#include "mock_server_state.hpp"
@@ -179,6 +188,38 @@ BOOST_FIXTURE_TEST_CASE(apply_fragment_remove_fragments_commit_failure_calls_aft
BOOST_REQUIRE_EQUAL(sa->after_apply_calls_, after_apply_calls_before + 2);
}
// apply_fragment(): a fragment fails to apply, the rollback of that
// attempt succeeds, but remove_fragments() itself (not the commit that
// follows it) fails, e.g. a storage-engine level failure while removing
// streaming fragments. Distinct from the commit-failure case above:
// remove_fragments() failing here must skip neither commit() nor
// after_apply() any differently, but it exercises a separate short-circuit
// point that a fix targeting only the commit() failure would miss.
BOOST_FIXTURE_TEST_CASE(apply_fragment_remove_fragments_failure_calls_after_apply,
applier_cleanup_fixture)
{
wsrep::mock_high_priority_service* sa(
start_streaming_fragment(ss, hps, ws_handle,
wsrep::id("1"), wsrep::transaction_id(1)));
sa->fail_next_applying_ = true;
sa->fail_next_remove_fragments_ = true;
size_t const after_apply_calls_before(sa->after_apply_calls_);
wsrep::ws_meta mid_meta(
wsrep::gtid(wsrep::id("1"), wsrep::seqno(2)),
wsrep::stid(wsrep::id("1"), wsrep::transaction_id(1),
wsrep::client_id(1)),
wsrep::seqno(1),
0);
char buf[1] = { 1 };
BOOST_REQUIRE(ss.on_apply(hps, ws_handle, mid_meta,
wsrep::const_buffer(buf, 1)) != 0);
// One after_apply() call is expected from the rollback of the failed
// apply, and a second one after the (failing) remove_fragments() call.
BOOST_REQUIRE_EQUAL(sa->after_apply_calls_, after_apply_calls_before + 2);
}
// apply_fragment(): a fragment applies successfully, but
// append_fragment_and_commit() (appending it to persistent fragment
// storage) fails. after_apply() must still run on the coordinating
@@ -201,6 +242,35 @@ BOOST_FIXTURE_TEST_CASE(apply_fragment_append_fragment_failure_calls_after_apply
BOOST_REQUIRE_EQUAL(hps.after_apply_calls_, after_apply_calls_before + 1);
}
// commit_fragment(): applying the last (commit) fragment itself fails, and
// the rollback of that failed apply also fails (e.g. a storage-engine
// level rollback failure). This exercises commit_fragment()'s apply_err
// branch, which has its own, separate "ret || (after_apply(), 0)" call
// site that the commit-failure test below never reaches (that test takes
// the apply_err == 0 path). after_apply() must still run.
BOOST_FIXTURE_TEST_CASE(commit_fragment_apply_and_rollback_failure_calls_after_apply,
applier_cleanup_fixture)
{
wsrep::mock_high_priority_service* sa(
start_streaming_fragment(ss, hps, ws_handle,
wsrep::id("1"), wsrep::transaction_id(1)));
sa->fail_next_applying_ = true;
sa->fail_next_rollback_ = true;
size_t const after_apply_calls_before(sa->after_apply_calls_);
wsrep::ws_meta commit_meta(
wsrep::gtid(wsrep::id("1"), wsrep::seqno(2)),
wsrep::stid(wsrep::id("1"), wsrep::transaction_id(1),
wsrep::client_id(1)),
wsrep::seqno(1),
wsrep::provider::flag::commit);
char buf[1] = { 1 };
BOOST_REQUIRE(ss.on_apply(hps, ws_handle, commit_meta,
wsrep::const_buffer(buf, 1)) != 0);
BOOST_REQUIRE_EQUAL(sa->after_apply_calls_, after_apply_calls_before + 1);
}
// commit_fragment(): the final commit of a streaming (or XA) transaction
// fails, e.g. because commit order could not be entered while the node is
// leaving the primary component -- the streaming-transaction analogue of
@@ -273,3 +343,40 @@ BOOST_FIXTURE_TEST_CASE(rollback_fragment_rollback_failure_calls_after_apply,
rollback_hps.rollback(ws_handle, rollback_meta);
rollback_hps.after_apply();
}
// rollback_fragment(): the streaming applier's own out-of-order rollback
// succeeds, but remove_fragments() on the coordinating high_priority_service
// fails while removing the now-obsolete fragments from storage. Unlike its
// siblings above, this call site already calls after_apply()
// unconditionally rather than through "ret || (after_apply(), 0)", so this
// is a coverage/non-regression check rather than a bug demonstration.
BOOST_FIXTURE_TEST_CASE(rollback_fragment_remove_fragments_failure_calls_after_apply,
applier_cleanup_fixture)
{
wsrep::mock_high_priority_service* sa(
start_streaming_fragment(ss, hps, ws_handle,
wsrep::id("1"), wsrep::transaction_id(1)));
(void)sa;
// Use a separate high priority service for the rollback write set,
// mirroring how a real rollback fragment is delivered on its own
// applier thread (see wsrep_test::terminate_streaming_applier()).
wsrep::mock_client rollback_cc(ss, wsrep::client_id(2),
wsrep::client_state::m_high_priority);
rollback_cc.open(wsrep::client_id(2));
rollback_cc.before_command();
wsrep::mock_high_priority_service rollback_hps(ss, &rollback_cc, false);
rollback_hps.fail_next_remove_fragments_ = true;
size_t const after_apply_calls_before(rollback_hps.after_apply_calls_);
wsrep::ws_meta rollback_meta(
wsrep::gtid(wsrep::id("1"), wsrep::seqno(2)),
wsrep::stid(wsrep::id("1"), wsrep::transaction_id(1),
wsrep::client_id(1)),
wsrep::seqno(1),
wsrep::provider::flag::rollback);
BOOST_REQUIRE(ss.on_apply(rollback_hps, ws_handle, rollback_meta,
wsrep::const_buffer(0, 0)) != 0);
BOOST_REQUIRE_EQUAL(rollback_hps.after_apply_calls_,
after_apply_calls_before + 1);
}