Skip to content

DAOS-18690 vos: fix engine crash when extend committed DTX table - #18671

Merged
gnailzenh merged 4 commits into
masterfrom
Nasf-Fan/DAOS-18690_5
Aug 26, 2026
Merged

gnailzenh merged 4 commits into
masterfrom
Nasf-Fan/DAOS-18690_5

Conversation

@Nasf-Fan

@Nasf-Fan Nasf-Fan commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

PR#18141 (b36dd7b) introduced some regression:

When the pool is physically full (ENOSPC), extending committed DTX table for punch/update commit may crash the whole engine with SIGABRT. That is because the backend TX logic (such as PMDK) will abort the TX by default if hit failure when allocation. Then subsequent operation (such as tx_add_ptr that is triggered by vos_dtx_extend_cmt_table()) against such aborted backend TX will trigger assertion inside backend TX logic.

The patch replaces umem_zalloc() with umem_alloc_verb() for extending committed DTX table with new UMEM_FLAG_NO_ABORT flag that will notify backend TX logic to return ENOSPC without aborted related TX. Similar enhancement is also added for tx_add_ptr related logic.

New unit tests (UMEM010/BMEM016/BMEM024) for that.

Steps for the author:

  • Commit message follows the guidelines.
  • Appropriate Features or Test-tag pragmas were used.
  • Appropriate Functional Test Stages were run.
  • At least two positive code reviews including at least one code owner from each category referenced in the PR.
  • Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

  • Gatekeeper requested (daos-gatekeeper added as a reviewer).

@github-actions

github-actions Bot commented Jul 15, 2026 •

Copy link
Copy Markdown

Ticket title is 'Aurora daos_user: SCM single target ran out of space (min:0 B) and not able to finish GC. '
Status is 'In Review'
Labels: 'request_for_2.8.1'
https://daosio.atlassian.net/browse/DAOS-18690

@daosbuild3

Copy link
Copy Markdown
Collaborator

@daosbuild3

Copy link
Copy Markdown
Collaborator

Test stage Unit Test with memcheck completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18671/2/testReport/

@hgichon hgichon left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @Nasf-Fan — cross-checked against #18612 and everything landed: flag consistency, full failure_behavior unwind (no dangling refs), and the commit path (vos_dtx_commit_internal) is fully NO_ABORT-protected, so the crash can't recur. Nice that UMEM010 now covers the xadd_ptr path too. LGTM.

Two non-blocking checks:

  1. vos_dtx_extend_act_table() / vos_dtx_set_flags_one() reverted to plain umem_tx_add_ptr() (abort on failure) — intentional for those non-commit paths?
  2. UMEM010 runs under setup_pmem only; the reported crash was on md-on-SSD (DAV). Same coverage as #18612, just flagging in case a bmem variant is wanted.

@Nasf-Fan

Copy link
Copy Markdown
Contributor Author

Thanks @Nasf-Fan — cross-checked against #18612 and everything landed: flag consistency, full failure_behavior unwind (no dangling refs), and the commit path (vos_dtx_commit_internal) is fully NO_ABORT-protected, so the crash can't recur. Nice that UMEM010 now covers the xadd_ptr path too. LGTM.

Two non-blocking checks:

  1. vos_dtx_extend_act_table() / vos_dtx_set_flags_one() reverted to plain umem_tx_add_ptr() (abort on failure) — intentional for those non-commit paths?

vos_dtx_add_ptr() is used to handle space pressure when we try to release some space (directly or indirectly), such as DTX aggregation or DTX commit. But vos_dtx_extend_act_table() is during update/punch processing that will consume more space, even if we use vos_dtx_add_ptr(), related modification itself will also consume more space and maybe out of space when space pressure. So they are not the candidate users for vos_dtx_add_ptr().

  1. UMEM010 runs under setup_pmem only; the reported crash was on md-on-SSD (DAV). Same coverage as DAOS-18690 vos: fix engine abort in DTX commit blob fallback on ENOSPC #18612, just flagging in case a bmem variant is wanted.

I will add new test cases to cover bmem and bmem_v2.

@Nasf-Fan
Nasf-Fan force-pushed the Nasf-Fan/DAOS-18690_5 branch 2 times, most recently from 9619ce3 to 3b39604 Compare July 16, 2026 04:19
@daosbuild3

Copy link
Copy Markdown
Collaborator

@daosbuild3

Copy link
Copy Markdown
Collaborator

Test stage Unit Test with memcheck completed with status UNSTABLE. https://jenkins-3.daos.hpc.amslabs.hpecorp.net/job/daos-stack/job/daos//view/change-requests/job/PR-18671/4/testReport/

@Nasf-Fan
Nasf-Fan force-pushed the Nasf-Fan/DAOS-18690_5 branch from 3b39604 to 7788fea Compare July 17, 2026 05:02
@daosbuild3

