Skip to content

DAOS-19616 dtx: fix race issues between DTX resync and rebuild - b28 #19227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
Nasf-Fan wants to merge 1 commit into
base: release/2.8
Choose a base branch
from
+370 −189
Draft

DAOS-19616 dtx: fix race issues between DTX resync and rebuild - b28 #19227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/container/srv_container.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1967,9 +1967,16 @@ cont_refresh_track_eph_one(void *data)

cont_child->sc_ec_agg_eph_valid = 1;

/* Only should update local stable epoch if the target is in UPIN status */
/*
* Only update the global stable epoch if the target is in UPIN or DRAIN status.
* A DRAIN target is still serving I/O, participates in rebuild scanning and is
* counted in the leader's min calculation, so it must keep tracking the GSE
* (rebuild_container_scan_cb() waits for GSE >= rt_stable_epoch before scan).
* Do NOT include UP (reintegrating) targets: their GSE must stay frozen at the
* value it had when they were excluded.
*/
if (cont_child->sc_global_stable_eph < arg->min_stable_eph &&
(arg->tgt_status[idx] & PO_COMP_ST_UPIN)) {
(arg->tgt_status[idx] & (PO_COMP_ST_UPIN | PO_COMP_ST_DRAIN))) {
rc = vos_cont_set_global_stable_epoch(cont_child->sc_hdl, arg->min_stable_eph);
if (rc == 0)
cont_child->sc_global_stable_eph = arg->min_stable_eph;
Expand Down
79 changes: 31 additions & 48 deletions src/dtx/dtx_coll.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,61 +176,43 @@ dtx_coll_prep(uuid_t po_uuid, daos_unit_oid_t oid, struct dtx_id *xid, struct dt

dce->dce_bitmap_sz = dct->dct_bitmap_sz;

if (!for_check) {
memcpy(dce->dce_bitmap, dct->dct_tgts + dct->dct_tgt_nr, dct->dct_bitmap_sz);
} else {
map = pl_map_find(po_uuid, oid.id_pub);
if (map == NULL) {
D_ERROR("Failed to find valid placement map in pool "DF_UUID"\n",
DP_UUID(po_uuid));
D_GOTO(out, rc = -DER_INVAL);
}
map = pl_map_find(po_uuid, oid.id_pub);
if (map == NULL) {
D_ERROR("Failed to find valid placement map in pool " DF_UUID "\n",
DP_UUID(po_uuid));
D_GOTO(out, rc = -DER_INVAL);
}

for (i = 0, j = 0; i < dct->dct_tgt_nr; i++) {
rc = pool_map_find_target(map->pl_poolmap, dct->dct_tgts[i], &target);
D_ASSERT(rc == 1);

/* Skip the targets that reside on other engines. */
if (unlikely(target->ta_comp.co_rank != my_rank))
continue;

/* Skip the target that (re-)joined the system after the DTX. */
if (target->ta_comp.co_ver > dtx_ver)
continue;

/* Skip non-healthy one. */
if (target->ta_comp.co_status != PO_COMP_ST_UP &&

@gnailzenh gnailzenh Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why UP should be removed from here?

@Nasf-Fan Nasf-Fan Oct 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not removed, still be there at L199.

target->ta_comp.co_status != PO_COMP_ST_UPIN &&
target->ta_comp.co_status != PO_COMP_ST_DRAIN)
continue;

/* Skip current (new) leader target. */
if (my_tgtid != target->ta_comp.co_index) {
setbit(dce->dce_bitmap, target->ta_comp.co_index);
j++;
}
}
for (i = 0; i < dct->dct_tgt_nr; i++) {
rc = pool_map_find_target(map->pl_poolmap, dct->dct_tgts[i], &target);
D_ASSERT(rc == 1);

rc = 0;
/* Skip the targets that reside on other engines. */
if (unlikely(target->ta_comp.co_rank != my_rank))
continue;

if (unlikely(j == 0)) {
D_FREE(dce->dce_bitmap);
dce->dce_bitmap_sz = 0;
}
/* Skip the target that (re-)joined the system after the DTX. */
if (target->ta_comp.co_ver > dtx_ver)
continue;

/* Skip non-healthy one. */
if (target->ta_comp.co_status != PO_COMP_ST_UP &&
target->ta_comp.co_status != PO_COMP_ST_UPIN &&
target->ta_comp.co_status != PO_COMP_ST_DRAIN)
continue;

/* Skip UP target for CHECK case. */
if (for_check && target->ta_comp.co_status == PO_COMP_ST_UP)
continue;

setbit(dce->dce_bitmap, target->ta_comp.co_index);
}

rc = 0;

if (!need_hint)
goto out;

if (map == NULL) {
map = pl_map_find(po_uuid, oid.id_pub);
if (map == NULL) {
D_ERROR("Failed to find valid placement map in pool "DF_UUID"\n",
DP_UUID(po_uuid));
D_GOTO(out, rc = -DER_INVAL);
}
}

md.omd_id = oid.id_pub;
md.omd_ver = pm_ver;
md.omd_fdom_lvl = dct->dct_fdom_lvl;
Expand Down Expand Up @@ -365,7 +347,8 @@ dtx_coll_local_one(void *args)
rc = vos_dtx_abort(cont->sc_hdl, &dcla->dcla_xid, dcla->dcla_epoch, dcla->dcla_ver);
break;
case DTX_COLL_CHECK:
rc = vos_dtx_check(cont->sc_hdl, &dcla->dcla_xid, NULL, NULL, NULL, false);
rc = vos_dtx_check(cont->sc_hdl, &dcla->dcla_xid, NULL, &dcla->dcla_ver, NULL,
DCI_RESYNC);
if (rc == DTX_ST_INITED) {
/*
* For DTX_CHECK, non-ready one is equal to non-exist. Do not directly
Expand Down
15 changes: 9 additions & 6 deletions src/dtx/dtx_common.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,17 +959,20 @@ dtx_handle_init(struct dtx_id *dti, daos_handle_t xoh, struct dtx_epoch *epoch,
dth->dth_dkey_hash = 0;

if (!(flags & DTX_LOCAL)) {
if (daos_is_zero_dti(dti))
return 0;

if (!dtx_epoch_chosen(epoch)) {
if (daos_is_zero_dti(dti))
return 0;

D_ERROR("initializing DTX " DF_DTI " with invalid epoch: value=" DF_U64
" first=" DF_U64 " flags=%x\n",
DP_DTI(dti), epoch->oe_value, epoch->oe_first, epoch->oe_flags);
return -DER_INVAL;
}
dth->dth_epoch = epoch->oe_value;
dth->dth_epoch_bound = dtx_epoch_bound(epoch);

if (daos_is_zero_dti(dti))
return 0;
}

rc = vos_dtx_rsrvd_init(dth);
Expand Down Expand Up @@ -1328,9 +1331,9 @@ dtx_leader_end(struct dtx_leader_handle *dlh, struct ds_cont_child *cont, int re
*/
vos_dtx_cleanup(dth, true);
if (dlh->dlh_coll)
dtx_coll_abort(cont, dlh->dlh_coll_entry, dth->dth_epoch);
dtx_coll_abort(cont, dlh->dlh_coll_entry, dth->dth_epoch, dth->dth_ver);
else
dtx_abort(cont, &dth->dth_dte, dth->dth_epoch);
dtx_abort(cont, &dth->dth_dte, dth->dth_epoch, dth->dth_ver);
aborted = true;
}

Expand Down Expand Up @@ -1870,7 +1873,7 @@ dtx_handle_resend(daos_handle_t coh, struct dtx_id *dti,
*/
return -DER_NONEXIST;

rc = vos_dtx_check(coh, dti, epoch, pm_ver, NULL, false);
rc = vos_dtx_check(coh, dti, epoch, pm_ver, NULL, DCI_RESENT);
switch (rc) {
case DTX_ST_INITED:
return -DER_INPROGRESS;
Expand Down
10 changes: 5 additions & 5 deletions src/dtx/dtx_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,11 @@ int dtx_coll_local_exec(uuid_t po_uuid, uuid_t co_uuid, struct dtx_id *xid, daos
/* clang-format on */

enum dtx_status_handle_result {
DSHR_NEED_COMMIT = 1,
DSHR_NEED_RETRY = 2,
DSHR_IGNORE = 3,
DSHR_ABORT_FAILED = 4,
DSHR_CORRUPT = 5,
DSHR_NEED_COMMIT = DTX_ST_MAX + 100,
DSHR_NEED_RETRY,
DSHR_IGNORE,

@gnailzenh gnailzenh Oct 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: I think it's confusing (and easy to make mistake) for vos_dtx_check() to return two different sets of value, would be better to add a new output paramter to vos_dtx_check(), but we can do this cleanup later on master

DSHR_ABORT_FAILED,
DSHR_CORRUPT,
};

enum dtx_rpc_flags {
Expand Down
14 changes: 8 additions & 6 deletions src/dtx/dtx_resync.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ dtx_resync_commit(struct ds_cont_child *cont,
* committed or aborted the DTX during we handling other
* DTXs. So double check the status before current commit.
*/
rc = vos_dtx_check(cont->sc_hdl, &dre->dre_xid, NULL, NULL, NULL, false);
rc = vos_dtx_check(cont->sc_hdl, &dre->dre_xid, NULL, NULL, NULL, DCI_DEFAULT);

/*
* Skip this DTX since it has been committed or aggregated.
Expand Down Expand Up @@ -240,6 +240,8 @@ dtx_status_handle_one(struct ds_cont_child *cont, struct dtx_entry *dte, daos_un
rc = dtx_check(cont, dte, epoch);
}
switch (rc) {
case DSHR_IGNORE:
goto out;
case DTX_ST_COMMITTED:
case DTX_ST_COMMITTABLE:
/* The DTX has been committed on some remote replica(s),
Expand Down Expand Up @@ -282,9 +284,9 @@ dtx_status_handle_one(struct ds_cont_child *cont, struct dtx_entry *dte, daos_un
* dtx_abort() with 0 @epoch.
*/
if (mbs->dm_flags & DMF_COLL_TARGET)
rc = dtx_coll_abort(cont, dce, 0);
rc = dtx_coll_abort(cont, dce, 0, 0);
else
rc = dtx_abort(cont, dte, 0);
rc = dtx_abort(cont, dte, 0, 0);
if (rc < 0 && err != NULL)
*err = rc;

Expand All @@ -298,7 +300,7 @@ dtx_status_handle_one(struct ds_cont_child *cont, struct dtx_entry *dte, daos_un
* committed or aborted the DTX during we handling other
* DTXs. So double check the status before next action.
*/
rc = vos_dtx_check(cont->sc_hdl, &dte->dte_xid, NULL, NULL, NULL, false);
rc = vos_dtx_check(cont->sc_hdl, &dte->dte_xid, NULL, NULL, NULL, DCI_DEFAULT);

/* Skip the DTX that may has been committed or aborted. */
if (rc == DTX_ST_COMMITTED || rc == DTX_ST_COMMITTABLE || rc == -DER_NONEXIST)
Expand Down Expand Up @@ -326,9 +328,9 @@ dtx_status_handle_one(struct ds_cont_child *cont, struct dtx_entry *dte, daos_un
* abort the DTXs one by one, not batched.
*/
if (mbs->dm_flags & DMF_COLL_TARGET)
rc = dtx_coll_abort(cont, dce, epoch);
rc = dtx_coll_abort(cont, dce, epoch, cont->sc_pool->spc_map_version);
else
rc = dtx_abort(cont, dte, epoch);
rc = dtx_abort(cont, dte, epoch, cont->sc_pool->spc_map_version);

DL_CDEBUG(rc != 0, DLOG_ERR, DB_TRACE, rc,
"As new leader for DTX " DF_DTI ", abort it (2)", DP_DTI(&dte->dte_xid));
Expand Down
Loading
Loading