From fae8bd5ed7c62c6fca628b72b2326bb761679a8a Mon Sep 17 00:00:00 2001 From: Kenneth Cain Date: Thu, 4 Sep 2025 12:58:46 -0400 Subject: [PATCH 1/2] DAOS-17281 rebuild: handle admin stop during op:Fail_reclaim Before this change, upon a failed rebuild, both an op:Fail_reclaim and a retry of the original op:Rebuild were scheduled. With this change, the scheduling of the retry of the original op:Rebuild is deferred until the subsequent completion of op:Fail_reclaim. This is done so that if a rebuild stop command happens to arrive during Fail_reclaim, the administrator can stop an endless retry loop of op:Rebuild / Fail_reclaim from otherwise occurring. ds_rebuild_scheudle() now accepts additional arguments specifically interpreted when scheduling op:Fail_reclaim. So that when Fail_reclaim later completes and might schedule a retry of the original op:Rebuild, it has the appropriate pool map version and rebuild opcode to supply to that later ds_rebuild_schedule() call. rebuild_task_complete_schedule() is updated to schedule Fail_reclaim, and detect on its completion if a stop command was received, and decide whether to schedule a retry of original op:Rebuild. Exercise the new stop handling during Fail_reclaim from daos_test -v (simple rebuild, rebuild_many_objects_with_failure test) when run with --rebuild_interactive. This is done via a new fault injection, DAOS_REBUILD_ADMIN_STOP_RECLAIM. Features: rebuild Allow-unstable-test: true Signed-off-by: Kenneth Cain --- src/include/daos/common.h | 1 + src/include/daos_srv/rebuild.h | 3 +- src/pool/srv_pool.c | 4 +- src/rebuild/rebuild_internal.h | 7 +- src/rebuild/srv.c | 209 +++++++++++++++++++------- src/tests/suite/daos_rebuild_common.c | 25 +++ src/tests/suite/daos_rebuild_simple.c | 52 +++++-- src/tests/suite/daos_test.h | 2 + 8 files changed, 232 insertions(+), 71 deletions(-) diff --git a/src/include/daos/common.h b/src/include/daos/common.h index ba8dfc04667..7999879193a 100644 --- a/src/include/daos/common.h +++ b/src/include/daos/common.h @@ -822,6 +822,7 @@ enum { */ #define DAOS_FAIL_PARITY_EPOCH_DIFF (DAOS_FAIL_UNIT_TEST_GROUP_LOC | 0x29) #define DAOS_FAIL_SHARD_NONEXIST (DAOS_FAIL_UNIT_TEST_GROUP_LOC | 0x2a) +#define DAOS_REBUILD_ADMIN_STOP_RECLAIM (DAOS_FAIL_UNIT_TEST_GROUP_LOC | 0x2b) #define DAOS_DTX_COMMIT_SYNC (DAOS_FAIL_UNIT_TEST_GROUP_LOC | 0x30) #define DAOS_DTX_LEADER_ERROR (DAOS_FAIL_UNIT_TEST_GROUP_LOC | 0x31) diff --git a/src/include/daos_srv/rebuild.h b/src/include/daos_srv/rebuild.h index e3f1d8a4794..0965f1f3b4a 100644 --- a/src/include/daos_srv/rebuild.h +++ b/src/include/daos_srv/rebuild.h @@ -86,7 +86,8 @@ typedef enum { int ds_rebuild_schedule(struct ds_pool *pool, uint32_t map_ver, daos_epoch_t stable_eph, uint32_t layout_version, struct pool_target_id_list *tgts, - daos_rebuild_opc_t rebuild_op, bool stop_admin, uint64_t delay_sec); + daos_rebuild_opc_t rebuild_op, daos_rebuild_opc_t retry_rebuild_op, + uint32_t retry_map_ver, bool stop_admin, uint64_t delay_sec); void ds_rebuild_restart_if_rank_wip(uuid_t pool_uuid, d_rank_t rank); int ds_rebuild_query(uuid_t pool_uuid, struct daos_rebuild_status *status); diff --git a/src/pool/srv_pool.c b/src/pool/srv_pool.c index 40b6e0cc127..182d165fbd0 100644 --- a/src/pool/srv_pool.c +++ b/src/pool/srv_pool.c @@ -6238,6 +6238,7 @@ pool_check_upgrade_object_layout(struct rdb_tx *tx, struct pool_svc *svc, if (current_layout_ver < DS_POOL_OBJ_VERSION) { rc = ds_rebuild_schedule(svc->ps_pool, svc->ps_pool->sp_map_version, upgrade_eph, DS_POOL_OBJ_VERSION, NULL, RB_OP_UPGRADE, + RB_OP_NONE /* retry_rebuild_op */, 0 /* retry_map_ver */, false /* stop_admin */, 0); if (rc == 0) *scheduled_layout_upgrade = true; @@ -7554,7 +7555,8 @@ pool_svc_update_map(struct pool_svc *svc, crt_opcode_t opc, bool exclude_rank, if (tgt_map_ver != 0) { rc = ds_rebuild_schedule(svc->ps_pool, tgt_map_ver, rebuild_eph, 0, &target_list, - RB_OP_REBUILD, false /* stop_admin */, delay); + RB_OP_REBUILD, RB_OP_NONE /* retry_rebuild_op */, + 0 /* retry_map_ver */, false /* stop_admin */, delay); if (rc != 0) { D_ERROR("rebuild fails rc: "DF_RC"\n", DP_RC(rc)); D_GOTO(out, rc); diff --git a/src/rebuild/rebuild_internal.h b/src/rebuild/rebuild_internal.h index 2b08c101675..c05af6c02e9 100644 --- a/src/rebuild/rebuild_internal.h +++ b/src/rebuild/rebuild_internal.h @@ -245,8 +245,13 @@ struct rebuild_task { */ uint32_t dst_reclaim_ver; + /* For failed tasks that may be retried after RB_OP_FAIL_RECLAIM, original map ver and opc + */ + uint32_t dst_retry_map_ver; + daos_rebuild_opc_t dst_retry_rebuild_op; + /* For dst_rebuild_op == RB_OP_FAIL_RECLAIM: If true, rebuild was stopped by admin. - * Then, on fail_reclaim finish, the pool rebuild state will be set to completed. + * Then, on fail_reclaim finish, the pool rebuild state will be set to idle (NOT_STARTED). */ bool dst_stop_admin; }; diff --git a/src/rebuild/srv.c b/src/rebuild/srv.c index af0ea5d937c..77b9bf44dd4 100644 --- a/src/rebuild/srv.c +++ b/src/rebuild/srv.c @@ -903,8 +903,13 @@ ds_rebuild_admin_stop(struct ds_pool *pool, uint32_t force) D_INFO(DF_RB ": NOT stopping rebuild during opc %u(%s)\n", DP_RB_RGT(rgt), rgt->rgt_opc, RB_OP_STR(rgt->rgt_opc)); } - rgt_put(rgt); + /* admin stop command does not terminate op:Fail_reclaim, but it is remembered to avoid + * retrying the original op:Rebuild. + */ + if (rgt->rgt_abort || (rgt->rgt_opc == RB_OP_FAIL_RECLAIM)) + rgt->rgt_stop_admin = 1; + rgt_put(rgt); return 0; } @@ -920,9 +925,9 @@ rebuild_leader_status_check(struct ds_pool *pool, uint32_t op, unsigned int total; struct sched_req_attr attr = {0}; d_rank_t myrank; - unsigned int nsleeps = 0; - bool fi_admin_stop_fi = false; - bool fi_admin_stop = false; + unsigned int nsleeps = 0; + bool fi_admin_stop = false; + bool fi_admin_stop_frc = false; int rc; rc = crt_group_size(pool->sp_group, &total); @@ -947,11 +952,12 @@ rebuild_leader_status_check(struct ds_pool *pool, uint32_t op, bool rebuild_abort = false; int i; - if (!fi_admin_stop_fi) - fi_admin_stop_fi = DAOS_FAIL_CHECK(DAOS_REBUILD_ADMIN_STOP); - fi_admin_stop = (fi_admin_stop_fi && (op == RB_OP_REBUILD)); + if (!fi_admin_stop && (op == RB_OP_REBUILD)) + fi_admin_stop = DAOS_FAIL_CHECK(DAOS_REBUILD_ADMIN_STOP); + if (!fi_admin_stop_frc && (op == RB_OP_FAIL_RECLAIM)) + fi_admin_stop_frc = DAOS_FAIL_CHECK(DAOS_REBUILD_ADMIN_STOP_RECLAIM); - if (fi_admin_stop && (nsleeps > 0)) { + if ((fi_admin_stop || fi_admin_stop_frc) && (nsleeps > 0)) { uuid_copy(test_stop_start_puuid, rgt->rgt_pool_uuid); /* What the real RPC handler will call. We will exit below due to rgt_abort @@ -1054,7 +1060,7 @@ rebuild_leader_status_check(struct ds_pool *pool, uint32_t op, sleep: update_and_warn_for_slow_engines(rgt); sched_req_sleep(rgt->rgt_ult, RBLD_CHECK_INTV); - if (fi_admin_stop) + if (fi_admin_stop || fi_admin_stop_frc) nsleeps++; } @@ -1674,12 +1680,12 @@ rebuild_leader_start(struct ds_pool *pool, struct rebuild_task *task, static void retry_rebuild_task(struct rebuild_task *task, struct rebuild_global_pool_tracker *rgt, - daos_rebuild_opc_t *opc) + bool do_map_revert, daos_rebuild_opc_t *opc) { int error = rgt->rgt_status.rs_errno; - int rc; + bool do_log = do_map_revert; - /* Only be called if rebuild task failed */ + /* Only called if task failed. Whether to log is also based on do_map_revert argument */ /* Do not retry if the administrator manually stopped the rebuild */ if (rgt->rgt_stop_admin) { @@ -1692,8 +1698,10 @@ retry_rebuild_task(struct rebuild_task *task, struct rebuild_global_pool_tracker */ if (daos_crt_network_error(error) || error == -DER_TIMEDOUT || error == -DER_GRPVER || error == -DER_STALE || error == -DER_VOS_PARTIAL_UPDATE) { - DL_INFO(error, DF_UUID" opc %u/%u retry", DP_UUID(task->dst_pool_uuid), - task->dst_rebuild_op, task->dst_map_ver); + if (do_log) + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u retry", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, + task->dst_map_ver); *opc = task->dst_rebuild_op; return; } @@ -1701,37 +1709,70 @@ retry_rebuild_task(struct rebuild_task *task, struct rebuild_global_pool_tracker /* Reclaim and exclude task needs to retry indefinitely */ if (task->dst_rebuild_op == RB_OP_RECLAIM || task->dst_rebuild_op == RB_OP_FAIL_RECLAIM) { - DL_INFO(error, DF_UUID" opc %u/%u retry", DP_UUID(task->dst_pool_uuid), - task->dst_rebuild_op, task->dst_map_ver); + if (do_log) + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u retry", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, + task->dst_map_ver); *opc = task->dst_rebuild_op; return; } /* Do not need retry for upgrade */ if (task->dst_rebuild_op == RB_OP_UPGRADE) { - DL_INFO(error, DF_UUID" opc %u/%u, no need to retry", DP_UUID(task->dst_pool_uuid), - task->dst_rebuild_op, task->dst_map_ver); + if (do_log) + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, + DF_UUID " opc %u/%u, no need to retry", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, + task->dst_map_ver); *opc = RB_OP_NONE; return; } - DL_INFO(error, DF_UUID" opc %u/%u, revert pool map", DP_UUID(task->dst_pool_uuid), - task->dst_rebuild_op, task->dst_map_ver); - rc = ds_pool_tgt_revert_rebuild(task->dst_pool_uuid, &task->dst_tgts); - if (rc < 0) - D_ERROR(DF_UUID" revert pool map status: %d\n", - DP_UUID(task->dst_pool_uuid), rc); + if (do_map_revert) { + int rc; + + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u, revert pool map", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); + + rc = ds_pool_tgt_revert_rebuild(task->dst_pool_uuid, &task->dst_tgts); + if (rc < 0) + D_ERROR(DF_UUID " revert pool map status: %d\n", + DP_UUID(task->dst_pool_uuid), rc); + } /* Though there might be DOWN targets in the list needs to retry anyway, * NB: those drain/extend/reintegration targets have been revert to UPIN * or DOWNOUT status, so during retry, rebuild job will be canceled anyway. * Only those exclude targets will keep retry here. */ - DL_INFO(error, DF_UUID" opc %u/%u, retry.", DP_UUID(task->dst_pool_uuid), - task->dst_rebuild_op, task->dst_map_ver); + if (do_log) + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u, retry.", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); *opc = RB_OP_REBUILD; } +static void +check_to_retry_orig_rebuild(struct rebuild_task *task, struct rebuild_global_pool_tracker *rgt) +{ + /* Only called for Fail_reclaim (whether it fails or succeeds) to know if the original + * op:Rebuild should be retried later when Fail_reclaim is (successfully) done. + */ + if (task->dst_rebuild_op != RB_OP_FAIL_RECLAIM) + return; + + /* If this Fail_reclaim was stopped, and the original Rebuild was not stopped, + * we no longer want the original Rebuild to be retried. + */ + if (rgt->rgt_stop_admin && (task->dst_retry_rebuild_op != RB_OP_NONE)) { + D_INFO( + DF_RB + ": rebuild stop command during Fail_reclaim - do NOT retry original %u(%s)\n", + DP_RB_RGT(rgt), task->dst_retry_rebuild_op, + RB_OP_STR(task->dst_retry_rebuild_op)); + task->dst_retry_rebuild_op = RB_OP_NONE; + } +} + static int rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, struct rebuild_global_pool_tracker *rgt, int ret) @@ -1752,8 +1793,9 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, rc = ds_rebuild_schedule(pool, task->dst_map_ver, task->dst_reclaim_eph, task->dst_new_layout_version, &task->dst_tgts, - task->dst_rebuild_op, task->dst_stop_admin, delay_sec); - DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, DF_UUID ": schedule retry opc %u(%s)", + task->dst_rebuild_op, task->dst_retry_rebuild_op, + task->dst_retry_map_ver, task->dst_stop_admin, delay_sec); + DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, DF_UUID ": reschedule not-started %u(%s)", DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, RB_OP_STR(task->dst_rebuild_op)); return rc; @@ -1770,8 +1812,11 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, DP_UUID(task->dst_pool_uuid), DP_RC(rc1)); } + /* see if we got stop during Fail_reclaim, configure whether to retry original rebuild */ + check_to_retry_orig_rebuild(task, rgt); + if (!is_rebuild_global_done(rgt) || rgt->rgt_status.rs_errno != 0) { - daos_rebuild_opc_t retry_opc = 0; + daos_rebuild_opc_t retry_opc = 0; /* If current job failed */ rgt->rgt_status.rs_state = DRS_IN_PROGRESS; @@ -1781,17 +1826,22 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, task->dst_rebuild_op == RB_OP_FAIL_RECLAIM) { rc = ds_rebuild_schedule(pool, task->dst_map_ver, rgt->rgt_stable_epoch, task->dst_new_layout_version, &task->dst_tgts, - task->dst_rebuild_op, task->dst_stop_admin, + task->dst_rebuild_op, task->dst_retry_rebuild_op, + task->dst_retry_map_ver, task->dst_stop_admin, delay_sec); DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, - DF_RB ": errno " DF_RC ", schedule retry", DP_RB_RGT(rgt), - DP_RC(rgt->rgt_status.rs_errno)); + DF_RB ": errno " DF_RC ", schedule retry %u(%s)", DP_RB_RGT(rgt), + DP_RC(rgt->rgt_status.rs_errno), task->dst_rebuild_op, + RB_OP_STR(task->dst_rebuild_op)); return rc; } - /* Schedule fail_reclaim to clean up current op. Retry original rebuild, even if - * this fails. However, do not retry the original rebuild if the admin has stopped - * it. + /* Determine if original rebuild needs a retry (don't revert pool map yet - see + * below) */ + retry_rebuild_task(task, rgt, false /* do_map_revert */, &retry_opc); + + /* Schedule fail_reclaim to clean up current op. After fail_reclaim, we may retry + * original. Scheduling any retry is deferred until fail_reclaim is actually done. */ if (rgt->rgt_init_scan) { /* NB: dst_reclaim_ver is the minimum rebuild target version, once rebuild @@ -1799,25 +1849,36 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, * (reclaim - 1 see obj_reclaim()), but keep the in-flight I/O data. */ rc = ds_rebuild_schedule( - pool, task->dst_reclaim_ver - 1, rgt->rgt_stable_epoch, + pool, task->dst_reclaim_ver - 1 /* map_ver */, rgt->rgt_stable_epoch, task->dst_new_layout_version, &task->dst_tgts, RB_OP_FAIL_RECLAIM, + retry_opc /* retry_rebuild_op */, task->dst_map_ver /* retry_map_ver */, rgt->rgt_stop_admin, delay_sec); DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, - DF_RB ": errno " DF_RC ", schedule %u(%s)", DP_RB_RGT(rgt), - DP_RC(rgt->rgt_status.rs_errno), RB_OP_FAIL_RECLAIM, - RB_OP_STR(RB_OP_FAIL_RECLAIM)); + DF_RB ": errno " DF_RC + ", schedule %u(%s) and %s retry of original %u(%s)", + DP_RB_RGT(rgt), DP_RC(rgt->rgt_status.rs_errno), + RB_OP_FAIL_RECLAIM, RB_OP_STR(RB_OP_FAIL_RECLAIM), + (retry_opc == RB_OP_NONE) ? "do NOT schedule" : "defer schedule", + task->dst_rebuild_op, RB_OP_STR(task->dst_rebuild_op)); + D_GOTO(complete, rc); } - /* Then check if it needs to retry */ - retry_rebuild_task(task, rgt, &retry_opc); + /* Determine if original rebuild needs a retry, and if so revert pool map. */ + retry_rebuild_task(task, rgt, true /* do_map_revert */, &retry_opc); + + /* Schedule a retry if needed (NB: possibly deferred until after a Fail_reclaim is + * done) */ if (retry_opc == RB_OP_NONE) D_GOTO(complete, rc); rc = ds_rebuild_schedule(pool, task->dst_map_ver, rgt->rgt_stable_epoch, - task->dst_new_layout_version, &task->dst_tgts, retry_opc, - false /* stop_admin */, delay_sec); - DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, DF_RB ": errno " DF_RC ", schedule retry", - DP_RB_RGT(rgt), DP_RC(rgt->rgt_status.rs_errno)); + task->dst_new_layout_version, &task->dst_tgts, + retry_opc /* rebuild_op*/, 0 /* retry_rebuild_op */, + 0 /* retry_map_ver */, false /* stop_admin */, delay_sec); + DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, + DF_RB ": errno " DF_RC ", schedule retry of original %u(%s)", + DP_RB_RGT(rgt), DP_RC(rgt->rgt_status.rs_errno), retry_opc, + RB_OP_STR(retry_opc)); } else if (task->dst_rebuild_op == RB_OP_REBUILD || task->dst_rebuild_op == RB_OP_UPGRADE) { /* Schedule reclaim for successful op:Rebuild * (exclude/drain/reintegrate/extend/upgrade). */ @@ -1825,7 +1886,8 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, rc = ds_rebuild_schedule(pool, task->dst_map_ver, rgt->rgt_reclaim_epoch, task->dst_new_layout_version, &task->dst_tgts, - RB_OP_RECLAIM, false /* stop_admin */, delay_sec); + RB_OP_RECLAIM, RB_OP_NONE /* retry_rebuild_op */, + 0 /* retry_map_ver */, false /* stop_admin */, delay_sec); DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, DF_RB ": errno " DF_RC ", schedule %u(%s)", DP_RB_RGT(rgt), DP_RC(rgt->rgt_status.rs_errno), RB_OP_RECLAIM, RB_OP_STR(RB_OP_RECLAIM)); @@ -1834,13 +1896,21 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, complete: /* Update the rebuild complete status for pool query */ if (task->dst_rebuild_op != RB_OP_FAIL_RECLAIM) { + /* update pool query rebuild status: original job is done. + * if op:Reclaim scheduled --> DRS_IN_PROGRESS, otherwise DRS_COMPLETED + */ rc1 = rebuild_status_completed_update(task->dst_pool_uuid, &rgt->rgt_status); DL_CDEBUG(rc1, DLOG_ERR, DLOG_INFO, rc1, DF_RB ": updated, state %d errno " DF_RC, DP_RB_RGT(rgt), rgt->rgt_status.rs_state, DP_RC(rgt->rgt_status.rs_errno)); - } else if (task->dst_rebuild_op == RB_OP_FAIL_RECLAIM && task->dst_stop_admin) { - /* Fail_reclaim normally does not update status - except in the manual admin stop - * case */ + } else if (task->dst_rebuild_op == RB_OP_FAIL_RECLAIM && + (task->dst_stop_admin || rgt->rgt_stop_admin)) { + /* update pool query rebuild status: either the original job was stopped, + * or a stop command was received during Fail_reclaim */ + if (rgt->rgt_stop_admin) + D_INFO(DF_RB ": stop received during Fail_reclaim, change errno=%d to %d\n", + DP_RB_RGT(rgt), rgt->rgt_status.rs_errno, -DER_OP_CANCELED); + rgt->rgt_status.rs_errno = -DER_OP_CANCELED; rgt->rgt_status.rs_state = DRS_NOT_STARTED; rc1 = rebuild_status_completed_update_partial( @@ -1848,6 +1918,19 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, DL_CDEBUG(rc1, DLOG_ERR, DLOG_INFO, rc1, DF_RB ": updated, state %d errno " DF_RC, DP_RB_RGT(rgt), rgt->rgt_status.rs_state, DP_RC(rgt->rgt_status.rs_errno)); + } else if ((task->dst_rebuild_op == RB_OP_FAIL_RECLAIM) && + (task->dst_retry_rebuild_op != RB_OP_NONE)) { + /* Fail_reclaim done (and a stop command wasn't received during) - retry original + * rebuild */ + rc1 = ds_rebuild_schedule(pool, task->dst_retry_map_ver, rgt->rgt_reclaim_epoch, + task->dst_new_layout_version, &task->dst_tgts, + task->dst_retry_rebuild_op, + RB_OP_NONE /* retry_rebuild_op */, 0 /* retry_map_ver */, + false /* stop_admin */, delay_sec); + DL_CDEBUG(rc1, DLOG_ERR, DLOG_INFO, rc, + DF_RB ": errno " DF_RC ", schedule retry %u(%s)", DP_RB_RGT(rgt), + DP_RC(rgt->rgt_status.rs_errno), task->dst_retry_rebuild_op, + RB_OP_STR(task->dst_retry_rebuild_op)); } if ((rc1 != 0) && (rc == 0)) @@ -2284,7 +2367,8 @@ rebuild_print_list_update(const uuid_t uuid, const uint32_t map_ver, int ds_rebuild_schedule(struct ds_pool *pool, uint32_t map_ver, daos_epoch_t reclaim_eph, uint32_t layout_version, struct pool_target_id_list *tgts, - daos_rebuild_opc_t rebuild_op, bool stop_admin, uint64_t delay_sec) + daos_rebuild_opc_t rebuild_op, daos_rebuild_opc_t retry_rebuild_op, + uint32_t retry_map_ver, bool stop_admin, uint64_t delay_sec) { struct rebuild_task *new_task; struct rebuild_task *task; @@ -2293,6 +2377,7 @@ ds_rebuild_schedule(struct ds_pool *pool, uint32_t map_ver, daos_epoch_t reclaim uint64_t cur_ts = 0; D_ASSERT(dss_get_module_info()->dmi_xs_id == 0); + D_ASSERT(rebuild_op != RB_OP_NONE); if (pool->sp_stopping) { D_DEBUG(DB_REBUILD, DF_UUID " is stopping, do not need schedule here\n", DP_UUID(pool->sp_uuid)); @@ -2330,7 +2415,15 @@ ds_rebuild_schedule(struct ds_pool *pool, uint32_t map_ver, daos_epoch_t reclaim new_task->dst_rebuild_op = rebuild_op; new_task->dst_reclaim_eph = reclaim_eph; new_task->dst_new_layout_version = layout_version; - new_task->dst_stop_admin = (rebuild_op == RB_OP_FAIL_RECLAIM) ? stop_admin : false; + if (rebuild_op == RB_OP_FAIL_RECLAIM) { + new_task->dst_retry_map_ver = retry_map_ver; + new_task->dst_retry_rebuild_op = retry_rebuild_op; + new_task->dst_stop_admin = stop_admin; + } else { + new_task->dst_retry_map_ver = 0; + new_task->dst_retry_rebuild_op = RB_OP_NONE; + new_task->dst_stop_admin = false; + } uuid_copy(new_task->dst_pool_uuid, pool->sp_uuid); D_INIT_LIST_HEAD(&new_task->dst_list); @@ -2417,14 +2510,16 @@ regenerate_task_internal(struct ds_pool *pool, struct pool_target *tgts, id_list.pti_number = 1; if (tgt->ta_comp.co_status & (PO_COMP_ST_DOWN | PO_COMP_ST_DRAIN)) { - rc = ds_rebuild_schedule(pool, tgt->ta_comp.co_fseq, - current_eph == 0 ? eph : current_eph, 0, &id_list, - RB_OP_REBUILD, false /* stop_admin */, delay); + rc = ds_rebuild_schedule( + pool, tgt->ta_comp.co_fseq, current_eph == 0 ? eph : current_eph, 0, + &id_list, RB_OP_REBUILD, RB_OP_NONE /* retry_rebuild_op */, + 0 /* retry_map_ver */, false /* stop_admin */, delay); } else { D_ASSERT(tgt->ta_comp.co_status == PO_COMP_ST_UP); - rc = ds_rebuild_schedule(pool, tgt->ta_comp.co_in_ver, - current_eph == 0 ? eph : current_eph, 0, &id_list, - RB_OP_REBUILD, false /* stop_admin */, delay); + rc = ds_rebuild_schedule( + pool, tgt->ta_comp.co_in_ver, current_eph == 0 ? eph : current_eph, 0, + &id_list, RB_OP_REBUILD, RB_OP_NONE /* retry_rebuild_op */, + 0 /* retry_map_ver */, false /* stop_admin */, delay); } if (rc) { D_ERROR(DF_UUID" schedule ver %d failed: "DF_RC"\n", diff --git a/src/tests/suite/daos_rebuild_common.c b/src/tests/suite/daos_rebuild_common.c index f65f0ad4f41..5ecd1e0439d 100644 --- a/src/tests/suite/daos_rebuild_common.c +++ b/src/tests/suite/daos_rebuild_common.c @@ -1207,6 +1207,31 @@ fi_rebuild_stop(void *data) return 0; } +/* inject fault on leader engine to receive stop command during op:Fail_reclaim */ +int +fi_rebuild_stop_reclaim(void *data) +{ + test_arg_t *arg = data; + d_rank_t leader; + int rc; + + if (arg->myrank != 0) + return 0; + + rc = test_get_leader(arg, &leader); + if (rc) { + print_message("get pool " DF_UUIDF " leader failed: %d\n", + DP_UUID(arg->pool.pool_uuid), rc); + assert_rc_equal(rc, 0); + } + + printf("%s(): pool " DF_UUIDF ": send ADMIN_STOP_RECLAIM to leader rank %d\n", __FUNCTION__, + DP_UUID(arg->pool.pool_uuid), leader); + test_set_engine_fail_loc(arg, leader, DAOS_REBUILD_ADMIN_STOP_RECLAIM | DAOS_FAIL_ONCE); + + return 0; +} + /* resume after a stopped rebuild, and optionally inject a fault to start a new rebuild */ int fi_rebuild_resume_wait(void *data) diff --git a/src/tests/suite/daos_rebuild_simple.c b/src/tests/suite/daos_rebuild_simple.c index 85c05dc41ad..6b95dc09047 100644 --- a/src/tests/suite/daos_rebuild_simple.c +++ b/src/tests/suite/daos_rebuild_simple.c @@ -1475,9 +1475,11 @@ rebuild_wait_reset_fail_cb(void *data) { test_arg_t *arg = data; - print_message("wait 300 seconds for rebuild/reclaim/retry...."); + print_message("wait 60 seconds for rebuild/reclaim%s\n", + arg->interactive_rebuild ? "" : "/retry"); sleep(60); + print_message("clearing fault injection on all engines\n"); daos_debug_set_params(arg->group, -1, DMG_KEY_FAIL_LOC, 0, 0, NULL); daos_debug_set_params(arg->group, -1, DMG_KEY_FAIL_VALUE, 0, 0, NULL); daos_debug_set_params(arg->group, -1, DMG_KEY_FAIL_NUM, 0, 0, NULL); @@ -1496,11 +1498,6 @@ rebuild_many_objects_with_failure(void **state) if (!test_runable(arg, 6)) return; - if (arg->interactive_rebuild) { - print_message("SKIP due to interactive_rebuild enabled, but not tested here\n"); - skip(); - } - D_ALLOC_ARRAY(oids, 8000); for (i = 0; i < 8000; i++) { char buffer[256]; @@ -1517,15 +1514,48 @@ rebuild_many_objects_with_failure(void **state) ioreq_fini(&req); } + /* Inject faults on engines. Special handling for interactive_rebuild case */ if (arg->myrank == 0) { - daos_debug_set_params(arg->group, -1, DMG_KEY_FAIL_LOC, - DAOS_REBUILD_OBJ_FAIL | DAOS_FAIL_ALWAYS, 0, NULL); - daos_debug_set_params(arg->group, -1, DMG_KEY_FAIL_VALUE, 50, - 0, NULL); + if (!arg->interactive_rebuild) { + /* All engines DAOS_REBUILD_OBJ_FAIL */ + print_message("inject fault DAOS_REBUILD_OBJ_FAIL on all engines\n"); + daos_debug_set_params(arg->group, -1, DMG_KEY_FAIL_LOC, + DAOS_REBUILD_OBJ_FAIL | DAOS_FAIL_ALWAYS, 0, NULL); + daos_debug_set_params(arg->group, -1, DMG_KEY_FAIL_VALUE, 50, 0, NULL); + } else { + d_rank_t leader; + + /* interactive_rebiuld: DAOS_REBUILD_OBJ fail on all engines except PS + * leader; DAOS_REBUILD_ADMIN_STOP_RECLAIM on the PS leader engine + */ + + rc = test_get_leader(arg, &leader); + if (rc != 0) { + print_message("get pool " DF_UUIDF " leader failed: %d\n", + DP_UUID(arg->pool.pool_uuid), rc); + assert_rc_equal(rc, 0); + } + + for (i = 0; i < arg->srv_nnodes; i++) { + if (i == leader) + fi_rebuild_stop_reclaim(arg); + else { + print_message("inject DAOS_REBUILD_OBJ_FAIL on non-leader " + "engine %u\n", + i); + daos_debug_set_params( + arg->group, i, DMG_KEY_FAIL_LOC, + DAOS_REBUILD_OBJ_FAIL | DAOS_FAIL_ALWAYS, 0, NULL); + daos_debug_set_params(arg->group, i, DMG_KEY_FAIL_VALUE, 50, + 0, NULL); + } + } + } } arg->rebuild_cb = rebuild_wait_reset_fail_cb; - + if (arg->interactive_rebuild) + arg->rebuild_post_cb = fi_rebuild_resume_wait; rebuild_single_pool_target(arg, 3, -1, false); for (i = 0; i < 8000; i++) { diff --git a/src/tests/suite/daos_test.h b/src/tests/suite/daos_test.h index f7ddeb7ff84..97a56bdec7d 100644 --- a/src/tests/suite/daos_test.h +++ b/src/tests/suite/daos_test.h @@ -470,6 +470,8 @@ int rebuild_sub_setup_common(void **state, daos_size_t pool_size, int node_nr, u int fi_rebuild_stop(void *data); +int +fi_rebuild_stop_reclaim(void *data); int fi_rebuild_resume_wait(void *data); From 10a2d3fdde2fef2e8f2fba87c30f54c767873759 Mon Sep 17 00:00:00 2001 From: Kenneth Cain Date: Wed, 10 Sep 2025 21:47:57 -0400 Subject: [PATCH 2/2] Address review comments and candidate test fix daos_test -B --subtests="15" EXTEND16 failure Features: rebuild Allow-unstable-test: true Signed-off-by: Kenneth Cain --- src/rebuild/srv.c | 63 +++++++++++++++++------------------------------ 1 file changed, 23 insertions(+), 40 deletions(-) diff --git a/src/rebuild/srv.c b/src/rebuild/srv.c index c5373de83a2..415b7df791f 100644 --- a/src/rebuild/srv.c +++ b/src/rebuild/srv.c @@ -898,7 +898,6 @@ ds_rebuild_admin_stop(struct ds_pool *pool, uint32_t force) D_INFO(DF_RB ": stopping rebuild force=%u opc %u(%s)\n", DP_RB_RGT(rgt), force, rgt->rgt_opc, RB_OP_STR(rgt->rgt_opc)); rgt->rgt_abort = 1; - rgt->rgt_stop_admin = 1; rgt->rgt_status.rs_errno = -DER_OP_CANCELED; } else { D_INFO(DF_RB ": NOT stopping rebuild during opc %u(%s)\n", DP_RB_RGT(rgt), @@ -1681,10 +1680,10 @@ rebuild_leader_start(struct ds_pool *pool, struct rebuild_task *task, static void retry_rebuild_task(struct rebuild_task *task, struct rebuild_global_pool_tracker *rgt, - bool do_map_revert, daos_rebuild_opc_t *opc) + daos_rebuild_opc_t *opc) { + int rc; int error = rgt->rgt_status.rs_errno; - bool do_log = do_map_revert; /* Only called if task failed. Whether to log is also based on do_map_revert argument */ @@ -1699,10 +1698,8 @@ retry_rebuild_task(struct rebuild_task *task, struct rebuild_global_pool_tracker */ if (daos_crt_network_error(error) || error == -DER_TIMEDOUT || error == -DER_GRPVER || error == -DER_STALE || error == -DER_VOS_PARTIAL_UPDATE) { - if (do_log) - DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u retry", - DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, - task->dst_map_ver); + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u retry", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); *opc = task->dst_rebuild_op; return; } @@ -1710,45 +1707,34 @@ retry_rebuild_task(struct rebuild_task *task, struct rebuild_global_pool_tracker /* Reclaim and exclude task needs to retry indefinitely */ if (task->dst_rebuild_op == RB_OP_RECLAIM || task->dst_rebuild_op == RB_OP_FAIL_RECLAIM) { - if (do_log) - DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u retry", - DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, - task->dst_map_ver); + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u retry", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); *opc = task->dst_rebuild_op; return; } /* Do not need retry for upgrade */ if (task->dst_rebuild_op == RB_OP_UPGRADE) { - if (do_log) - DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, - DF_UUID " opc %u/%u, no need to retry", - DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, - task->dst_map_ver); + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u, no need to retry", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); *opc = RB_OP_NONE; return; } - if (do_map_revert) { - int rc; + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u, revert pool map", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); - DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u, revert pool map", - DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); - - rc = ds_pool_tgt_revert_rebuild(task->dst_pool_uuid, &task->dst_tgts); - if (rc < 0) - D_ERROR(DF_UUID " revert pool map status: %d\n", - DP_UUID(task->dst_pool_uuid), rc); - } + rc = ds_pool_tgt_revert_rebuild(task->dst_pool_uuid, &task->dst_tgts); + if (rc < 0) + D_ERROR(DF_UUID " revert pool map status: %d\n", DP_UUID(task->dst_pool_uuid), rc); /* Though there might be DOWN targets in the list needs to retry anyway, * NB: those drain/extend/reintegration targets have been revert to UPIN * or DOWNOUT status, so during retry, rebuild job will be canceled anyway. * Only those exclude targets will keep retry here. */ - if (do_log) - DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u, retry.", - DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); + DL_CDEBUG(error, DLOG_ERR, DLOG_INFO, error, DF_UUID " opc %u/%u, retry.", + DP_UUID(task->dst_pool_uuid), task->dst_rebuild_op, task->dst_map_ver); *opc = RB_OP_REBUILD; } @@ -1837,10 +1823,6 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, return rc; } - /* Determine if original rebuild needs a retry (don't revert pool map yet - see - * below) */ - retry_rebuild_task(task, rgt, false /* do_map_revert */, &retry_opc); - /* Schedule fail_reclaim to clean up current op. After fail_reclaim, we may retry * original. Scheduling any retry is deferred until fail_reclaim is actually done. */ @@ -1855,17 +1837,18 @@ rebuild_task_complete_schedule(struct rebuild_task *task, struct ds_pool *pool, retry_opc /* retry_rebuild_op */, task->dst_map_ver /* retry_map_ver */, rgt->rgt_stop_admin, delay_sec); DL_CDEBUG(rc, DLOG_ERR, DLOG_INFO, rc, - DF_RB ": errno " DF_RC - ", schedule %u(%s) and %s retry of original %u(%s)", - DP_RB_RGT(rgt), DP_RC(rgt->rgt_status.rs_errno), - RB_OP_FAIL_RECLAIM, RB_OP_STR(RB_OP_FAIL_RECLAIM), - (retry_opc == RB_OP_NONE) ? "do NOT schedule" : "defer schedule", - task->dst_rebuild_op, RB_OP_STR(task->dst_rebuild_op)); + DF_RB ": errno " DF_RC ", schedule %u(%s)", DP_RB_RGT(rgt), + DP_RC(rgt->rgt_status.rs_errno), RB_OP_FAIL_RECLAIM, + RB_OP_STR(RB_OP_FAIL_RECLAIM)); + + /* revert pool map and defer scheduling a retry until Fail_reclaim is done + */ + retry_rebuild_task(task, rgt, &retry_opc); D_GOTO(complete, rc); } /* Determine if original rebuild needs a retry, and if so revert pool map. */ - retry_rebuild_task(task, rgt, true /* do_map_revert */, &retry_opc); + retry_rebuild_task(task, rgt, &retry_opc); /* Schedule a retry if needed (NB: possibly deferred until after a Fail_reclaim is * done) */