* fix(shell): error on missing volume id in fsck, mergeVolumes, vacuum
Three shell commands silently report success when -volumeId /
-fromVolumeId / -toVolumeId names a volume the master doesn't know
about: typos, already-deleted volumes, and stale scripts all look
identical to a clean no-op, which is what made the confusion in #9116
take as long as it did to diagnose.
- volume.fsck: filter at the per-datanode loop drops unknown ids and
findExtraChunksInVolumeServers ends with totalOrphanChunkCount==0,
printing "no orphan data".
- fs.mergeVolumes: createMergePlan iterates only known volumes, so an
unknown -fromVolumeId produces an empty plan and we print just the
"max volume size: N MB" header (indistinguishable from "nothing to
merge").
- volume.vacuum: the master's VacuumVolume RPC silently iterates
matching volumes; a missing id returns success having done nothing.
Validate the requested ids against the current topology up front and
return an explicit "volume(s) not found on master: [X Y]" error. Also
drop a stale duplicate `if err != nil` in volume.fsck.Do left over from
a prior refactor.
Surfaces #9116 follow-up from madalee-com.
* address review: propagate reloadVolumesInfo error; dedupe vacuum missing ids
- fs.mergeVolumes: c.reloadVolumesInfo's return was ignored. If the
master is unreachable or VolumeList fails, c.volumes stays empty and
the new validation block reports "fromVolumeId X not found on master"
— masking the real connection/RPC failure. Return the wrapped error
instead.
- volume.vacuum: "volume.vacuum -volumeId 5,5,5" on a missing volume 5
listed [5 5 5] in the error. Collect missing ids in a set so each
missing id appears once.
* address review: reject fromVolumeId/toVolumeId values that overflow uint32
flag.Uint produces a uint (64-bit on amd64), and the existing cast to
needle.VolumeId silently truncates to uint32. A typo like
`-fromVolumeId=4294967297` would wrap to volume 1 and slip past every
other validation, so the merge would run against a completely
different volume than the operator intended.
Bail out with an explicit error when the raw flag value exceeds the
uint32 range, before the cast.
* feat(shell): fs.mergeVolumes deletes source needles after filer update
Before this change, mergeVolumes only copied chunks to the destination
volume and updated the filer — the source needle sat untouched on its
original volume as a silent orphan. Operators had to run a separate
volume.fsck + volume.vacuum pass to actually reclaim the space, and
#9116 (comment 4282692876) showed how that pipeline can look exactly
like "mergeVolumes did nothing": the source volume keeps reporting its
original size even though every chunk has been logically moved out.
Clean up the source inline. For each entry, track the pre-move fids as
they're captured, and after the UpdateEntry RPC commits, issue
BatchDelete on every replica of each source volume. Key invariants:
- Source fids are only deleted AFTER UpdateEntry succeeds; if the
filer write fails we skip the cleanup for that entry so we never
delete data the filer still references.
- rewriteManifestChunk grew a fourth return value so nested manifest
and sub-chunk moves propagate their moved-source list back to the
top-level callsite. The outer manifest itself is recorded at the
callsite, since only the callsite sees the pre-rewrite fid.
- deleteMovedSourceNeedles logs errors but never returns them.
Propagating would abort TraverseBfs mid-merge, stranding remaining
entries; logging leaves the fallback path (fsck reconciles later)
intact.
- StatusNotModified from the volume server is expected whenever a
concurrent fsck purge beat us to the delete or a replica already
reconciled — don't warn on it.
Readonly source volumes are already rejected up front by
createMergePlan, so by the time we reach the delete the source is
writable. If a replica's readonly bit has flipped since then the
delete will fail and get logged; the user can re-run once they've
fixed the replica (same failure mode as today's fsck purge).
Fixes the space-not-reclaimed half of #9116.
Related design discussion: #8589.
* address review: cast r.Status to int in StatusNotModified compare
http.StatusNotModified is an untyped constant so the compare works as
written, but the int32/int mixed-type signal trips static analyzers
and PR tooling. Cast explicitly and note why.
* fix(shell): fs.mergeVolumes now rewrites manifest chunks for large files
Previously fs.mergeVolumes skipped any chunk whose IsChunkManifest flag was
true, printing "Change volume id for large file is not implemented yet" and
continuing. Because the BFS traversal only looks at top-level
entry.Chunks, sub-chunks referenced inside a manifest were never
considered either. For any file stored as a chunk manifest (large files
go this path), chunks in the source volume stayed put, leaving behind a
few MB of live data that vacuum and volume.deleteEmpty couldn't clean
up.
This change resolves each manifest chunk recursively, moves any
sub-chunk whose volume id is in the merge plan via the existing
moveChunk path, and re-serializes the manifest. If the manifest chunk
itself lives in a source volume, or any sub-chunk moved, the new
manifest blob is uploaded to a freshly assigned file id (the old
needle becomes orphaned and is reclaimed by vacuum like any other
moved chunk).
Fixes#9116.
* address review: batch UpdateEntry, fix dry-run, defer restore, avoid source volumes
- Call UpdateEntry once per entry after the chunk loop instead of once per
moved chunk (gemini nit).
- In dry-run mode, mark anySubChanged when a sub-chunk in the plan is
encountered and return changed=true after printing "rewrite manifest",
so nested manifests also surface their would-rewrites (gemini nit).
- Defer filer_pb.AfterEntryDeserialization so the manifest chunk list is
restored even when proto.Marshal fails (coderabbit nit).
- Reject AssignVolume results whose file id lands on a volume that is a
source in the merge plan, and retry — otherwise the replacement
manifest could be written to the volume being emptied (coderabbit).
The log message was comparing against the planned size of the destination
volume (including volumes already planned to merge into it) but only
displaying the raw volume size, making the output confusing when the
displayed sizes clearly didn't add up to exceed the limit.
* Add TraverseBfsWithContext and fix race conditions in error handling
- Add TraverseBfsWithContext function to support context cancellation
- Fix race condition in doTraverseBfsAndSaving using atomic.Bool and sync.Once
- Improve error handling with fail-fast behavior and proper error propagation
- Update command_volume_fsck to use error-returning saveFn callback
- Enhance error messages in readFilerFileIdFile with detailed context
* refactoring
* fix error format
* atomic
* filer_pb: make enqueue return void
* shell: simplify fs.meta.save error handling
* filer_pb: handle enqueue return value
* Revert "atomic"
This reverts commit 712648bc35.
* shell: refine fs.meta.save logic
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
* Added global http client
* Added Do func for global http client
* Changed the code to use the global http client
* Fix http client in volume uploader
* Fixed pkg name
* Fixed http util funcs
* Fixed http client for bench_filer_upload
* Fixed http client for stress_filer_upload
* Fixed http client for filer_server_handlers_proxy
* Fixed http client for command_fs_merge_volumes
* Fixed http client for command_fs_merge_volumes and command_volume_fsck
* Fixed http client for s3api_server
* Added init global client for main funcs
* Rename global_client to client
* Changed:
- fixed NewHttpClient;
- added CheckIsHttpsClientEnabled func
- updated security.toml in scaffold
* Reduce the visibility of some functions in the util/http/client pkg
* Added the loadSecurityConfig function
* Use util.LoadSecurityConfiguration() in NewHttpClient func
Chris Lu