Skip to content

Commit

Permalink
DAOS-13047 chk: misc fixes for chk related issues - X
Browse files Browse the repository at this point in the history 
Mainly contains the following fixes:

1. Do not start pool service for orphan pool after check.

2. Enable CR25 to test "for-all" option for check repair.

3. Some cleanup for former review feedback.

Signed-off-by: Fan Yong <[email protected]>
  • Loading branch information
@Nasf-Fan
Nasf-Fan committed Sep 12, 2023
1 parent b33e84d commit 0086f2a
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 138 deletions.
25 changes: 23 additions & 2 deletions src/chk/chk_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,8 @@ chk_pool_stop_one(struct chk_instance *ins, uuid_t uuid, int status, uint32_t ph
}

if (!ins->ci_is_leader &&
cpr->cpr_bk.cb_pool_status != CHK__CHECK_POOL_STATUS__CPS_CHECKED)
(cpr->cpr_bk.cb_pool_status != CHK__CHECK_POOL_STATUS__CPS_CHECKED ||
cpr->cpr_not_export_ps))
chk_pool_shutdown(cpr, false);

/* Drop the reference that is held when create in chk_pool_alloc(). */
Expand All @@ -509,6 +510,26 @@ chk_pool_stop_one(struct chk_instance *ins, uuid_t uuid, int status, uint32_t ph
*ret = rc;
}

void
chk_pool_stop_all(struct chk_instance *ins, uint32_t status, int *ret)
{
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;

/*
* Hold reference on each before stop one to guarantee that the next
* 'tmp' will not be unlinked from the list during stop current cpr.
*/
d_list_for_each_entry(cpr, &ins->ci_pool_list, cpr_link)
chk_pool_get(cpr);

d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link) {
if (ret == NULL || *ret == 0)
chk_pool_stop_one(ins, cpr->cpr_uuid, status, CHK_INVAL_PHASE, ret);
chk_pool_put(cpr);
}
}

int
chk_pools_pause_cb(struct sys_db *db, char *table, d_iov_t *key, void *args)
{
Expand Down Expand Up @@ -792,7 +813,7 @@ chk_pool_handle_notify(struct chk_instance *ins, struct chk_iv *iv)
rc = chk_bk_update_pool(cbk, uuid_str);
}

if (rc == 0 && !ins->ci_is_leader && !cpr->cpr_destroyed &&
if (rc == 0 && !ins->ci_is_leader && !cpr->cpr_destroyed && !cpr->cpr_not_export_ps &&
cbk->cb_pool_status == CHK__CHECK_POOL_STATUS__CPS_CHECKED)
chk_pool_start_svc(cpr, NULL);

Expand Down
55 changes: 27 additions & 28 deletions src/chk/chk_engine.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,13 +167,10 @@ chk_engine_exit(struct chk_instance *ins, uint32_t ins_phase, uint32_t ins_statu
uint32_t pool_status)
{
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
struct chk_iv iv = { 0 };
int rc;

d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link)
chk_pool_stop_one(ins, cpr->cpr_uuid, pool_status, CHK_INVAL_PHASE, NULL);
chk_pool_stop_all(ins, pool_status, NULL);

chk_destroy_pending_tree(ins);
chk_destroy_pool_tree(ins);
Expand Down Expand Up @@ -1796,13 +1793,17 @@ chk_engine_pool_ult(void *args)
rc1 = chk_bk_update_pool(cbk, uuid_str);

if (cbk->cb_pool_status == CHK__CHECK_POOL_STATUS__CPS_CHECKED) {
chk_pool_start_svc(cpr, &rc2);
if (cpr->cpr_started && cpr->cpr_start_post)
if (cpr->cpr_not_export_ps) {
chk_pool_shutdown(cpr, false);
} else {
chk_pool_start_svc(cpr, &rc2);
/*
* The pool may has been marked as non-connectable before
* corruption, re-enable it to allow new connection.
*/
rc2 = ds_pool_mark_connectable(svc);
if (cpr->cpr_started && cpr->cpr_start_post)
rc2 = ds_pool_mark_connectable(svc);
}
}
}

Expand All @@ -1822,6 +1823,8 @@ chk_engine_sched(void *args)
{
struct chk_instance *ins = args;
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
uint32_t ins_phase;
uint32_t ins_status;
uint32_t pool_status;
Expand Down Expand Up @@ -1868,6 +1871,14 @@ chk_engine_sched(void *args)
if (rc != 0)
goto out;
}

d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link) {
if (cpr->cpr_done && cpr->cpr_started && cpr->cpr_not_export_ps) {
chk_pool_get(cpr);
chk_pool_shutdown(cpr, false);
chk_pool_put(cpr);
}
}
}

