Skip to content

[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
mainfrom
codex/nightly-docs-2cd89e148d4bb97e
Open

github-actions[bot] wants to merge 1 commit into
mainfrom
codex/nightly-docs-2cd89e148d4bb97e

Conversation

@github-actions

@github-actions github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

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

  • Passed the documentation path and size guard (under 1000 added plus deleted lines; no file-count limit).
  • Passed an independent accuracy and single-concern review.
  • Passed git diff --check and website content/link tests, type checks, lint and production build.

Checklist

  • Every commit is signed off (git commit -s)
  • pnpm lint && pnpm check && pnpm test && pnpm build passes (run by the publisher on an isolated copy)

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Nov 2, 2026

Copy link
Copy Markdown

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6b32602b-771b-47ad-8238-220708220ced

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Documentation maintenance: ready. Unsuccessful content rounds: 0/3. Operational attempts: 0/3.

Head: eee1a1f2d365960b063c603c3ef29d3e6910d21b; reviewed main: 2d7041091d7a485b4b8c51ff3d6b2d09b17aa3f1.

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.

Workflow evidence

Human review threads and CODEOWNER approval remain under repository policy.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants