Repository navigation
[Docs] An interrupted shared-copy cleanup resumes from its receipt: the deletion completes and nothing is stranded - #217
Open
github-actions[bot] wants to merge 1 commit into
Conversation
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
github-actions
Bot
requested review from
XinyueZhang369,
pallasathena92 and
slin1237
as code owners
November 2, 2026 20:43
|
Important Review skippedBot user detected. To trigger a single review, invoke the
|
Contributor
Author
|
Documentation maintenance: ready. Unsuccessful content rounds: 0/3. Operational attempts: 0/3. Head: The diff adds two paragraphs to shared-hf-artifacts.md documenting one concern: an interrupted shared-copy cleanup resumes from its receipt. Every claim verified against the pinned OME checkout: the receipt is written in the same ConfigMap CAS that removes both live references (removeModelReference, hf_artifact_repository.go:461-468) and records the snapshot, parent directory, child path and ParentStatusBeforeDeletion; handleDelete resumes an existing receipt first (hf_artifact_delete.go:19-25) and handleDownload/repair call resumeDeletionBeforeDownload before downloading (hf_artifact_download.go:24), with UID takeover for a recreated same-name model (claimPendingDeletion, TestHfArtifactPendingDeletionUIDTakeover); a new agent pod re-sends delete tasks via reconcilePendingDeletions (scout.go:579-611) and the pernode finalizer holds the model until node ConfigMap entries clear (pernode/deletion.go); a resumed cleanup preserves a path that gained a user and restores the copy's pre-deletion status (resumePendingDeletion, releaseParentDeletionLock, TestHfArtifactPendingDeletionPreservesReattachedPath); failed steps log "Shared artifact %s needs retry: %v" (gopher_hf_artifact.go:630) and nothing in the cleanup path marks the copy or models Failed (markLockedParentFailed removed); startup recovery retains receipt ownership instead of treating the copy as interrupted/damaged (hf_artifact_startup.go:194-221). No since badges or release claims added, no protected files edited. Context contains no review threads, so none to address. Human review threads and CODEOWNER approval remain under repository policy. |
2 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
I deleted the last model that shared a Hugging Face snapshot and the agent pod on a node was replaced mid-cleanup — is the half-deleted copy stranded there, and will kubectl delete ever finish?
Why we need it
Source change: ome-projects/ome@1368d5e
From commit 17 (#988, 'Resume interrupted shared artifact deletion'), whose patch adds pkg/modelagent/hf_artifact_pending_deletion.go as a new file (patch lines 630-635, read today), rewires handleDelete to write and resume a cleanup receipt (patch lines 522-528 and 565-570), and replaces the old markLockedParentFailed error path with retries (patch lines 593-612), so a failed cleanup step no longer marks the sharing models Failed. Current code verified today: hf_artifact_pending_deletion.go:13-25 — the receipt is 'a cleanup receipt, not a live parent reference. It is written in the same ConfigMap CAS that removes both live references', recording the snapshot identity, the shared copy's path, the model's link path and the copy's status before deletion (ParentStatusBeforeDeletion), so the moment the delete removes the model's recorded reference the remaining file work is durably owned; hf_artifact_delete.go:19-24 — every later delete task for the model finds the receipt and resumes it, and :75-81 — a fresh delete writes the receipt right after removing the reference; resumePendingDeletion (hf_artifact_pending_deletion.go:280-344) removes the model's link unless the path has gained another user, deletes the copy's directory when it was the last user and no recorded or on-disk children remain (deleteLockedParent, hf_artifact_delete.go:113-149), and when another model still uses the copy restores its pre-deletion state instead of deleting (releaseParentDeletionLock, hf_artifact_delete.go:151-159); hf_artifact_pending_deletion.go:368-381 resumeDeletionBeforeDownload — even a new download task for the same snapshot finishes the pending cleanup first; hf_artifact_startup.go:194-221 — at agent startup 'a matching deletion receipt retains ownership until cleanup is complete; other abandoned operations become retryable without publishing Failed'. Because the model's finalizer holds the object until every node's agent reports the cleanup done, the receipt is what lets a kubectl delete interrupted by a replaced or evicted agent pod complete on the next pass instead of hanging with half-deleted files. Tests in today's checkout: hf_artifact_pending_deletion_test.go, hf_artifact_pending_deletion_gopher_test.go, and gopher_hf_artifact_test.go:225 (a model whose storage source moved away still resumes its pending deletion). Docs gap verified today: guides/operate-ome/shared-hf-artifacts.md:32 documents deletion ('the agent deletes the shared copy once no model uses it and no link to it is left') and :30/:34 document only an interrupted DOWNLOAD (partial files deleted and re-downloaded; startup cleanup for ineligible models), so a reader whose agent pod died mid-cleanup finds a half-present _artifacts directory with no statement that the next pass finishes the job, that an error during cleanup retries rather than failing the other models, or that a copy another model attached to meanwhile is restored to its previous state rather than deleted; configure-model-artifact-retention.md:173-174 covers only the direct-model deletion retry ('the deletion resumes when it starts again'), not the shared-copy half-deleted state. A repo-wide grep under src/lib/content for an interrupted shared deletion or its receipt matches nothing (run today). One independent concern: one interruption-recovery contract for the shared-copy deletion path, from the one commit that introduced the receipt, extending the How-sharing-works paragraphs that own shared-copy deletion. Distinct from merged #165 (an interrupted shared DOWNLOAD stays retryable — commit 3), #102 (which files a deletion keeps for referenced paths — commit 4), #23 (deleting a model cancels its in-flight download — commit 8), #194 (re-adopting copies after ConfigMap loss — commit 1) and #97 (the .hf-artifact-locks directories — commit 9). shared-hf-artifacts.md is free of open PRs (open #213 reserves only base-models.md).
Scope: model-storage / interrupted-shared-deletion-resume. Other concerns are deferred.
How to test
git diff --checkand website content/link tests, type checks, lint and production build.Checklist
git commit -s)pnpm lint && pnpm check && pnpm test && pnpm buildpasses (run by the publisher on an isolated copy)