out:
Expand Down Expand Up @@ -2128,8 +2139,6 @@ chk_engine_start(uint64_t gen, uint32_t rank_nr, d_rank_t *ranks, uint32_t polic
{
struct chk_instance *ins = chk_engine;
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
struct umem_attr uma = { 0 };
char uuid_str[DAOS_UUID_STR_SIZE];
d_rank_t myrank = dss_self_rank();
Expand Down Expand Up @@ -2219,9 +2228,7 @@ chk_engine_start(uint64_t gen, uint32_t rank_nr, d_rank_t *ranks, uint32_t polic
goto out_done;

out_stop:
d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link)
chk_pool_stop_one(ins, cpr->cpr_uuid, CHK__CHECK_POOL_STATUS__CPS_IMPLICATED,
CHK_INVAL_PHASE, NULL);
chk_pool_stop_all(ins, CHK__CHECK_POOL_STATUS__CPS_IMPLICATED, NULL);
if (cbk->cb_ins_status == CHK__CHECK_INST_STATUS__CIS_RUNNING) {
cbk->cb_time.ct_stop_time = time(NULL);
cbk->cb_ins_status = CHK__CHECK_INST_STATUS__CIS_FAILED;
Expand Down Expand Up @@ -2265,8 +2272,6 @@ chk_engine_stop(uint64_t gen, int pool_nr, uuid_t pools[], uint32_t *flags)
{
struct chk_instance *ins = chk_engine;
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
d_rank_t myrank = dss_self_rank();
int rc = 0;
int i;
Expand All @@ -2291,12 +2296,9 @@ chk_engine_stop(uint64_t gen, int pool_nr, uuid_t pools[], uint32_t *flags)
D_INFO(DF_ENGINE" stopping on rank %u with %d pools\n", DP_ENGINE(ins), myrank, pool_nr);

if (pool_nr == 0) {
d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link) {
chk_pool_stop_one(ins, cpr->cpr_uuid, CHK__CHECK_POOL_STATUS__CPS_STOPPED,
CHK_INVAL_PHASE, &rc);
if (rc != 0)
D_GOTO(out, rc);
}
chk_pool_stop_all(ins, CHK__CHECK_POOL_STATUS__CPS_STOPPED, &rc);
if (rc != 0)
D_GOTO(out, rc);
} else {
for (i = 0; i < pool_nr; i++) {
chk_pool_stop_one(ins, pools[i], CHK__CHECK_POOL_STATUS__CPS_STOPPED,
Expand Down Expand Up @@ -2630,9 +2632,7 @@ chk_engine_act(uint64_t gen, uint64_t seq, uint32_t cla, uint32_t act, uint32_t

if (likely(prop->cp_policies[cla] != act)) {
prop->cp_policies[cla] = act;
rc = chk_prop_update(prop, NULL);
if (rc != 0)
goto out;
chk_prop_update(prop, NULL);
}

/*
Expand Down Expand Up @@ -2884,6 +2884,9 @@ chk_engine_pool_start(uint64_t gen, uuid_t uuid, uint32_t phase, uint32_t flags)
if (unlikely(cpr->cpr_started))
D_GOTO(out, rc = -DER_ALREADY);

if (flags & CPSF_NOT_EXPORT_PS)
cpr->cpr_not_export_ps = 1;

cbk = &cpr->cpr_bk;
chk_pool_get(cpr);

Expand Down Expand Up @@ -3240,8 +3243,6 @@ chk_engine_rejoin(void *args)
struct chk_instance *ins = chk_engine;
struct chk_property *prop = &ins->ci_prop;
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
uuid_t *pools = NULL;
struct chk_iv iv = { 0 };
struct umem_attr uma = { 0 };
Expand Down Expand Up @@ -3349,9 +3350,7 @@ chk_engine_rejoin(void *args)
goto out_log;

out_stop:
d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link)
chk_pool_stop_one(ins, cpr->cpr_uuid, CHK__CHECK_POOL_STATUS__CPS_IMPLICATED,
CHK_INVAL_PHASE, NULL);
chk_pool_stop_all(ins, CHK__CHECK_POOL_STATUS__CPS_IMPLICATED, NULL);
if (cbk->cb_ins_status == CHK__CHECK_INST_STATUS__CIS_RUNNING) {
cbk->cb_time.ct_stop_time = time(NULL);
cbk->cb_ins_status = CHK__CHECK_INST_STATUS__CIS_FAILED;
Expand Down
26 changes: 19 additions & 7 deletions src/chk/chk_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,10 @@ enum chk_mbs_flags {
};

enum chk_pool_start_flags {
/* The pool is not in check list, but it is reported by engine for potential orphan pool. */
CPSF_FOR_ORPHAN = 1,
/* Do not export pool service after check done. */
CPSF_NOT_EXPORT_PS = 2,
};

enum chk_rejoin_flags {
Expand Down Expand Up @@ -594,6 +597,7 @@ struct chk_pool_rec {
cpr_healthy:1,
cpr_delay_label:1,
cpr_exist_on_ms:1,
cpr_not_export_ps:1,
cpr_map_refreshed:1;
int cpr_advice;
int cpr_refs;
Expand Down Expand Up @@ -689,6 +693,8 @@ void chk_pool_start_svc(struct chk_pool_rec *cpr, int *ret);

void chk_pool_stop_one(struct chk_instance *ins, uuid_t uuid, int status, uint32_t phase, int *ret);

void chk_pool_stop_all(struct chk_instance *ins, uint32_t status, int *ret);

int chk_pools_pause_cb(struct sys_db *db, char *table, d_iov_t *key, void *args);

int chk_pools_cleanup_cb(struct sys_db *db, char *table, d_iov_t *key, void *args);
Expand Down Expand Up @@ -1040,13 +1046,19 @@ chk_pool_shutdown(struct chk_pool_rec *cpr, bool locked)
if (!locked)
ABT_mutex_lock(cpr->cpr_mutex);

d_iov_set(&psid, cpr->cpr_uuid, sizeof(uuid_t));
rc = ds_rsvc_stop(DS_RSVC_CLASS_POOL, &psid, RDB_NIL_TERM, false);
D_DEBUG(DB_MD, "Stop PS for "DF_UUIDF": "DF_RC"\n",
DP_UUID(cpr->cpr_uuid), DP_RC(rc));
ds_pool_stop(cpr->cpr_uuid);
cpr->cpr_started = 0;
cpr->cpr_start_post = 0;
if (cpr->cpr_started) {
d_iov_set(&psid, cpr->cpr_uuid, sizeof(uuid_t));
rc = ds_rsvc_stop(DS_RSVC_CLASS_POOL, &psid, RDB_NIL_TERM, false);
D_DEBUG(DB_MD, "Shutdown PS for "DF_UUIDF": "DF_RC"\n",
DP_UUID(cpr->cpr_uuid), DP_RC(rc));
cpr->cpr_start_post = 0;

ds_pool_stop(cpr->cpr_uuid);
cpr->cpr_started = 0;

D_DEBUG(DB_MD, "Stop pool for "DF_UUIDF" with locked %s\n",
DP_UUID(cpr->cpr_uuid), locked ? "true" : "false");
}

if (!locked)
ABT_mutex_unlock(cpr->cpr_mutex);
Expand Down
40 changes: 18 additions & 22 deletions src/chk/chk_leader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,10 @@ chk_leader_exit(struct chk_instance *ins, uint32_t ins_phase, uint32_t ins_statu
uint32_t pool_status, bool bcast)
{
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
struct chk_iv iv = { 0 };
int rc = 0;

d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link)
chk_pool_stop_one(ins, cpr->cpr_uuid, pool_status, CHK_INVAL_PHASE, NULL);
chk_pool_stop_all(ins, pool_status, NULL);

if ((bcast && ins_status == CHK__CHECK_INST_STATUS__CIS_FAILED) ||
ins_status == CHK__CHECK_INST_STATUS__CIS_IMPLICATED ||
Expand Down Expand Up @@ -687,6 +684,9 @@ chk_leader_orphan_pool(struct chk_pool_rec *cpr)
act = prop->cp_policies[cla];
cbk->cb_statistics.cs_total++;

/* For orphan pool, do not export pool service until being registered to MS successfully. */
cpr->cpr_not_export_ps = 1;

switch (act) {
case CHK__CHECK_INCONSIST_ACTION__CIA_DEFAULT:
/*
Expand Down Expand Up @@ -719,6 +719,7 @@ chk_leader_orphan_pool(struct chk_pool_rec *cpr)
} else {
cbk->cb_statistics.cs_repaired++;
cpr->cpr_exist_on_ms = 1;
cpr->cpr_not_export_ps = 0;
}
}
break;
Expand Down Expand Up @@ -869,6 +870,7 @@ chk_leader_orphan_pool(struct chk_pool_rec *cpr)
} else {
cbk->cb_statistics.cs_repaired++;
cpr->cpr_exist_on_ms = 1;
cpr->cpr_not_export_ps = 0;
}
}
break;
Expand Down Expand Up @@ -1901,6 +1903,7 @@ chk_leader_pool_ult(void *arg)
struct ds_pool_clue *clue;
struct chk_iv iv = { 0 };
char uuid_str[DAOS_UUID_STR_SIZE];
uint32_t flags = 0;
int rc = 0;

D_INFO(DF_LEADER" pool ult enter for "DF_UUIDF"\n", DP_LEADER(ins), DP_UUID(cpr->cpr_uuid));
Expand Down Expand Up @@ -1962,13 +1965,17 @@ chk_leader_pool_ult(void *arg)
D_GOTO(out, rc = -DER_NOMEM);
}

if (cpr->cpr_for_orphan)
flags |= CPSF_FOR_ORPHAN;
if (cpr->cpr_not_export_ps)
flags |= CPSF_NOT_EXPORT_PS;

/*
* Notify all related pool shards to start the pool. Piggyback the
* phase CHK__CHECK_SCAN_PHASE__CSP_POOL_LIST to the pool shards.
*/
rc = chk_pool_start_remote(ranks, cbk->cb_gen, cpr->cpr_uuid,
CHK__CHECK_SCAN_PHASE__CSP_POOL_LIST,
cpr->cpr_for_orphan ? CPSF_FOR_ORPHAN : 0);
CHK__CHECK_SCAN_PHASE__CSP_POOL_LIST, flags);
d_rank_list_free(ranks);
if (rc != 0)
cpr->cpr_skip = 1;
Expand Down Expand Up @@ -2742,8 +2749,6 @@ chk_leader_start(uint32_t rank_nr, d_rank_t *ranks, uint32_t policy_nr, struct c
{
struct chk_instance *ins = chk_leader;
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
uuid_t *c_pools = NULL;
struct umem_attr uma = { 0 };
uuid_t dummy_pool = { 0 };
Expand Down Expand Up @@ -2901,9 +2906,7 @@ chk_leader_start(uint32_t rank_nr, d_rank_t *ranks, uint32_t policy_nr, struct c
goto out_exit;

out_stop_pools:
d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link)
chk_pool_stop_one(ins, cpr->cpr_uuid, CHK__CHECK_POOL_STATUS__CPS_IMPLICATED,
CHK_INVAL_PHASE, NULL);
chk_pool_stop_all(ins, CHK__CHECK_POOL_STATUS__CPS_IMPLICATED, NULL);
if (cbk->cb_ins_status == CHK__CHECK_INST_STATUS__CIS_RUNNING) {
cbk->cb_time.ct_stop_time = time(NULL);
cbk->cb_ins_status = CHK__CHECK_INST_STATUS__CIS_FAILED;
Expand Down Expand Up @@ -2970,8 +2973,6 @@ chk_leader_stop(int pool_nr, uuid_t pools[])
{
struct chk_instance *ins = chk_leader;
struct chk_bookmark *cbk = &ins->ci_bk;
struct chk_pool_rec *cpr;
struct chk_pool_rec *tmp;
int rc = 0;
int i;

Expand Down Expand Up @@ -3021,12 +3022,9 @@ chk_leader_stop(int pool_nr, uuid_t pools[])
goto out;

if (pool_nr == 0) {
d_list_for_each_entry_safe(cpr, tmp, &ins->ci_pool_list, cpr_link) {
chk_pool_stop_one(ins, cpr->cpr_uuid, CHK__CHECK_POOL_STATUS__CPS_STOPPED,
CHK_INVAL_PHASE, &rc);
if (rc != 0)
D_GOTO(out, rc);
}
chk_pool_stop_all(ins, CHK__CHECK_POOL_STATUS__CPS_STOPPED, &rc);
if (rc != 0)
D_GOTO(out, rc);
} else {
for (i = 0; i < pool_nr; i++) {
chk_pool_stop_one(ins, pools[i], CHK__CHECK_POOL_STATUS__CPS_STOPPED,
Expand Down Expand Up @@ -3418,9 +3416,7 @@ chk_leader_act(uint64_t seq, uint32_t act, bool for_all)

if (likely(prop->cp_policies[cla] != act)) {
prop->cp_policies[cla] = act;
rc = chk_prop_update(prop, NULL);
if (rc != 0)
goto out;
chk_prop_update(prop, NULL);
}

/*
Expand Down
4 changes: 3 additions & 1 deletion src/pool/srv_pool_check.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,8 +220,10 @@ ds_pool_clue_init(uuid_t uuid, enum ds_pool_dir dir, struct ds_pool_clue *clue)
rc = 1;
} else {
D_ASSERT(rc <= 0);
if (rc < 0)
if (rc < 0) {
D_FREE(clue->pc_tgt_status);
clue->pc_tgt_nr = 0;
}
}

clue->pc_rc = rc;
Expand Down
Loading

0 comments on commit 0086f2a

Please sign in to comment.