From cdc730314bf288cb1dfe039e1714ebc6695f7add Mon Sep 17 00:00:00 2001 From: Vitaliy Filippov Date: Fri, 19 Jun 2026 01:49:26 +0300 Subject: [PATCH] Fix EC PGs hanging in REPEERING when flush error is last in the batch --- src/osd/osd_flush.cpp | 19 +++++++---- src/osd/osd_peering_pg.h | 2 +- src/test/osd_test.cpp | 72 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/src/osd/osd_flush.cpp b/src/osd/osd_flush.cpp index 63dd993f..4b5e6f24 100644 --- a/src/osd/osd_flush.cpp +++ b/src/osd/osd_flush.cpp @@ -79,9 +79,9 @@ void osd_t::handle_flush_op(bool rollback, pool_id_t pool_id, pg_num_t pg_num, p // Throw the result away return; } - fb->flush_done++; if (retval != 0) { + fb->flush_errors++; if (peer_osd == this->osd_num) { throw std::runtime_error( @@ -97,17 +97,26 @@ void osd_t::handle_flush_op(bool rollback, pool_id_t pool_id, pg_num_t pg_num, p auto peer_it = msgr.osd_peers.find(peer_osd); if (peer_it != msgr.osd_peers.end()) { - // Will repeer/stop this PG + // stop_client won't repeer the PG because flush_batch is not deleted yet msgr.stop_client(peer_it->second->client_id); - return; } } } + auto errors = fb->flush_errors; + fb->flush_done++; if (fb->flush_done == fb->flush_ops) { // This flush batch is done - std::vector continue_ops; auto & pg = pgs.at(pg_id); + delete fb; + pg.flush_batch = NULL; + if (errors > 0) + { + // Repeer the PG on errors + repeer_pg(pg); + return; + } + std::vector continue_ops; auto it = pg.flush_actions.begin(), prev_it = it; while (1) { @@ -143,8 +152,6 @@ void osd_t::handle_flush_op(bool rollback, pool_id_t pool_id, pg_num_t pg_num, p } prev_it = it++; } - delete fb; - pg.flush_batch = NULL; if (!pg.flush_actions.size()) { pg.state = pg.state & ~PG_HAS_UNCLEAN; diff --git a/src/osd/osd_peering_pg.h b/src/osd/osd_peering_pg.h index 9c98e91a..017a161f 100644 --- a/src/osd/osd_peering_pg.h +++ b/src/osd/osd_peering_pg.h @@ -78,7 +78,7 @@ struct pg_flush_batch_t { std::map> rollback_lists; std::map> stable_lists; - int flush_ops = 0, flush_done = 0; + int flush_ops = 0, flush_done = 0, flush_errors = 0; int flush_objects = 0; }; diff --git a/src/test/osd_test.cpp b/src/test/osd_test.cpp index 6b9a516e..fbdd6836 100644 --- a/src/test/osd_test.cpp +++ b/src/test/osd_test.cpp @@ -1138,6 +1138,77 @@ void test_chained_read_eio_retry() printf("test_chained_read_eio_retry passed\n"); } +// Regression test for the bug in osd_flush.cpp where a remote +// stabilize/rollback failure permanently hangs the PG in PG_REPEERING. +bool test_flush_error_pg_repeer(int ctr) +{ + printf("test_flush_error_pg_repeer\n"); + + osd_test_fixture_t f; + f.configure_ec_pool(2, 1); + f.start(json11::Json::object { + { "osd_num", 2 }, + { "etcd_address", "127.0.0.1:2379" }, + { "immediate_commit", "all" }, + { "block_size", 131072 }, + { "bitmap_granularity", 4096 }, + }); + f.connect_peer(1); + f.connect_peer(3); + + // Object exists on all osds but unstable on osd 1 and 3 + inode_t ino = INODE_WITH_POOL(1, 1); + f.reply_peer_list(1, { { { ino, 0 }, 1 } }, /*stable_count*/ 0); + f.reply_local_list({ { { ino, 1 }, 1 } }, /*stable_count*/ 1); + f.reply_peer_list(3, { { { ino, 2 }, 1 } }, /*stable_count*/ 0); + f.pump(); + + auto & pg = f.pg(1, 1); + assert(pg.state == (PG_ACTIVE|PG_HAS_UNCLEAN)); + auto fb = pg.flush_batch; + assert(fb != nullptr); + assert(fb->flush_ops == 2); + + if (ctr == 0) + { + // Complete OP_SEC_STAB on osd 1, then fail on osd 3 + auto *stab = f.peer_take(1, OSD_OP_SEC_STABILIZE); + stab->reply.hdr.retval = 0; + stab->callback(stab); + + assert(pg.state == (PG_ACTIVE|PG_HAS_UNCLEAN)); + + stab = f.peer_take(3, OSD_OP_SEC_STABILIZE); + stab->reply.hdr.retval = -EPIPE; + stab->callback(stab); + } + else + { + // Fail OP_SEC_STAB on osd 3, then complete on osd 1 + auto *stab = f.peer_take(3, OSD_OP_SEC_STABILIZE); + stab->reply.hdr.retval = -EPIPE; + stab->callback(stab); + + // The PG should only repeer when the batch is completed + assert(pg.state == (PG_HAS_UNCLEAN|PG_REPEERING)); + assert(pg.flush_batch == fb); + + stab = f.peer_take(1, OSD_OP_SEC_STABILIZE); + stab->reply.hdr.retval = 0; + stab->callback(stab); + } + + // Now the PG should be peering + assert(pg.state == PG_PEERING); + assert(!pg.flush_batch); + + // FIXME: Make this test pass without leaks without the following line: + f.complete_peering_empty(); + + printf("test_flush_error_pg_repeer[%d] passed\n", ctr); + return ctr < 1; +} + int main(int narg, char *args[]) { test_load_global_config(); @@ -1149,5 +1220,6 @@ int main(int narg, char *args[]) test_ec42_write_parityless(); test_ec33_chain_read_phantom_bitmap_source(); test_chained_read_eio_retry(); + for (int i = 0; test_flush_error_pg_repeer(i); i++) {} return 0; }