Skip to content

fix(sealing): don't require free space to move files already in storage - #13949

Open
hmdsefi wants to merge 3 commits into
filecoin-project:masterfrom
hmdsefi:fix/move-selector-full-storage
Open

hmdsefi wants to merge 3 commits into
filecoin-project:masterfrom
hmdsefi:fix/move-selector-full-storage

Conversation

@hmdsefi

@hmdsefi hmdsefi commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Related Issues

Fixes #10697

Proposed Changes

  • moveSelector.Ok first checks if the worker already has every requested file type in one of its paths of the destination type (CanStore for long-term storage), allowed by that path's type filters. If it does, the move is a no-op, and the worker is accepted and preferred without calling StorageBestAlloc.
  • Add TestMoveSelectorFullStorage in sched_test.go. It covers a full long-term storage path that already has the sealed and cache files, and a move that also includes an unsealed file from the sealing path, which still fails with no good path found.
  • CHANGELOG entry.

Additional Info

The selector called StorageBestAlloc for every move and returned its error. When the storage paths are full that is finding best dest storage: no good path found for every worker, so the scheduler logs trySched(1) req.Sel.Ok error on each pass and the fetch task never runs. FinalizeReplicaUpdate hits this for snap deals: it runs move(FTCache) and move(FTSealed), and those files are normally already in long-term storage. Once the update files are moved there, the path may not have a full sector of free space left, and the sector stays in FinalizeReplicaUpdate.

The move itself is already a no-op on master. Since e86d81b, Local.AcquireSector also looks for existing files when it's asked to allocate, and uses them if the path fits the request, so Local.MoveStorage finds the destination without needing free space. I checked this with a scratch test on a full path: MoveStorage returns nil, while StorageBestAlloc returns no good path found. After the move, LocalWorker.MoveStorage still removes copies left in sealing paths.

If any requested file still has to be moved, the selector works as before and needs free space for it.

This changes a different part of Ok than my PR for #9946 (DisallowRemoteFinalize). The two merge without conflicts in selector_move.go, and both tests pass on the merged code.

Tests:

  • go test ./storage/sealer/ passes.
  • With the selector_move.go change reverted, TestMoveSelectorFullStorage fails with finding best dest storage: no good path found.
  • gofmt, go vet and golangci-lint v1.64.8 report no issues for ./storage/sealer/....

Checklist

Before you mark the PR ready for review, please make sure that:

The move selector always asked the index for the best destination path
to allocate in, and returned an error when no path had enough free
space. A move of files that are already in a long-term storage path is
a no-op, but on a full path the selector failed for every worker and the
fetch task stayed in the scheduler queue. FinalizeReplicaUpdate hits
this because it moves the sealed and cache files, which are usually
already in long-term storage.

Accept and prefer a worker that already has all requested files in
paths of the destination type, before looking for free space.

Fixes filecoin-project#10697
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FilOz Oct 21, 2026
@github-project-automation github-project-automation Bot moved this from 📌 Triage to ✔️ Approved by reviewer in FilOz Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✔️ Approved by reviewer

Development

Successfully merging this pull request may close these issues.

sealing: MoveStorage in a full storage path is broken

2 participants