Copy link
Copy Markdown
Collaborator

Test stage Functional Hardware Large 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-18671/4/execution/node/1773/log

@daosbuild3

Copy link
Copy Markdown
Collaborator

Test stage Functional Hardware Medium 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-18671/4/execution/node/1763/log

@Nasf-Fan
Nasf-Fan marked this pull request as ready for review July 18, 2026 01:47
@Nasf-Fan
Nasf-Fan requested review from a team July 18, 2026 01:47
@Nasf-Fan
Nasf-Fan requested review from a team as code owners July 18, 2026 01:47
@Nasf-Fan
Nasf-Fan requested review from hgichon, janekmi and knard38 and removed request for hgichon July 18, 2026 01:47
PR#18141 (b36dd7b) introduced some regression:

When the pool is physically full (ENOSPC), extending committed DTX
table for punch/update commit may crash the whole engine with SIGABRT.
That is because the backend TX logic (such as PMDK) will abort the TX
by default if hit failure when allocation. Then subsequent operation
(such as tx_add_ptr that is triggered by vos_dtx_extend_cmt_table())
against such aborted backend TX will trigger assertion inside backend
TX logic.

The patch replaces umem_zalloc() with umem_alloc_verb() for extending
committed DTX table with new UMEM_FLAG_NO_ABORT flag that will notify
backend TX logic to return ENOSPC without aborted related TX. Similar
enhancement is also added for tx_add_ptr related logic.

New unit tests (UMEM010/BMEM016/BMEM024) for that.

Signed-off-by: Fan Yong <fan.yong@hpe.com>
Co-authored-by: hgichon <hgichon@gmail.com>
@Nasf-Fan
Nasf-Fan force-pushed the Nasf-Fan/DAOS-18690_5 branch from 7788fea to feb6d69 Compare July 21, 2026 04:27

@knard38 knard38 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

*/
int utest_check_mem_initial_status(struct utest_context *utx);

/** Check transaction failure with NO_ABROT flag

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/** Check transaction failure with NO_ABROT flag
/** Check transaction failure with NO_ABORT flag

knard38
knard38 previously approved these changes Jul 21, 2026
@Nasf-Fan

Copy link
Copy Markdown
Contributor Author

Ping reviewers, thanks!

@hgichon

hgichon commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Apologies for the very late confirmation on this — thank you for reworking the fix.

I verified this patch (feb6d69) against the original DAOS-18690 scenario we hit during our project-quota work:

  • Cherry-picked cleanly onto our exastor feature/proj-quota branch (only src/common/mem.c needed a trivial adaptation for our older POBJ_XALLOC_NO_ABORT → your POBJ_FLAG_TX_NO_ABORT naming; no functional change).
  • Built and ran the VOS unit tests: UMEM010: Test TX failure with NO_ABORT on pmem passes, i.e. a failed NO_ABORT allocation now leaves the TX alive and subsequent alloc/tx_add_ptr/commit succeed (this crashed/aborted the engine before the fix). All 10 umem tests green.
  • Full regression of our projid-quota feature on top of this fix: container proj_quota set-prop, projid xattr set/get, directory inheritance, over-limit enforcement, and unlimited isolation all pass, with no engine SIGABRT across the run.

So this resolves the DTX-commit-under-ENOSPC engine crash for us, and the umem_alloc_verb + UMEM_FLAG_NO_ABORT approach (with umem_tx_xadd_ptr in vos_dtx_add_ptr) is a clean superset of what we had locally. LGTM — thanks again.

@hgichon hgichon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified against the original DAOS-18690 scenario (UMEM010 NO_ABORT regression + full projid-quota regression, no engine abort) — see comment above. LGTM.

@Nasf-Fan
Nasf-Fan requested a review from a team July 26, 2026 04:39
The reusing behavior is equal to (DTX) aggregating one committed DTX blob
from victim container. So need to refresh relatred ::cd_newest_aggregated.

Signed-off-by: Fan Yong <fan.yong@hpe.com>
@Nasf-Fan

Nasf-Fan commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Ping reviewers, thanks!

knard38
knard38 previously approved these changes Aug 10, 2026
@Nasf-Fan
Nasf-Fan requested a review from a team August 11, 2026 06:49
@daltonbohning
daltonbohning removed the request for review from a team August 11, 2026 15:03
@daltonbohning

Copy link
Copy Markdown
Contributor

This still needs a second approval before landing.

Resolve merge conflict.

Signed-off-by: Fan Yong <fan.yong@hpe.com>
@gnailzenh
gnailzenh merged commit 434fec6 into master Aug 26, 2026
46 of 49 checks passed
@gnailzenh
gnailzenh deleted the Nasf-Fan/DAOS-18690_5 branch August 26, 2026 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants