Repository navigation
DAOS-19636 object: fix EC selection and CPD layout refresh races - #19189
Conversation
Layout refresh during rebuild or reintegration can replace shard arrays between selection, target preparation, and dispatch. Protect each layout access and preserve request identities rather than relying only on a pre-dispatch version check. - Revalidate an opened shard against the current layout under cob_lock. Copy its endpoint, target ID, logical shard ID, EC index, and rebuild flags consistently instead of looking up the target in a newer layout. - Validate map versions and layout bounds during EC fetch and degraded shard selection. Pass a copied target ID to failure-list checks so they no longer dereference the layout without protection. - Make parity and enumeration selection version-aware, retain the group size used for selection, and propagate DER_STALE instead of treating a refresh as unavailable parity and falling back to data shards. - Reject stale target lists before fanout and explicitly complete the object task so completion callbacks can arrange map refresh and retry. - Skip success-output processing on failed attempts, preserving enumeration counts and anchors. Obtain key-to-anchor group boundaries from a version-checked snapshot and return stale errors rather than asserting or combining an old group index with a new group size. - Serialize leader-fetch timestamp updates with the layout write lock. - Resolve the CPD leader from classified request/shard entries, retaining the saved target and logical shard ID. Reject unclassified leaders before dispatch construction and protect forced-leader lookup against concurrent layout refresh. Signed-off-by: Liang Zhen <gnailzenh@gmail.com>
|
Ticket title is 'Object client may use inconsistent layout data during replica retry' |
obj_ptr2shards() read cob_shards_nr, cob_grp_size and cob_grp_nr without cob_lock. A concurrent obj_layout_refresh() (e.g. right after reintegration) frees and rebuilds the layout under the write lock, so a stat/punch/sync could see cob_shards_nr == 0 and trip: Assertion '*grp_nr == obj->cob_grp_nr' failed: Unmatched grp nr: 0/16 Read the layout under cob_lock, check it matches the request map version, and return -DER_STALE instead of asserting, as obj_dkey2grpidx() does. Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>
Use saved EC target indices for recx conversion and failure tracking. Normalize enumeration/query-key shard positions within each group, and use a version-checked layout stride for query-key selection. Signed-off-by: Liang Zhen <gnailzenh@gmail.com>
|
Test stage Functional Hardware Medium Verbs Provider MD on SSD completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19189/3/execution/node/1682/log |
| bool | ||
| obj_need_coll(struct dc_object *obj, uint32_t *start_shard, uint32_t *shard_nr, | ||
| int | ||
| obj_ptr2shards(struct dc_object *obj, uint32_t map_ver, uint32_t *start_shard, uint32_t *shard_nr, |
There was a problem hiding this comment.
Suggest to move it to cli_obj.c since it is not special for collective operation.
| if (i < 0) | ||
| D_GOTO(out, rc = i); | ||
|
|
||
| rc = dc_tx_leader_resolve(dtrgs, tgt_cnt, dc_tx_leftmost_req(tx, true) - start, i, |
There was a problem hiding this comment.
Why need this? Why not directly check pool map version?
…anges (#19207) dfuse aborted in obj_shard_list_prep() on D_ASSERT(idx != -1) while daos_array_stat() listed the dkeys of an EC_2P2 file during reintegration. A reintegration or extend adds extra shards to each group, so the in-memory group size (cob_grp_size) changes between enumeration calls when the layout is refreshed. Two things in the saved anchors depended on that size: - The dkey anchor stored grp_idx * cob_grp_size. When the group size dropped back from 6 to 4, the stored layout index (30) mapped to a different group (7 instead of 5). - EC sub-anchors were keyed by layout index (st_shard). The new request targets had no matching sub-anchor. shard_anchors_eof_check() only ignored untracked targets when the target count grew, so the shard reached obj_shard_list_prep() and the assert fired. If the indices happened to overlap, enumeration silently resumed on the wrong group with another group's anchors, which could return the wrong set of dkeys (e.g. a wrong array size). Make the anchors independent of layout extension: - Encode and decode the dkey anchor group with the object class group size (obj_get_replicas()), and use cob_grp_nr for the last-group check. key2anchor stores the logical shard id, and daos_obj_anchor_split()/daos_obj_anchor_set() use the class group size, matching the existing DIOF_TO_SPEC_GROUP decoding. - Key EC sub-anchors by logical shard id (st_shard_id) instead of layout index. - Always ignore forwarded shards that no sub-anchor tracks, not only when the shard count grew. Without a layout extension the class and layout group sizes are equal and logical ids equal layout indices, so behavior is unchanged. Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>
Encode key2anchor groups with the class width, replace CPD leader resolution with a locked version-checked snapshot, and move obj_ptr2shards to cli_obj.c. Signed-off-by: Liang Zhen <gnailzenh@gmail.com>
Snapshot the version-checked group size for allocation and iteration, and reject layout refresh before completing classification. Signed-off-by: Liang Zhen <gnailzenh@gmail.com>
Invalidate the initial shard marker when a replicated fetch retries with a different pool map, and reselect a replica in the new layout. Record the new cycle start to avoid looping on an old layout index. Preserve the original redundancy error when no replica remains. Signed-off-by: Liang Zhen <gnailzenh@gmail.com>
Signed-off-by: Liang Zhen <gnailzenh@gmail.com>
|
Test stage Functional on EL 9 completed with status FAILURE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net//job/daos-stack/job/daos/view/change-requests/job/PR-19189/7/execution/node/1362/log |
The three functional test failures in the VM stage look to be related to https://daosio.atlassian.net/browse/DAOS-19766. |
|
Sorry I clicked the wrong bottom...I wanted to add a comment but I closed the PR by mistake. |
| cb_args.epoch = &args->ka_auxi.epoch; | ||
| cb_args.th = &obj_args->th; | ||
| cb_args.anchor = args->ka_anchor; | ||
| cb_args.shard = obj_shard->do_shard_idx; |
There was a problem hiding this comment.
Is it correct? during reintegration, shard index maybe different from shard ID. Which one is expected to be used?
There was a problem hiding this comment.
do_shard_idx is the position in client layout array, which can be invalid after refresh, do_shard is the logic shard ID, which can identify the actual shard in the layout. I agree naming is confusing, we need a cleanup in the future.
There was a problem hiding this comment.
What will happen if two shards with the identifiers (one old, the other is new) for the same object in the same redundancy group during reintegration?
Nasf-Fan
left a comment
There was a problem hiding this comment.
According to the discussion with Liang, he will resolve daos_anchor_t::shard related issue in the future.
Layout refresh during rebuild or reintegration can replace shard arrays between selection, target preparation, and dispatch. Protect each layout access and preserve request identities rather than relying only on a pre-dispatch version check.
Steps for the author:
After all prior steps are complete: