Commit Graph
3 Commits
Author SHA1 Message Date
Chris LuandClaude Opus 5.5 d13d975b6e shell: keep fs.mergeVolumes and fs.distributeChunks from reverting entries written while they run (#12362)
* shell: keep fs.mergeVolumes from reverting entries written during the merge

The walk reads a page of entries and moves their chunks one at a time, then
wrote each entry back unconditionally. A file overwritten in between got its
old version back, and the filer deleted the new version's chunks as garbage,
so an acknowledged write was lost without any error.

The update now carries IF_ENTRY_EQUAL over the entry as the walk read it. When
the filer refuses because the entry changed or was deleted, the copies made on
the target volumes are deleted, their plan room is released, and the entry is
skipped; the source needles stay where the filer still points.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: keep fs.distributeChunks from reverting a file overwritten while it runs

The command looks the entry up once, moves its chunks, and then wrote that
entry back unconditionally, so an overwrite during the redistribution was
reverted and the filer deleted the new version's chunks as garbage.

The update now carries IF_ENTRY_EQUAL over the looked-up entry. When the filer
refuses because the file changed or was deleted, the moved copies and any new
manifest chunks are deleted and the command reports it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: keep s3.versions.audit -heal from clearing a pointer that just moved

The heal cleared the latest-version pointer on the directory entry the walk
read, unconditionally. A version written between the audit and the heal has
already pointed the directory at a live file, and clearing that pointer hid it.
The heal now carries IF_ENTRY_EQUAL over the audited entry and reports a
refusal as a failed heal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: keep fs.meta.changeVolumeId from reverting entries written meanwhile

The walk rewrote each entry's volume ids and wrote back the copy it had read,
unconditionally, so an overwrite between the read and the write was reverted
and its chunks were deleted as garbage. The update now carries IF_ENTRY_EQUAL
over the entry as read, and a refused entry is skipped and reported.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: keep the target copies another fs.mergeVolumes run committed

moveChunk keeps the key and cookie, so two overlapping merges of the same
source into the same target write the same fid. When one run commits, the
other's update is refused, and deleting its copies deleted the needle the
committed entry now points at.

A refused update now re-reads the entry, resolving its manifests, and deletes
only the copies it no longer references. If the entry cannot be read, the
copies are left for volume.fsck.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: let a later hard-link sibling retry a merge the filer refused

The first sibling of a HardLinkId claims the move for all of them. When its
update is refused, the claim made every later sibling skip, so the shared
chunks stayed on the source volume for the rest of the run. A refused update
now releases the claim, and a sibling listed after the change retries the move
against the entry as it is now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: delete the manifest blobs a failed fold in fs.distributeChunks saved

When re-manifestizing failed partway, the blobs already uploaded were only
counted in a log line and stayed on their volumes with nothing referencing
them. They have freshly assigned fids, so they are now deleted with the
cookie-checked delete the other orphan cleanups use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: check the heal clears the pointer file name too

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: require the shell lock for fs.mergeVolumes -apply

moveChunk keeps the key and cookie, so two overlapping runs that move the same
chunk into the same volume write the same fid. Re-reading the entry before
deleting a refused run's copies covers the run that already committed, but not
one that commits after that read. With the lock, two applying runs can no
longer overlap, as volume.fsck and volume.merge already require. A dry run
still works without it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: check unreferencedTargets reads the refused entry itself

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: check unreferencedTargets keeps a copy a manifest references

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* shell: check unreferencedTargets keeps the manifest copy and skips an unreadable manifest

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-03 07:59:18 -08:00
Chris Lu a9cfbd8d3a s3: tear down the emptied .versions directory on last-version delete; drain existing residue (#10278)
* s3: routed last-version delete removes the emptied .versions directory

The routed versioned delete (routedDeleteSpecificVersion) repoints the
latest pointer and deletes the version file, but unlike the lock-path
fallback (updateLatestVersionAfterDeletion) it never tears down the
.versions/ directory it just emptied. The residue keeps the key's read
path in the self-heal rescan loop: every GET of the deleted key logs
event=surfaced plus a GetObject error until the background
EmptyFolderCleaner gets to the directory — at least two minutes away on
its delay queue, and possibly never (the queue is in-memory, bounded,
and gated on the bucket's allow-empty-folders policy). Veeam's lock
arbitration probes deleted lock keys continuously, so those windows are
always open and the log spam is chronic.

ObjectMutation DELETE gains remove_empty_parent: after the child delete,
the filer best-effort removes the parent directory in the same locked
transaction. Non-recursive on purpose — a concurrent write that lands a
new version fails the removal instead of being lost with it. The routed
last-version delete sets it on the version-file DELETE, matching the
lock-path fallback's contract.

Claude-Session: https://claude.ai/code/session_014mMYAHXZySkCCUfpRFNtSv

* s3: drain empty .versions residue on read heal and in s3.versions.audit

Directories already stranded by pre-teardown deletes (or dropped from
the EmptyFolderCleaner's bounded in-memory queue) previously re-entered
the self-heal rescan on every GET forever: the heal only cleared the
pointer and nothing ever removed the directory, and s3.versions.audit
counted the state as clean.

When the heal rescan finds no remaining version, remove the directory
outright (non-recursive, so orphan children still block and fall back to
the pointer clear) and log event=healed mode=empty_dir_removed; the next
GET takes the clean not-found path. The audit gains an empty category so
the residue is visible, and -heal removes such directories in bulk.

Claude-Session: https://claude.ai/code/session_014mMYAHXZySkCCUfpRFNtSv
2026-07-08 18:50:54 -07:00
Chris Lu 79859fc21d feat(s3/versioning): grep-able heal logs + scan-anomaly diagnostics + audit cmd (#9468)
* feat(s3/versioning): grep-able heal logs + scan-anomaly diagnostics + audit cmd

Three diagnostic additions on top of #9460, all aimed at making the next
production incident faster to triage than the one we just spent hours on.

1. [versioning-heal] grep prefix on every heal-related log line, with a
   small fixed event vocabulary (produced / surfaced / healed / enqueue /
   drain / retry / gave_up / anomaly / clear_failed / heal_persist_failed
   / teardown_failed / queue_full). One grep gives operators a single
   event stream across the produce-to-drain lifecycle.

2. Escalate the "scanned N>0 entries but no valid latest" case in
   updateLatestVersionAfterDeletion from V(1) Infof to a Warning that
   names the orphan entries it saw. This is the listing-after-rm
   inconsistency signature that pinned down 259064a8's failure — it
   should not be invisible at default log levels.

3. New weed shell command `s3.versions.audit -prefix <path> [-v] [-heal]`
   that walks .versions/ directories under a prefix and reports the
   stranded population. With -heal it clears the latest-version pointer
   in place on stranded directories so subsequent reads return a clean
   NoSuchKey instead of replaying the 10-retry self-heal loop.

* fix(s3/versioning): audit pagination, exclusive categories, ctx-aware retry

Address PR review:

1. s3.versions.audit walked only the first 1024-entry page of each
   .versions/ directory, false-positiving "stranded" on large dirs.
   Loop until the page returns < 1024 entries, advancing startName.

2. clean and orphan-only categories double-counted when a directory
   had no pointer and at least one orphan: incremented both. Make them
   mutually exclusive so report totals sum to versionsDirs.

3. retryFilerOp's worst-case ~6.3s backoff was a bare time.Sleep,
   non-interruptible by ctx. A server shutdown / client disconnect
   would wait out the budget per in-flight delete. Thread ctx through
   deleteSpecificObjectVersion -> repointLatestBeforeDeletion /
   updateLatestVersionAfterDeletion -> retryFilerOp; backoff now uses
   a select{<-ctx.Done(), <-timer.C}. HTTP handlers pass r.Context();
   gRPC lifecycle handlers pass the stream ctx.

   New test pins the behavior: cancelling ctx mid-backoff returns
   ctx.Err() in <500ms instead of blocking ~6.3s.

* fix(s3/versioning): clearStale outcome + escape grep-able log fields

Two coderabbit follow-ups:

1. Successful pointer clear should suppress `produced`.
   updateLatestVersionAfterDeletion's transient-rm fallback called
   clearStaleLatestVersionPointer best-effort, then unconditionally
   returned retryErr. The caller (deleteSpecificObjectVersion) saw the
   error and emitted `event=produced` + enqueued the reconciler, even
   though clearStaleLatestVersionPointer had just driven the pointer to
   consistency and the next reader would get NoSuchKey via the
   clean-miss path. Make clearStaleLatestVersionPointer return cleared
   bool; on success the caller returns nil so neither produced nor the
   reconciler enqueue fires. Concurrent-writer aborts, re-scan errors,
   and CAS mismatches still report false so genuinely stranded state
   keeps surfacing.

2. Escape user-controlled fields in heal log lines.
   versioningHealInfof / Warningf / Errorf interpolated raw bucket /
   key / filename / err text into a single-space-separated line. An S3
   key (or error string from gRPC) containing whitespace, newlines, or
   `event=...` could split one event into multiple tokens and spoof
   fake fields downstream. Sanitize each arg in the helper: safe
   values pass through; anything with whitespace, quotes, control
   chars, or backslashes is replaced with its strconv.Quote form. No
   caller changes — the format strings remain unchanged.

Tests pin both behaviors: sanitization table covers the field
boundary cases; an end-to-end shape test confirms a key containing
`event=spoof` stays inside a single quoted token.
2026-05-13 10:48:58 -07:00