Github-Mirror/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-09-08 15:41:15 +02:00
Code Issues Packages Projects Releases Wiki Activity
13,866 Commits 177 Branches 359 Tags
01b3e4a71cd5c4a64765f1ebca97a98cf55b571f
Commit Graph
100 Commits
This Branch
This Branch
All Branches
Author SHA1 Message Date
Chris Lu 6cab199400 fix(iceberg): dial filer gRPC address verbatim in plugin worker (#9527) 
* fix(iceberg): dial filer gRPC address verbatim in plugin worker

dialFiler was running its address argument through pb.ServerAddress.ToGrpcAddress,
whose single-port fallback adds +10000 to any host:port — so when the admin
forwards ClusterContext.FilerGrpcAddresses (already host:grpcPort) to the worker,
the iceberg handler turns the real gRPC port (e.g. 18888) into a non-existent
28888 and dispatched jobs fail with connection refused.

Drop the conversion; the address is already dialable. Tests that produced fake
filer addresses in dual-port form now return host:grpcPort to match the new
contract.

* test(ec): use renamed detection_interval_minutes field

The admin_runtime.detection_interval_seconds field was renamed to
detection_interval_minutes back in May. This integration test was not
updated, so the unknown JSON field was silently ignored and the scheduler
fell back to the default detection interval (17 min for erasure_coding),
which exceeds the test's 5-minute wait and times out.

Switch to detection_interval_minutes: 1 — local run completes in ~120s.
 2026-05-17 23:03:00 -07:00
Chris Lu c11ff6657b fix(ec): mirror EC sidecars onto every shard-bearing disk at startup (#9525) 
* fix(ec): mirror EC sidecars onto every shard-bearing disk at startup

In a multi-disk volume server, ec.balance and ec.rebuild can land shards
on a disk that does not also hold the matching .ecx / .ecj / .vif index
files. The orphan-shard reconciler in reconcileEcShardsAcrossDisks
already loads those shards by pointing the EcVolume at the sibling
disk's index files; reads work, but any failure on the index-owning
disk silently disables every shard on the other disk, even though those
shards are physically fine.

This change adds mirrorEcMetadataToShardDisks, a startup pass that
physically replicates .ecx / .ecj / .vif onto each disk that holds
shards but is missing them. Each copy is atomic (tmp + fsync + rename)
and idempotent (a destination that already has the sidecar is
preserved). After mirroring, the cross-disk reconciler prefers the
local IdxDirectory so the EcVolume mounts self-contained; the
cross-disk virtual mount remains as a fallback for volumes whose mirror
failed (read-only target, out of space, partial copy on a previous
boot).

The same-disk invariant the EC lifecycle (encode / decode / balance /
vacuum / repair) was already documented as promising is now actually
restored at boot, so a future failure of one disk in a split-shards
layout no longer takes the other disk's shards with it.

Tests cover the orphan-layout mirror (dir0 receives the .ecx / .ecj /
.vif from dir1) and idempotency (an existing destination .ecx is not
overwritten with the owner's copy).

* fix(ec): handle legacy pre-dir.idx sidecar layout in mirror skip-check

hasAllEcSidecarsLocally checked only the modern destination path
(IdxDirectory for .ecx/.ecj, Directory for .vif). A destination disk
that still had a legacy .ecx in its data dir (written before -dir.idx
was set) would report "not present" and the mirror would write a
second copy to IdxDirectory, leaving two .ecx files on disk.

Matches HasEcxFileOnDisk's open-with-fallback contract: check the
modern path first, then the opposite directory. Factored the
exists-and-not-a-dir check into a small statRegular helper so the
fallback ladder stays readable.

* rust(seaweed-volume): mirror EC sidecars onto shard-bearing disks at startup

Port of the Go fix (commit 088e26ea6) to the Rust volume server.
Adds Store::mirror_ec_metadata_to_shard_disks, called from
add_location / load_new_volumes before the cross-disk orphan
reconciler. Physically copies .ecx / .ecj / .vif from the disk that
owns the index files onto every disk holding shards but missing
sidecars, so each shard-bearing disk ends up self-contained.

The reconciler now prefers the local idx_directory when the mirror
has installed a .ecx there; the cross-disk virtual mount remains as
the fallback for volumes whose mirror failed (read-only target, out
of space, partial copy on a previous boot). Adds ec_local_ecx_path
helper shared between reconcile and mirror to detect the post-mirror
fast path.

Mirrors the Go-side fallback in hasAllEcSidecarsLocally: when
-dir.idx is configured and the destination still has a legacy .ecx
in its data dir, that's recognized so the mirror does not write a
duplicate copy into idx_directory.

Tests cover the two key cases: orphan layout (dir0 receives the
sidecars from dir1) and idempotency (a pre-existing destination .ecx
is not overwritten).

* trim verbose comments on EC mirror code

Comments now lead with the WHY (non-obvious constraints, the
post-mirror fast path, why local copies are authoritative) and drop
restate-the-code blocks, headers, and section dividers. Behavior is
unchanged; all existing tests still pass on both the Go volume
server and the seaweed-volume Rust port.

* drop github issue refs from added comments

Two stray "#9212" references slipped into comments I added on the
cross-disk reconciler call site. The git log carries the issue
history; comments stand on their own.

* test(ec): accept rebuild on either disk after sidecar mirror

TestEcLifecycleAcrossMultipleDisks asserted the rebuilt shard 9 must
land at the disk-0 path. With the boot-time sidecar mirror, every
shard-bearing disk owns its own .ecx, so VolumeEcShardsRebuild now
picks whichever disk hosts the most shards — disk 1 in this layout
after the deletion. The shard can legitimately rebuild on either
disk; the test now accepts both and uses the chosen path for the
subsequent mount + read verification.
 2026-05-17 19:55:15 -07:00
Chris Lu 6b94701213 mini: quieter startup with a docker-compose-style progress board (#9524) 
* mini: quieter startup with a docker-compose-style progress board

Replaces noisy startup/shutdown logs with a single in-place progress
table on a TTY (or one line per state change off-TTY). Each component
renders as `pending -> starting -> ready` during startup and
`stopping -> stopped` during shutdown, with elapsed time on transition.

Also folds in a few cleanups uncovered while making this readable:

- route the admin.go startup prints through glog so quietMiniLogs()
  filters them under mini but standalone weed admin still shows them
- generate a dev SSE-S3 KEK + passphrase on first run via WEED_S3_SSE_KEK
  and WEED_S3_SSE_KEK_PASSPHRASE env vars (viper.Set has a nested-key
  conflict between s3.sse.kek and s3.sse.kek.passphrase); persisted under
  the data folder so restarts reuse the same key
- demote worker/master gRPC Recv 'context canceled' to V(1); those are
  the normal shutdown signal, not Errors/Warnings
- drop the 'Optimized Settings' block and the 'credentials loaded from
  environment variables' message from the welcome banner
- only show the credentials setup hints when no S3 identities exist
  (new s3api.HasAnyIdentity accessor backed by an atomic.Bool)
- use S3_BUCKET in the credentials hint so it pairs with
  AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY
- reorder running-services list to master / volume / filer / webdav /
  s3 / iceberg / admin

* mini: refuse in-memory-only SSE-S3 dev keys; surface admin serve errors

loadOrCreateMiniHexSecret returns "" when os.WriteFile fails, so SSE-S3
won't encrypt data under a KEK that the next restart can't reproduce
(which would orphan whatever was written this run). The caller already
treats "" as "skip setting WEED_S3_SSE_* env vars", so SSE-S3 and IAM
just stay disabled for this run.

startAdminServer's serve goroutine used to only log ListenAndServe
failures, so a bind error left the caller blocked on ctx.Done() with
no listener. Forward the error through a buffered channel and select
on it alongside ctx.Done().

* ci(s3-proxy-signature): match weed mini's new progress-board ready line

The readiness probe grepped for "S3 (gateway|service).*(started|ready)",
which matched weed mini's old "S3 service is ready at ..." line. Mini
now emits "  S3           ready (Xs)" from its progress board, so the
old pattern misses and the test timed out at the 30-second wait.

Widen the alternation to also accept "S3\s+ready". The curl HEAD
fallback already covers any remaining cases.
 2026-05-17 19:13:09 -07:00
Chris Lu ff6f9fd90a iam: honor configured credential store for IAM API policies and propagate to S3 caches (fixes #9518) (#9522) 
* iamapi: route managed policies through credential manager (fixes #9518)

CreatePolicy via the IAM API wrote straight to the filer
/etc/iam/policies.json, ignoring any non-filer credential store. When
credential.postgres was configured, policies created via the IAM API
landed only in the filer while the Admin UI wrote to postgres,
producing a split-brain where ListPolicies/GetPolicy never saw the
Admin UI's policies and vice versa.

GetPolicies/PutPolicies on IamS3ApiConfigure now load managed policies
from credentialManager and persist Create/Update/Delete as a delta
against the store. Inline user/group policies still live in the legacy
policies.json file (no credential-store API for them yet). Pre-existing
managed policies in the legacy file are merged on read so deployments
don't lose data, and re-persisted to the store on the next write so
the legacy file is drained over time.

* credential: route IAM API inline policies through credential manager

Extends the #9518 fix to user-inline and group-inline policies so the
IAM API never writes the legacy /etc/iam/policies.json bundle directly.
The previous patch only routed managed policies; this one finishes the
job for the other two policy types.

- Add GroupInlinePolicyStore + GroupInlinePoliciesLoader optional
  interfaces, mirroring the existing user-inline ones, and matching
  Put/Get/Delete/List/LoadAll wrappers on CredentialManager.
- Implement group-inline storage in memory (new map), filer_etc (new
  field on PoliciesCollection, reusing the legacy file under policyMu),
  and postgres (new group_inline_policies table with ON DELETE CASCADE
  off the groups FK).
- Wire the new methods through PropagatingCredentialStore so wrapped
  stores still delegate correctly.
- IamS3ApiConfigure.PutPolicies now applies managed + user-inline +
  group-inline as deltas through the credential manager; the legacy
  /etc/iam/policies.json file is never written when a credential
  manager is wired up. GetPolicies still reads the legacy bundle once
  as a fallback so unmigrated data is picked up and re-persisted into
  the store on the next write.

* credential: propagate SaveConfiguration writes to running S3 caches

Postgres (and any non-filer) credential stores never fired the S3 IAM
cache invalidation path on bulk identity / group updates. The
PropagatingCredentialStore had explicit Put/Remove handlers for
single-entity calls (CreateUser, PutPolicy, etc.) but inherited
SaveConfiguration unchanged from the embedded store, so the bulk path
the IAM API takes at the end of every handler was silent. Inline-policy
changes recompute identity.Actions and persist via SaveConfiguration,
so until restart the cached Actions on each S3 server stayed stale and
authorization decisions used the pre-change view.

Override SaveConfiguration to snapshot the prior user / group lists,
delegate the save, then fan out PutIdentity / PutGroup for what's in
the new config and RemoveIdentity / RemoveGroup for what got pruned.
Reuses the existing SeaweedS3IamCache RPCs, no protobuf changes.

* iamapi: drain legacy policies.json after authoritative credential-store writes

Review pointed out a resurrection bug: GetPolicies still reads
/etc/iam/policies.json as a one-way migration fallback, but PutPolicies
in the credential-manager path never wrote that file, so legacy-only
entries reappeared on the next read even after the IAM API "deleted"
them. PutPolicies now overwrites the bundle with an empty {} after a
successful credential-store write, unless the store is filer_etc
(which owns the bundle as its own inline-policy backing — clearing it
would wipe filer_etc's data). Also wraps the filer read, JSON
unmarshal, and marshal errors with context per the other review
comments.
 2026-05-17 13:15:27 -07:00
Chris Lu b4289abb0a admin: convert filer address to gRPC form before dispatch (#9523) 
The master returns each registered filer in pb.ServerAddress dual-port
form (host:httpPort.grpcPort, e.g. 10.0.0.1:8888.18888). The admin's
plugin context builder forwarded that string verbatim as
filer_grpc_address, so workers calling grpc.DialContext on it failed
every job in ~3ms with "dial tcp: lookup tcp/8888.18888: unknown port".

Run each entry through pb.ServerAddress.ToGrpcAddress before populating
ClusterContext.FilerGrpcAddresses.

The lifecycle integration test now pins filer.port.grpc to a value that
breaks the FILER_PORT+10000 assumption, and a new dispatch test drives
the admin's /api/plugin/job-types/s3_lifecycle/run path end-to-end and
asserts the dispatched job both reaches the filer and deletes the
backdated object.
 2026-05-17 11:33:54 -07:00
Chris Lu 2a41e76101 fix(ec): blanket-clean every destination over the full shard range (#9512) 
* fix(ec): blanket-clean every destination over the full shard range

The previous cleanup pass walked t.sources only, with the shard ids the
topology had reported at detection time. In the wild, a destination can
end up with EC shards mounted that the topology snapshot didn't list —
shards on a sibling disk that hadn't heartbeated, or shards left over
from a concurrent attempt's mount step. FindEcVolume still returns
true, so the next ReceiveFile trips the mounted-volume guard.

Cleanup now unions t.sources (with ShardIds) and t.targets and issues
unmount + delete over [0..totalShards-1] on each. Both RPCs are
idempotent on missing shards, so the wider sweep is free.

Two new tests cover the gap: shards mounted beyond what t.sources
lists, and a target-only destination with no source row.

* log(ec): include disk_id in EC unmount/delete/refusal log lines

The current logs identify the volume and shard but leave disk_id off,
which makes the cross-server cleanup story hard to follow when
multiple disks of one server hold pieces of the same volume:

  UnmountEcShards 4121.1                              -> add disk_id
  ec volume video-recordings_4121 shard delete [1 5]  -> add per-loc disk_id
  volume server X:Y deletes ec shards from 4121 [...] -> add disk_id
  ReceiveFile: ec volume 4121 is mounted; refusing... -> add disk_ids

ReceiveFile's refusal now names the disk_ids actually holding the
mount so operators can see whether the next cleanup pass needs to
target a sibling disk. Added Store.FindEcVolumeDiskIds /
Store::find_ec_volume_disk_ids as the supporting primitive.

Mirrored in seaweed-volume/src/ (unmount log in Store::unmount_ec_shard,
heartbeat delete log in diff_ec_shard_delta_messages, refusal in the
ReceiveFile handler).

* test(ec): stub VolumeEcShardsUnmount/Delete on the fake volume server

The plugin-worker EC tests boot a fake volume server that embeds
UnimplementedVolumeServerServer. After the worker started calling
VolumeEcShardsUnmount + VolumeEcShardsDelete pre-distribute, the
default Unimplemented response surfaced as fourteen "method not
implemented" errors and TestErasureCodingExecutionEncodesShards
failed. Both RPCs are no-ops here — nothing on the fake server has
mounted state or persisted shard files to remove.
 2026-05-17 11:31:37 -07:00
Chris Lu bf9110ebd3 fix(ec): mount falls back to sibling-disk .ecx (fixes #9519) (#9521) 
* fix(ec): mount falls back to sibling-disk .ecx (fixes #9519)

MountEcShards iterated DiskLocations and on each disk called LoadEcShard
with that disk's IdxDirectory as the .ecx home. When ec.balance lands the
.ec?? shard on disk A but the .ecx on sibling disk B of the same volume
server, NewEcVolume ENOENTs the .ecx and returns "cannot open ec volume
index ...". That error is not os.ErrNotExist, so the per-disk continue
branch did not engage and the mount loop bailed before trying any other
disk.

The startup reconciliation in reconcileEcShardsAcrossDisks already
handles this layout for orphan shards discovered on boot (issue #9212).
This change mirrors the same primitive on the mount path: look up the
.ecx owner across all DiskLocations once and route NewEcVolume at that
directory whenever the disk being mounted does not own its own copy of
the .ecx. Same-disk mounts are unaffected because HasEcxFileOnDisk keeps
LocalIdxDirectory in play.

Adds a regression test that plants the index files on a sibling disk
AFTER NewStore returns (so the startup reconcile is a no-op for that
vid) and verifies MountEcShards succeeds; also pins the same-disk
baseline against accidental re-routing.

* fix(ec): skip redundant stats in cross-disk .ecx lookup (review)

Two follow-ups from gemini-code-assist on #9521:

1. MountEcShards: when findEcxIdxDirForVolume already returned a path
   that lives on this disk's IdxDirectory or Directory, the disk owns
   the .ecx — skip the HasEcxFileOnDisk stat and use the local idx dir
   directly. Only re-check when the disk's directories are neither, so
   the duplicate-.ecx-on-multiple-disks edge case is still honored.

2. findEcxIdxDirForVolume: hoist the seen map across the location loop
   so a shared IdxDirectory (one -dir.idx paired with several -dir
   entries) is only stat'd once per call.

Both are I/O optimizations; behavior is unchanged. Existing
cross-disk and same-disk regression tests still pass.

* docs(ec): drop issue/PR references from cross-disk mount comments

Comments and test docstrings stand on their own; the issue number
adds nothing a reader can act on and goes stale across forks. Keep
the description of *what* the layout is and *why* the fallback
exists, just without the reference.
 2026-05-17 11:10:37 -07:00
Chris Lu d51454adf4 rust(seaweed-volume): distributed EC read across peer servers (#9516) 
* feat(seaweed-volume): distributed EC read across peer servers

EcVolume::read_ec_shard_needle previously errored with NotFound when
any interval's shard wasn't local. In an RS(10,4)-across-N deployment
each server holds one shard, so every read needed >=9 peer fetches and
post-EC GETs returned 404 on volumes whose shards lived on more than
one server.

Mirror of weed/storage/store_ec.go's readOneEcShardInterval ->
readRemoteEcShardInterval -> recoverOneRemoteEcShardInterval chain:

  * server/store_ec.rs (new): entry point
    read_ec_shard_needle_distributed. Snapshots locate-needle + local
    reads under the Store sync lock, drops the lock, then async-fetches
    missing intervals via the peer's VolumeEcShardRead RPC. Falls back
    to Reed-Solomon reconstruction (read every other shard at the same
    (shard_offset, size) and run rs.reconstruct) when the direct peer
    read fails. Refreshes the per-EcVolume shard_locations cache from
    the master's LookupEcVolume RPC using Go's freshness thresholds
    (11s / 7min / 37min).
  * erasure_coding/ec_volume.rs: shard_locations now sits behind a
    std::sync::RwLock so the read path can refresh the map without
    holding the Store write lock. Adds shard_locations_refresh_time
    (Mutex<Option<Instant>>) for the staleness heuristic. Mirrors Go's
    ShardLocationsLock / ShardLocationsRefreshTime fields. set/get
    helpers updated for interior mutability.
  * server/handlers.rs: GET handler now tries the local-only fast
    path first, then falls through to the distributed path on
    NotFound.

* review: address PR 9516 feedback on distributed EC read

Five of the six PR-review comments addressed; the sixth (JWT on
outgoing peer gRPC) is deferred with an explicit TODO because the
crate-wide outgoing-JWT signing surface doesn't exist yet — adding it
in this one call site would split the credential plumbing across
peer paths that already lack it (copy_file_from_source, batch_delete,
…). Revisit when an outgoing-JWT helper lands.

Fixed in this commit:

  * Handlers: drop the two-tier (local-first, then distributed)
    read in handlers.rs. read_ec_shard_needle_distributed already
    does the local-first pass under the same store read lock; the
    redundant outer attempt re-read local intervals twice for any
    needle that spanned mixed-locality shards.
  * Scanner snapshot: replace inline locate-needle math with
    `ecv.locate_needle(needle_id)`. Same routine the local-only
    read path uses, so byte-identical on shard-size + interval
    boundaries.
  * EcVolume::set_shard_locations also advances
    shard_locations_refresh_time so the staleness check honors
    callers that populate the cache directly without going through
    the master LookupEcVolume RPC.
  * parse_grpc_address moved from grpc_server.rs into
    grpc_client.rs as `pub` and is reused by both grpc_server.rs
    and the new store_ec module. Single source of truth for the
    HTTP↔gRPC port-offset convention.
  * Reconstruction (recover_one_remote_ec_shard_interval) now seeds
    bufs from locally-mounted survivor shards BEFORE the remote
    fan-out. Previously the fan-out was remote-only, so when the
    shard_locations cache was cold or the master lookup failed,
    reconstruction errored even though enough siblings were on
    local disk to recover the missing interval.

* review: tighten parse_grpc_address; atomic shard-locations cache swap

Two follow-up findings from the PR 9516 review round 2:

  * `parse_grpc_address` now validates BOTH port components in the
    dotted form (`host:port.grpcPort`) — previously a non-numeric
    HTTP port like `host:abc.18080` slipped through and tripped a
    less-useful downstream URI parse error. The implicit form
    (`host:port` → port + 10000) also gains an overflow check so
    inputs like `host:60000` (which silently wrap past u16) are
    rejected here instead of producing an opaque connection
    failure later. Six unit tests cover each rejection path.

  * `EcVolume::set_shard_locations` no longer bumps the per-volume
    refresh timestamp. The previous fix introduced a freshness
    race: a multi-shard population that inserts shard-by-shard
    would flip `needs_refresh == false` on the first write, letting
    a concurrent reader observe a half-populated map already
    marked "fresh" and return NotFound for the not-yet-inserted
    shards. Added `EcVolume::replace_shard_locations(map)` for the
    atomic bulk swap; `write_back_shard_locations` in the
    distributed-read path uses it so the cache transitions
    old → fresh in a single observable step.
 2026-05-16 20:44:28 -07:00
Chris Lu f892b445b3 helm(admin): support secretExtraEnvironmentVars (refs #9511) (#9513) 
* helm(admin): support secretExtraEnvironmentVars

The admin statefulset only honored extraEnvironmentVars, forcing the
OIDC client secret (and any other sensitive WEED_* value) to be inlined
as plain text in values.yaml — not GitOps-friendly. The filer chart has
had secretExtraEnvironmentVars for this exact case; mirror that pattern
on admin so secrets can be projected via valueFrom.secretKeyRef.

Surfaced by an enterprise OIDC deployment (issue #9511) where the only
workaround was hardcoding WEED_ADMIN_OIDC_CLIENT_SECRET in values.yaml.

* helm(admin): sort secretExtraEnvironmentVars keys for stable output

Helm/Go template map iteration is non-deterministic, so the env entries
could shuffle between renders and trigger spurious StatefulSet rollouts
in GitOps tooling (ArgoCD/Flux). Sort the keys with sortAlpha, mirroring
the extraEnvironmentVars block immediately above.

Flagged by gemini-code-assist and coderabbitai on PR #9513.
 2026-05-15 13:19:05 -07:00
Chris Lu 62821964dd filer/iam-grpc: make admin Bearer auth opt-in (fixes #9509) (#9514) 
PR #9442 made the filer refuse to register the IAM gRPC service unless
jwt.filer_signing.key was set in security.toml, which broke the admin
UI Users/Groups/Policies pages for every deployment that ships without
a security.toml — weed mini, plain Helm, vanilla weed filer. The Users
tab returns Unimplemented and the page is unusable. Issues #9504,
#9505 and #9509 all trace to this gap.

The rest of the filer's gRPC surface is unauthenticated by default;
treat IAM the same way. The service now always registers, and the
auth gate is a no-op when no signing key is configured. When the key
is set, every RPC still requires an admin-signed Bearer token, matching
the post-#9442 behaviour. Operators who expose the filer gRPC port
beyond a trusted network should set the key on both filer and admin.

The admin client (IamGrpcStore.withIamClient) already skips attaching
the authorization metadata when its key is empty, so no changes there.
 2026-05-15 13:15:20 -07:00
Chris Lu 2ed95d7ea9 helm: decouple JWT signing from cert-manager mTLS (fixes #9506) (#9508) 
* helm(security): decouple JWT signing from cert-manager mTLS

The filer needs jwt.filer_signing.key to register the IAM gRPC service the
Admin UI Users tab calls (PR #9442). The chart only rendered security.toml
under enableSecurity, which also pulls in cert-manager for mTLS — much heavier
than the Admin UI needs. Operators on Helm without cert-manager have no way
to flip the JWT key on, so the Users tab fails with Unimplemented after
upgrading past 4.24.

Introduce seaweedfs.securityConfigEnabled, true when enableSecurity OR any
explicit jwtSigning toggle (volumeRead/filerWrite/filerRead) is set. The
configmap renders under that helper; the [grpc.*]/[https.*] sections inside
stay gated on enableSecurity. Each pod template splits the security-config
mount onto the helper and keeps the cert volume mounts on enableSecurity.

volumeWrite is intentionally excluded from the helper trigger because it
defaults to true; including it would silently start mounting security.toml on
every fresh install. With this change, enableSecurity=false + defaults
renders nothing (unchanged), enableSecurity=true renders the full toml
(unchanged), and enableSecurity=false + filerWrite=true renders just the
[jwt.*] sections so the Admin UI works without mTLS.

Fixes #9506.

* helm(security): trim verbose comments

* helm(security): handle null securityConfig in helper

Address review feedback: (.Values.global.seaweedfs.securityConfig).jwtSigning
errored if a user explicitly set securityConfig: null in their values. Drop
into intermediate $sec/$jwt with default dict at each step so a missing or
nulled-out parent is tolerated.

* helm(ci): cover IAM gRPC decoupling (issue #9506)

Five regression assertions exercised against the rendered chart so a
future change cannot silently re-couple jwt.filer_signing to mTLS:

1. defaults render no security-config ConfigMap (preserves baseline)
2. filerWrite=true alone renders [jwt.filer_signing] with no [grpc.*]
3. filerWrite=true mounts security-config on filer + admin without
   pulling in cert volumes — the actual fix for the Admin UI Users tab
4. enableSecurity=true still produces the full toml with [grpc.master]
5. securityConfig=null and securityConfig.jwtSigning=null both render
   cleanly (gemini-code-assist review nit, applied chart-wide)

Patch a pre-existing direct-access in filer-statefulset.yaml that
crashed on securityConfig=null, surfaced by the new null assertion.

* helm(ci): drop issue numbers from comments

* helm(ci): install pyyaml; assert [jwt.signing] in mTLS path

Address coderabbit review:

- The new IAM gRPC test block uses `import yaml` but ran before the
  later `pip install pyyaml -q` step that the security+S3 block
  performs. CI happens to pass because the runner image carries
  PyYAML, but make the dependency explicit so a future runner change
  cannot silently break the regression test.

- The enableSecurity=true assertion only checked for [grpc.master].
  Also assert [jwt.signing] so a refactor that drops the volume-side
  JWT stanza from the mTLS path fails the test instead of slipping
  through.
 2026-05-14 23:43:24 -07:00
Chris Lu bfb2661fec fix(tests): make 32-bit GOARCH tests build and run (#9507) 
fix(tests): make 32-bit GOARCH tests build and run (#9503)

verifyTestFilerClient had bare int64 atomic counters after a map header,
so atomic.AddInt64 panicked with "unaligned 64-bit atomic operation" on
linux/386. Switch to atomic.Int64, which the stdlib guarantees is
8-byte aligned on all platforms.

rpc_version_filter_test.go passed the untyped constant 0xdeadbeef to
t.Errorf, where it default-promoted to int and overflowed 32-bit int.
Bind it to a typed uint32 const used in both the comparison and the
error message.
 2026-05-14 20:55:37 -07:00
Chris Lu 2c1482f7a6 fix(ec): clear cross-server stale EC shards before re-distribute (#9478) (#9499) 
* fix(ec): clear cross-server stale EC shards before re-distribute (#9478)

A previous failed encode leaves partial .ec?? shards mounted on
destination volume servers that are not the .dat owner. PR #9480 only
prunes when the .dat sits on a sibling disk of the SAME store, so the
cross-server case stays stuck: every retry trips
volume_grpc_copy.go:570's "ec volume %d is mounted; refusing overwrite"
guard and the scheduler loops.

Detection already lists existing EC shards as CleanupECShards sources;
plumb the shard ids through (ActiveTopology.GetECShardLocations,
TaskSourceSpec, TaskSource.shard_ids) and have the EC worker call
VolumeEcShardsUnmount + VolumeEcShardsDelete on each destination after
the local shard set is generated and before distributeEcShards. Skip
EC-shard sources in getReplicas so the post-encode VolumeDelete step
does not target destination-only nodes.

Integration test mounts a partial shard subset, asserts the
mounted-volume refusal, runs cleanupStaleEcShards, and asserts the
next ReceiveFile lands.

* chore(ec): tighten code comments in stale-shard cleanup

Drop issue-number refs from code comments and shorten the docstrings
on cleanupStaleEcShards / unmountAndDeleteEcShards / getReplicas plus
the new test file. Behavior unchanged.

* fix(ec): skip empty-ShardIds locations; dedupe getReplicas by node

GetECShardLocations dropped entries where ecShardMatchesCollection saw a
phantom info record with EcIndexBits=0 — without ShardIds, getReplicas
misread the resulting source as a regular replica and would have called
VolumeDelete on a destination-only node.

getReplicas now dedupes by Node since VolumeDelete is server-wide;
per-disk source rows on the same server collapse to one call.

* refactor(ec): use MaxShardCount and ShardBits in collectShardIdsForDisk

Drop the literal 32 bit-iteration bound for erasure_coding.MaxShardCount
and treat the EcIndexBits union as a ShardBits so Count() drives the
slice preallocation. Keeps the helper aligned with the rest of the EC
code and survives any future expansion of the shard-count ceiling.
 2026-05-14 11:57:45 -07:00
Chris Lu db2d975b80 ci(docker): tag latest in unified release instead of rebuilding (#9500) 
The separate container_latest.yml workflow rebuilt the latest image from
scratch on every tag push (full multi-arch build + QEMU + trivy gate),
which is slow and frequently fails — leaving `latest` stranded on the
prior release (e.g. 4.23 after 4.24 shipped, #9497).

Drop the rebuild. The unified release workflow already publishes the
exact same content as `<tag>` and `<tag>_large_disk`, so just re-tag
those manifests with `crane tag` on both GHCR and Docker Hub once
copy-to-dockerhub completes. Seconds, not hours, and no QEMU.

Move the trivy scan into the unified workflow as report-only: SARIF
still uploads to GitHub Security for visibility, but vuln findings
no longer block the release.

container_latest.yml stays as a workflow_dispatch-only manual fallback.

Refs #9497.
 2026-05-14 11:26:28 -07:00
Chris Lu c47eab1a5d admin: attach admin-signed Bearer token on filer IAM gRPC calls (#9498) 
* admin: attach admin-signed Bearer token on filer IAM gRPC calls

PR #9442 added Bearer-JWT enforcement on the filer's IAM gRPC service
but didn't update its only production client, IamGrpcStore. The admin
UI Users/Groups pages went through that client and started failing in
4.24 with either Unimplemented (filer refuses to register the service
when jwt.filer_signing.key is empty) or Unauthenticated (the client
sent no token). Issues #9495 and #9496 both trace to this gap.

Plumb jwt.filer_signing.key into IamGrpcStore via a new SetAdminSigning
hook called from the admin server, and append a freshly minted Bearer
token to outgoing metadata on every call. The mint helper
security.GenJwtForFilerAdmin existed since #9442 but had no production
caller; this wires it up.

Add an integration test alongside grpc_store.go that runs a real
IamGrpcServer over a real grpc.Server listener and exercises the store
end-to-end: matching key succeeds, wrong key returns Unauthenticated,
no key returns Unauthenticated. Without the client-side token attach
the success path fails, so the regression cannot land again.

* address review: include adminSigningExpiresAfterSec in mu comment
 2026-05-14 10:51:04 -07:00
Chris Lu 3a8389cd68 fix(ec): verify full shard set before deleting source volume (#9490) (#9493) 
* fix(ec): verify full shard set before deleting source volume (#9490)

Before this change, both the worker EC task and the shell ec.encode
command would delete the source .dat as soon as MountEcShards returned —
even if distribute/mount failed partway, leaving fewer than 14 shards
in the cluster. The deletion was logged at V(2), so by the time someone
noticed missing data the only trace was a 0-byte .dat synthesized by
disk_location at next restart.

- Worker path adds Step 6: poll VolumeEcShardsInfo on every destination,
  union the bitmaps, and refuse to call deleteOriginalVolume unless all
  TotalShardsCount distinct shard ids are observed. A failed gate leaves
  the source readonly so the next detection scan can retry.
- Shell ec.encode adds the same gate after EcBalance, walking the master
  topology with collectEcNodeShardsInfo.
- VolumeDelete RPC success and .dat/.idx unlinks now log at V(0) so any
  source destruction is traceable in default-verbosity production logs.

The EC-balance-vs-in-flight-encode race is intentionally left for a
follow-up; balance should refuse to move shards for a volume whose
encode job is not in Completed state.

* fix(ec): trim doc comments on the new shard-verification path

Drop WHAT-describing godoc on freshly added helpers; keep only the WHY
notes (query-error policy in VerifyShardsAcrossServers, the #9490
reference at the call sites).

* fix(ec): drop issue-number anchors from new comments

Issue references age poorly — the why behind each comment already
stands on its own.

* fix(ec): parametrize RequireFullShardSet on totalShards

Take totalShards as an argument instead of reading the package-level
TotalShardsCount constant. The OSS callers continue to pass 14, but the
helper is now usable with any DataShards+ParityShards ratio.

* test(plugin_workers): make fake volume server respond to VolumeEcShardsInfo

The new pre-delete verification gate calls VolumeEcShardsInfo on every
destination after mount, and the fake server's UnimplementedVolumeServer
returns Unimplemented — the verifier read that as zero shards on every
node and aborted source deletion. Build the response from recorded
mount requests so the integration test exercises the gate end-to-end.

* fix(rust/volume): log .dat/.idx unlink with size in remove_volume_files

Mirror the Go-side change in weed/storage/volume_write.go: stat each
file before removing and emit an info-level log for .dat/.idx so a
destructive call is always traceable. The OSS Rust crate previously
unlinked them silently.

* fix(ec/decode): verify regenerated .dat before deleting EC shards

After mountDecodedVolume succeeds, the previous code immediately
unmounts and deletes every EC shard. A silent failure in generate or
mount could leave the cluster with neither shards nor a valid normal
volume. Probe ReadVolumeFileStatus on the target and refuse to proceed
if dat or idx is 0 bytes.

Also make the fake volume server's VolumeEcShardsInfo reflect whichever
shard files exist on disk (seeded for tests as well as mounted via
RPC), so the new gate can be exercised end-to-end.

* fix(ec): address PR review nits in verification + fake server

- Drop unused ServerShardInventory.Sizes field.
- Skip shard ids >= MaxShardCount before bitmap Set so the ShardBits
  bound is explicit (Set already no-ops on overflow, this is for
  clarity).
- Nil-guard the fake server's VolumeEcShardsInfo so a malformed call
  doesn't panic the test process.
 2026-05-13 19:29:24 -07:00
Chris Lu 0dde6a8c84 refactor(s3/lifecycle): drop Per-Run Time Limit knob; use scheduler's Execution Timeout (#9494) 
* refactor(s3/lifecycle): drop Per-Run Time Limit knob; use scheduler's Execution Timeout

"Per-Run Time Limit (minutes)" duplicated the admin scheduler's
"Execution Timeout (s)" — both are wall-clock caps on the same
Execute call, stacked via context.WithTimeout. Whichever was shorter
won. Under defaults the scheduler's 90s timeout always clobbered the
worker's 60-min cap, so the "Per-Run Time Limit" knob was effectively
dead unless an operator also raised Execution Timeout, and operators
had to keep two values in agreement.

Remove the worker-side knob and declare a sane scheduler default on
the handler descriptor:

- WorkerConfigForm: nil (was: one section with one field)
- Config.MaxRuntime removed; ParseConfig drops max_runtime_minutes
- Handler no longer wraps ctx in context.WithTimeout(MaxRuntime);
  runCtx is just the ctx the scheduler passes
- AdminRuntimeDefaults.ExecutionTimeoutSeconds = 3600 (1h) and
  JobTypeMaxRuntimeSeconds = 3600 — the scheduler's global 90s
  default would otherwise kill every real run

Tests:
- TestParseConfigDefaults loses the MaxRuntime check; new
  TestParseConfigIgnoresWorkerValues documents the contract
- TestDescriptor_WorkerConfigFormIsAbsent pins that the form is gone
  so a future re-add forces a conscious revisit
- TestDescriptor_AdminRuntimeDefaultsBoundExecutionTimeout pins the
  1h default with a comment about the 90s scheduler floor

* fix(s3/lifecycle): no per-pass timeout by default

Lifecycle is a scheduled batch — its natural duration is "as long as
today's events take." The 1h default ExecutionTimeoutSeconds from the
previous commit was still a footgun: too low truncates legitimate
large-bucket passes; too high makes the value meaningless.

Set both ExecutionTimeoutSeconds and JobTypeMaxRuntimeSeconds to
math.MaxInt32 (~68 years) to say "no timeout in practice" in a
code-review-readable way. Operators who genuinely want a wall-clock
cap can set one in the admin UI; the scheduler's context.WithTimeout
machinery is unchanged (we just hand it an effectively-infinite
duration).

Note: the scheduler floors ExecutionTimeout at 90s
(defaultScheduledExecutionTimeout in weed/admin/plugin/plugin_scheduler.go),
so 0 doesn't mean "unlimited" — it clamps back to 90s. A literal
math.MaxInt32 is the way to express the intent without touching the
shared scheduler code.

Test updated to pin math.MaxInt32 and document the rationale so a
future tighter cap fails the test and forces conscious revisit.
 2026-05-13 19:29:06 -07:00
Chris Lu e9bcb8f4ad docs(s3/lifecycle): refresh DESIGN.md as-built (#9491) 
* docs(s3/lifecycle): refresh DESIGN.md as-built + add wiki pages

DESIGN.md was written as a phased implementation plan ("Phase 2 will
ship X, Phase 4 will ship Y"). All phases are now merged, plus the
post-cutover changes from #9477/#9481/#9484/#9485/#9486 substantially
changed the worker model (single subscription, walker throttle,
observability gauges). Rewrite the doc in present tense describing
what's actually there.

Net changes vs the prior plan-style doc:
- Algorithm pseudo-code reflects the single-subscription fan-out plus
  walkedThisPass within-pass guard.
- Walker invocation table replaces the implicit "two distinct calls"
  prose with three call sites (recovery / steady-state / empty-replay)
  and their throttle gates.
- New section on the subscription model (one Reader, ShardPredicate,
  fan-out by ev.ShardID).
- New section on cursor.LastWalkedNs and the WalkerInterval throttle.
- Observability section: gauges, heartbeat tokens, what each means.
- "Implementation history" table maps phases to merged PRs.
- "Future work" lists the four optimizations we deferred (long-lived
  subscription, bucket-coordinated walker, per-bucket lag metric,
  filer meta-log retention).

Drop the "Phase N — ..." narrative from the bottom; the PR history
table is the durable artifact now.

Add wiki pages under docs/wiki/s3-lifecycle/ as source-of-truth for
the operator-facing docs. README explains the sync workflow with the
external seaweedfs.wiki.git repo. Five pages:

- Home.md — landing page, supported rule shapes, what the worker does
- Operator-Guide.md — config knobs, when to change each, walker
  interval recommendations by cluster size
- Monitoring.md — Prometheus metric reference + heartbeat token table
  + suggested PromQL alerts
- Troubleshooting.md — stuck cursor, walker stuck, failure outcomes,
  cursor schema for manual inspection
- Architecture.md — high-level overview for newcomers; sits between
  Home.md (operator) and DESIGN.md (developer)

* docs(s3/lifecycle): address PR review feedback on docs

Coderabbit + gemini findings on #9491:

- Monitoring.md: clarify the "matches all dispatched" phrasing; note
  that LIFECYCLE_DELETE_OUTCOME_UNSPECIFIED is the proto zero-value
  (shouldn't appear in healthy systems); filter PromQL alerts to
  ignore zero-valued gauges so fresh-install heartbeats don't trip.
- Operator-Guide.md, Troubleshooting.md: clarify weed shell -master
  format as host:http_port.grpc_port (SeaweedFS ServerAddress).
- Troubleshooting.md: pause the s3_lifecycle job in the admin UI
  before manually editing a cursor file, otherwise the worker's
  save races with the operator's edit.
- Architecture.md, Home.md, Operator-Guide.md, Monitoring.md,
  Troubleshooting.md, DESIGN.md: add language tags (`text`) to
  fenced code blocks for markdownlint MD040 compliance.
- DESIGN.md: standardize on the S3 spec rule names
  (`ExpiredObjectDeleteMarker`, `NewerNoncurrentVersions`,
  `AbortIncompleteMultipartUpload`) and add a one-line note mapping
  them to the engine's `ActionKind*` constants.
- README.md: prepend `cd "$(git rev-parse --show-toplevel)"` to the
  sync workflow so the `cp` commands' repo-root-relative paths work
  whether the operator's shell is at the repo root or at
  docs/wiki/s3-lifecycle/.
- Home.md: was lagging the wiki-repo merged version (had the older
  pre-merge content). Re-sync from the wiki repo so source matches.

* docs(s3/lifecycle): remove wiki pages from PR

The wiki pages belong in seaweedfs.wiki.git, not the main repo. The
source-of-truth concern that motivated adding them here is real but
the cost — every code-review touchpoint requires reviewers to load
operator-facing pages too — outweighs it. The wiki pages are already
pushed locally (~/dev/seaweedfs.wiki); they'll publish on the
operator-side workflow.

This PR remains scoped to DESIGN.md (the developer-facing reference
that does belong with the code).

* docs(s3/lifecycle): drop Implementation history section

git log is the durable record of what shipped when; the prose table
duplicates it and goes stale faster than commit metadata.

* docs(s3/lifecycle): soften 'exactly once per run' in Goal

The prior phrasing overstated the guarantee versus the failure model
documented later in the same file. Reword to: 'process due objects
each pass; retryable/blocked outcomes get retried from the cursor on
later runs.' Surfaces the head-of-line-blocking semantics up front so
the rest of the doc reads consistently.

Also: drop the stale 'see docs/wiki/s3-lifecycle/' pointer — those
pages live in the wiki repo, not the main repo.
 2026-05-13 17:06:14 -07:00
Chris Lu 813f1351f8 feat(s3/lifecycle): enable scheduler by default (#9492) 
S3 lifecycle is a standard bucket feature — operators set
PutBucketLifecycleConfiguration through the S3 API expecting the
configured expirations to actually fire. With the prior default
(scheduler enabled=false), buckets with lifecycle XML silently
retained data past their declared expiration until an operator
noticed and turned the scheduler on.

The failure mode of enabled-by-default is "worker runs every day
and fast-exits on buckets with no lifecycle rules" — cheap.
The failure mode of disabled-by-default is "data lingers, looks
like it expired, doesn't" — bad. Enabled-by-default matches both
the AWS S3 default behavior and the operator's natural mental
model.

Operators who want the worker off can still disable it via the
admin UI; once a persisted config exists, this descriptor default
no longer applies (the persisted Enabled state wins).

Test pins the choice so a future flip to false fails loud.
 2026-05-13 16:57:10 -07:00
Chris Lu d5e54f217d feat(s3/lifecycle): publish per-shard cursor + walker gauges and heartbeat (#9486) 
Operator visibility was the last item on the daily-replay must-have
list. The `S3LifecycleCursorMinTsNs` gauge already existed but nothing
ever set it — leftover from the streaming worker that got deleted.
Wire it up and add a parallel one for the walker so a single PromQL
query answers "is this thing working?":

- `cursor_min_ts_ns{shard}` set after each cursor save. Operators read
  `now - cursor_min_ts_ns` as the per-shard replay lag.
- `daily_run_last_walked_ns{shard}` new — set in parallel so operators
  can confirm WalkerInterval is actually being honored. A stuck value
  means the scheduler isn't invoking the worker, the throttle is too
  long, or the walker is failing.
- saveCursorAndPublish wraps every Save call site in runShard so the
  gauges and the persisted state stay aligned (gauges only advance on
  successful saves).
- Enhance the `daily_run: status=... duration=...` heartbeat with
  `cursor_lag_max=` and `walked_max_age=` summary tokens for ops grep.
  Existing tokens stay positional-stable; new ones append at the end.
  Marker `cold` distinguishes "not started" from "0s caught up."

Tests pin the summary line: cold-start state, max-across-shards
selection, and partial-fill (some shards drained, others walked).

Stacked on #9485.
 2026-05-13 14:18:35 -07:00
Chris Lu bbc075b353 feat(s3/lifecycle): plumb WalkerInterval through worker admin config (#9485) 
* feat(s3/lifecycle): throttle steady-state walker by cfg.WalkerInterval

The steady-state and empty-replay walker fired on every dailyrun.Run
invocation, which is fine when Run is called at the bucket-walk cadence
the operator intends (e.g., once per hour or once per day), but
catastrophic when a fast driver like the s3tests CI workflow or the
admin worker scheduler invokes Run at multi-second cadence — each tick
ran a full subtree scan per shard, crushing the filer.

Decouple walker cadence from Run() invocation cadence: persist
LastWalkedNs in the per-shard cursor and fire the steady-state /
empty-replay walker only when (runNow - LastWalkedNs) >= cfg.WalkerInterval.
Cold-start and recovery walker fires (RecoveryView) stay unconditional
since those are bounded events that must run when their trigger
condition (no cursor, hash mismatch) is met. Recovery walker fires also
update LastWalkedNs so the subsequent steady-state pass doesn't
double-walk.

cfg.WalkerInterval=0 keeps the prior "fire every pass" behavior — the
in-repo integration tests and s3tests fast driver continue to work
unchanged. Production deployments should set this to the walk cost
budget (typically 1h-24h depending on cluster size).

Cursor file is back-compat: last_walked_ns is omitempty, so cursor
files written before this change decode as LastWalkedNs=0, which
walkerDue treats as "never walked steady-state" → walker fires next
pass to establish the anchor (same path a cold-start cursor takes).
No version bump.

Operator surface for WalkerInterval is the dailyrun.Config struct;
plumbing through worker.tasks.s3_lifecycle.Config and the admin
schema is a follow-up.

* fix(s3/lifecycle): suppress walker double-fire within a single pass

Two gemini-code-assist findings:

1. walkerDue with interval=0 returned true even when lastWalkedNs ==
   runNow.UnixNano() — the cold-start / recovery branch already fired
   the walker this pass, and the steady-state fall-through fired it
   again. RecoveryView is a superset of every per-shard partition, so
   the second walk added zero coverage and burned a full subtree scan.
   Add a within-pass guard at the front of walkerDue: if the cursor's
   LastWalkedNs equals runNow's UnixNano, the walker already ran this
   pass — skip.

2. The empty-replay branch passed persisted.LastWalkedNs to walkerDue
   instead of the local lastWalkedNs variable the rest of runShard
   threads through. Trivially equal at this point in the function, but
   the inconsistency would mask a future bug if any code above the
   branch ever sets lastWalkedNs.

Test updates: TestWalkerDue gains the within-pass guard case plus a
companion "earlier same pass still fires" sanity check.
TestRunShard_ColdStartDoesNotDoubleWalk is new and pins the integration:
cold-start runShard with WalkerInterval=0 must call cfg.Walker exactly
once, not twice.

* fix(s3/lifecycle): reject negative WalkerInterval + lift within-pass guard

Two coderabbit findings:

1. validate() now rejects negative cfg.WalkerInterval. A typo like
   -1h previously fell through walkerDue's `interval <= 0` branch and
   silently re-enabled "walk every pass" — the exact behavior the
   throttle was added to prevent. The admin-config parser already
   clamps negative input to zero, but callers using dailyrun.Config
   directly (tests, embedders) now get a loud error instead.

2. Within-pass double-fire suppression moves out of walkerDue and
   into runShard's walkedThisPass local flag. walkerDue's equality
   check (lastWalkedNs == runNow.UnixNano) was correct in production
   (each pass freezes runNow at time.Now().UTC, no collisions) but
   fragile in tests that inject the same runNow across distinct
   passes — the test would see false suppression. Separating the
   concerns also makes walkerDue answer one question (persisted-state
   throttle) and runShard another (within-pass call-site dedup).

walker_interval_test.go: TestValidate_RejectsNegativeWalkerInterval
pins the new validation. TestWalkerDue's within-pass cases move out
(the function is pure throttle now); TestRunShard_ColdStartDoesNot
DoubleWalk still pins the integration behavior end-to-end.

* feat(s3/lifecycle): plumb WalkerInterval through worker admin config

#9484 added cfg.WalkerInterval to dailyrun.Config but left the worker
side wired to zero — operators couldn't actually use the throttle
without recompiling. Add the admin-schema knob:

- New constant WalkerIntervalMinutesAdminKey = "walker_interval_minutes"
  follows the MetaLogRetentionDaysAdminKey pattern (Int64, minutes
  unit, 0 = unbounded / fire every pass).
- New Config.WalkerInterval populated in ParseConfig from
  adminValues; negative / zero stay at zero so the prior "fire every
  pass" semantics keep the in-repo integration tests and the s3tests
  sub-minute driver working unchanged.
- handler.go: admin form field with operator-facing label and
  description, default in DefaultValues, value forwarded to
  dailyrun.Run via cfg.WalkerInterval.

Tests cover the default-zero, positive, and negative cases — same
shape as the MetaLogRetention tests so the parsing contract stays
consistent.

Stacked on #9484; rebase after that lands.
 2026-05-13 14:09:31 -07:00
Chris Lu c6582228b8 feat(s3/lifecycle): throttle steady-state walker by cfg.WalkerInterval (#9484) 
* feat(s3/lifecycle): throttle steady-state walker by cfg.WalkerInterval

The steady-state and empty-replay walker fired on every dailyrun.Run
invocation, which is fine when Run is called at the bucket-walk cadence
the operator intends (e.g., once per hour or once per day), but
catastrophic when a fast driver like the s3tests CI workflow or the
admin worker scheduler invokes Run at multi-second cadence — each tick
ran a full subtree scan per shard, crushing the filer.

Decouple walker cadence from Run() invocation cadence: persist
LastWalkedNs in the per-shard cursor and fire the steady-state /
empty-replay walker only when (runNow - LastWalkedNs) >= cfg.WalkerInterval.
Cold-start and recovery walker fires (RecoveryView) stay unconditional
since those are bounded events that must run when their trigger
condition (no cursor, hash mismatch) is met. Recovery walker fires also
update LastWalkedNs so the subsequent steady-state pass doesn't
double-walk.

cfg.WalkerInterval=0 keeps the prior "fire every pass" behavior — the
in-repo integration tests and s3tests fast driver continue to work
unchanged. Production deployments should set this to the walk cost
budget (typically 1h-24h depending on cluster size).

Cursor file is back-compat: last_walked_ns is omitempty, so cursor
files written before this change decode as LastWalkedNs=0, which
walkerDue treats as "never walked steady-state" → walker fires next
pass to establish the anchor (same path a cold-start cursor takes).
No version bump.

Operator surface for WalkerInterval is the dailyrun.Config struct;
plumbing through worker.tasks.s3_lifecycle.Config and the admin
schema is a follow-up.

* fix(s3/lifecycle): suppress walker double-fire within a single pass

Two gemini-code-assist findings:

1. walkerDue with interval=0 returned true even when lastWalkedNs ==
   runNow.UnixNano() — the cold-start / recovery branch already fired
   the walker this pass, and the steady-state fall-through fired it
   again. RecoveryView is a superset of every per-shard partition, so
   the second walk added zero coverage and burned a full subtree scan.
   Add a within-pass guard at the front of walkerDue: if the cursor's
   LastWalkedNs equals runNow's UnixNano, the walker already ran this
   pass — skip.

2. The empty-replay branch passed persisted.LastWalkedNs to walkerDue
   instead of the local lastWalkedNs variable the rest of runShard
   threads through. Trivially equal at this point in the function, but
   the inconsistency would mask a future bug if any code above the
   branch ever sets lastWalkedNs.

Test updates: TestWalkerDue gains the within-pass guard case plus a
companion "earlier same pass still fires" sanity check.
TestRunShard_ColdStartDoesNotDoubleWalk is new and pins the integration:
cold-start runShard with WalkerInterval=0 must call cfg.Walker exactly
once, not twice.

* fix(s3/lifecycle): reject negative WalkerInterval + lift within-pass guard

Two coderabbit findings:

1. validate() now rejects negative cfg.WalkerInterval. A typo like
   -1h previously fell through walkerDue's `interval <= 0` branch and
   silently re-enabled "walk every pass" — the exact behavior the
   throttle was added to prevent. The admin-config parser already
   clamps negative input to zero, but callers using dailyrun.Config
   directly (tests, embedders) now get a loud error instead.

2. Within-pass double-fire suppression moves out of walkerDue and
   into runShard's walkedThisPass local flag. walkerDue's equality
   check (lastWalkedNs == runNow.UnixNano) was correct in production
   (each pass freezes runNow at time.Now().UTC, no collisions) but
   fragile in tests that inject the same runNow across distinct
   passes — the test would see false suppression. Separating the
   concerns also makes walkerDue answer one question (persisted-state
   throttle) and runShard another (within-pass call-site dedup).

walker_interval_test.go: TestValidate_RejectsNegativeWalkerInterval
pins the new validation. TestWalkerDue's within-pass cases move out
(the function is pure throttle now); TestRunShard_ColdStartDoesNot
DoubleWalk still pins the integration behavior end-to-end.
 2026-05-13 14:09:13 -07:00
Chris Lu d5c0a7b153 fix(ec): make multi-disk same-server EC reads work + full-lifecycle integration test (#9487) 
* fix(master): include GrpcPort in LookupEcVolume response

LookupVolume already passes loc.GrpcPort through to the client; LookupEcVolume
builds Location with only Url / PublicUrl / DataCenter, so callers fall back to
ServerToGrpcAddress (httpPort + 10000). On any deployment where that
convention does not hold — multi-disk integration tests, custom port layouts
— EC reads dial the wrong port and quietly degrade to parity recovery.

* fix(volume/ec): probe every DiskLocation when serving local shard reads

reconcileEcShardsAcrossDisks (issue 9212) registers each .ec?? against the
DiskLocation that physically owns it, so a multi-disk volume server can hold
shards for the same vid in two separate ecVolumes — one per disk — with .ecx
on whichever disk owned the original .dat. The read path only consulted the
single EcVolume FindEcVolume picked, so requests for shards on the sibling
disk fell through to errShardNotLocal and then to remote/loopback recovery.

Walk all DiskLocations after the first probe in both readLocalEcShardInterval
and the VolumeEcShardRead gRPC handler; the latter also covers the loopback
that recoverOneRemoteEcShardInterval falls back to when a peer dial fails.

* test(volume/ec): cover the multi-disk EC lifecycle end-to-end

Two integration tests against a real volume server with two data dirs:

TestEcLifecycleAcrossMultipleDisks drives encode -> mount -> HTTP read ->
drop .dat -> stop -> redistribute shards across disks -> restart -> verify
reconcileEcShardsAcrossDisks attached the orphan shards and reads still
work -> blob delete -> stop -> drop a shard -> restart -> VolumeEcShardsRebuild
pulls input from both disks -> reads still work.

TestEcPartialShardsOnSiblingDiskCleanedUpOnRestart is the issue 9478
reproducer at the cluster level: seed a healthy .dat on disk 0, plant the
on-disk footprint of an interrupted EC encode on disk 1, restart, and assert
pruneIncompleteEcWithSiblingDat wipes disk 1 without touching disk 0.

Framework gets RestartVolumeServer / StopVolumeServer helpers; the previous
run's volume.log is rotated to volume.log.previous so a startup regression on
the second run does not lose the first run's diagnostics.

* review: trim verbose comments

* review: drop racy fast-path, use locked findEcShard directly

gemini-code-assist flagged the two-step lookup in readLocalEcShardInterval
and VolumeEcShardRead: the first probe (ecVolume.FindEcVolumeShard) reads
the EcVolume's Shards slice without holding ecVolumesLock, so a concurrent
mount / unmount could race with it. findEcShard already walks every
DiskLocation under the right lock, so the fast-path adds nothing but the
race. Collapse both call sites to a single locked call.

Also note in RestartVolumeServer why the log-rotation error is swallowed:
absence on first call is benign; anything else surfaces in the next
os.Create in startVolume.
 2026-05-13 13:56:20 -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
Chris Lu e025ec2334 fix(volume): seed indexFileOffset in SortedFileNeedleMap so Delete appends (#9483) 
* fix(volume): seed indexFileOffset in SortedFileNeedleMap so Delete appends

NewSortedFileNeedleMap never initialised the inherited
baseNeedleMapper.indexFileOffset, so it stayed at zero. The compact and
leveldb constructors both seed it from stat().Size(); this one did not.

When a read-only or noWriteCanDelete volume processed a Delete, the
inherited appendToIndexFile wrote the tombstone via WriteAt at
indexFileOffset=0 and advanced 16 bytes at a time. Every delete since
the SortedFileNeedleMap path was first exercised for writes (#5633,
which switched .sdx to O_RDWR) overwrote the front of .idx with
tombstones for unrelated keys.

Net effect on disk: the first N entries of .idx become sequentially
written tombstones with .dat offsets at the tail, while the original
Put records that lived in slots [0..N) are gone. fsck sees ~N phantom
orphans whose keys' runtime needle-map entries are already tombstoned
in .sdx, so the volume server replies 304 to every purge and the
orphan count is stable across retries.

Stat the .idx at open and seed indexFileOffset = size. The .idx now
grows on delete instead of being clobbered. Add a regression test that
populates .idx via the compact map, opens it as a SortedFileNeedleMap,
deletes one needle, and verifies the file grew by one entry, the
original Put records survived, and the tombstone landed at the tail.

The fix only stops further corruption. .idx files already damaged by
this bug have to be rebuilt from .dat before the next restart, or
isSortedFileFresh will regenerate .sdx from the bad .idx and propagate
the damage.

Refs #9479

* test: harden SortedFileNeedleMap regression assertions

Address PR review (gemini-code-assist, coderabbit):

- Close the writer before t.Fatalf in the Put loop so a failing seed
  doesn't leak the .idx file descriptor.
- Verify offset and size of preserved Put records, not just the key.
  Front-overwrite damage would clobber all three fields, but a key-only
  check would miss a different regression that corrupted offset/size
  while leaving the key intact.
 2026-05-13 10:22:01 -07:00
Chris Lu f5a4bfb514 fix(s3/versioning): repair dangling latest-version pointer after partial delete (#9460) 
* fix(s3/versioning): repair dangling latest-version pointer after partial delete

deleteSpecificObjectVersion did two non-atomic filer ops: rm the version
blob, then update the .versions/ pointer. Step 2 failures were silently
logged and the client got 204 OK, so any transient blip (filer timeout,
process restart between RPCs, lock contention) left the .versions/
directory naming a missing file. Subsequent GETs paid the 10-retry
self-heal cost and returned NoSuchKey — surfacing as "Storage not found"
to Veeam, which is what triggered this investigation.

Three changes:

1. Pre-roll the pointer for the singleton / multi-version-deleting-latest
   cases. The pointer is repointed (multi) or cleared (singleton) before
   the blob rm. A failure between leaves a recoverable orphan blob —
   pointer is consistent, GETs succeed or correctly miss without
   entering the stale-pointer self-heal path.

2. Wrap the load-bearing filer ops in updateLatestVersionAfterDeletion
   with bounded retries (~6.3s worst case). When retries are exhausted
   the function now returns a non-nil error instead of swallowing it.
   The caller logs at Error level and queues the path for the
   reconciler.

3. Background reconciler drains stranded .versions/ pointer-to-missing
   states off the hot path. Bounded in-memory queue with capped retries;
   read-path heal remains as a last-resort safety net.

* fix(s3/versioning): address review on #9460

Four fixes addressing review on PR #9460. All four are correctness;
no behavioural change for the happy path.

1. repointLatestBeforeDeletion: discriminate NotFound from transient
   errors when re-fetching the .versions/ entry. Previously any error
   returned rolled=true,nil — a transient filer hiccup at that point
   would cause the caller to skip the post-delete reconciliation AND
   proceed with the blob rm, producing exactly the dangling pointer
   state the PR aims to prevent. NotFound stays "vacuously consistent"
   (directory already gone); other errors surface so the caller aborts
   before removing the blob.

2. Move the singleton .versions/ teardown out of
   repointLatestBeforeDeletion (where it ran BEFORE the blob rm and
   always failed with "non-empty folder") into deleteSpecificObjectVersion
   AFTER the blob rm. Adds a wasSingleton return value so the caller
   knows when to run the teardown. Without this, every singleton-version
   delete in a versioned bucket leaked an empty .versions/ directory.

3. Wrap the list, getEntry, and mkFile calls inside
   repointLatestBeforeDeletion with retryFilerOp so the pre-roll has
   the same transient-failure resilience as the post-roll path. Without
   retries, a single transient blip causes the caller to fall back to
   the legacy non-atomic flow even when the filer recovers immediately.

4. healVersionsPointer in the reconciler: same NotFound-vs-transient
   discrimination on both the .versions/ getEntry and the latest-file
   presence probe. Previously a transient filer error would silently
   evict the candidate from the queue as "healed", leaving the real
   stranded state until a client read happened to surface it.

Also fixes the gemini-flagged consistency nit: the queued-for-reconciler
error log now uses normalizedObject instead of object so it matches the
queue entry's key.

* fix(s3/versioning): short-circuit terminal errors in retryFilerOp

Add isRetryableFilerErr that returns false for filer_pb.ErrNotFound,
gRPC NotFound, context.Canceled, and context.DeadlineExceeded.
retryFilerOp now bails immediately on a terminal error and returns it
unwrapped, so callers like repointLatestBeforeDeletion.getEntry and
updateLatestVersionAfterDeletion.rm see the raw NotFound instead of
paying the ~6.3 s retry-budget delay AND parsing it out of an
"exhausted N retries" wrapper.

errors.Is and status.Code already walk the %w chain so today's call
sites still work, but the delay was real on the hot DELETE path
whenever a key was genuinely absent. Test added covering all five
terminal-error shapes — each must run the wrapped fn exactly once and
return in under 50 ms.
 2026-05-13 10:14:27 -07:00
Chris Lu de28c4df61 fix(storage): prune partial EC shards when sibling disk has healthy .dat (#9478) (#9480) 
* fix(storage): prune partial EC shards when sibling disk has healthy .dat (#9478)

handleFoundEcxFile only checks for .dat in the same disk location as the
EC shards. In a multi-disk volume server an interrupted encode can leave
.ec?? + .ecx on disk B while the source .dat still lives on disk A: the
per-disk loader sees no .dat next to .ecx, mistakes the leftover for a
distributed-EC layout, and mounts the partial shards. The volume server
then heartbeats both a regular replica and an EC shard for the same vid
and the master keeps both.

Sweep the store after per-disk loading and before the cross-disk
reconcile to delete partial EC files when a healthy .dat for the same
(collection, vid) exists on a sibling disk. Push DeletedEcShardsChan for
every pruned shard so master forgets the new-shard message the per-disk
pass already emitted, instead of waiting for the next periodic heartbeat.

* fix(seaweed-volume): mirror prune of partial EC with sibling .dat (#9478)

Rust port of the same Store-level prune added to weed/storage. The
per-disk EC loader in disk_location.rs only checks for .dat in the same
disk as the EC shards, so an interrupted encode that leaves .ec?? + .ecx
on disk B while the source .dat sits on disk A is mounted as if it were
a distributed-EC layout. The volume server then heartbeats both a
regular replica and an EC shard for the same vid.

Sweep the store after per-disk loading and before the cross-disk
reconcile, dropping in-memory EcVolumes with fewer than DATA_SHARDS_COUNT
shards when a .dat for the same (collection, vid) exists on a sibling
disk, and remove all on-disk EC artefacts for them. The Rust heartbeat
path already diff-emits deletes from the next ec_volumes snapshot, so no
explicit delete-channel push is needed here.

Tests cover both the issue 9478 layout and a distributed-EC layout with
no .dat anywhere on the store, which must be left alone.

* fix(storage): validate sibling .dat size before deleting partial EC (#9478)

The earlier prune deleted partial EC files whenever any .dat for the
same vid existed on a sibling disk — including a zero-byte shell. A
shell is no more useful than the partial shard it would replace, and
the partial shard might still combine with shards on other servers
in a recoverable distributed-EC layout. Wiping it based on a corrupt
sibling .dat is data loss masquerading as cleanup.

Tighten the check: when the EC's .vif recorded a non-zero source size
in datFileSize, require the sibling .dat to be at least that many
bytes; otherwise fall back to "at least a superblock". The .vif value
is what the encoder wrote at the moment the source was sealed, so a
sibling .dat smaller than that is provably truncated. Carry the size
through indexDatOwners alongside the location.

The Rust port had the same gap and an additional bug behind it:
EcVolume::new wasn't reading datFileSize from .vif, so the safety
check always fell back to the superblock floor. Wire datFileSize
through. The existing shard-size calculation in
LocateEcShardNeedleInterval already uses dat_file_size when non-zero,
so populating it also matches Go's behaviour there.

Tests cover the truncated-sibling case in both ports.
 2026-05-13 09:25:10 -07:00
Chris Lu 3f1eaf9724 fix(s3/audit): emit audit log for successful GET/HEAD (#9467) 
* fix(s3/audit): emit audit log for successful GET/HEAD

Successful GET/HEAD object requests never produced a fluent audit entry
because those handlers write the response directly (streaming for GET,
WriteHeader for HEAD) and never reach a PostLog call site. The wiki
advertises GET as an audited verb, so the asymmetry surprises operators
who rely on the log for read-access auditing.

Move the safety net into the track() middleware: tag each request with
an audit-tracking flag, let PostLog/PostAccessLog (delete path) mark it,
and emit a single fallback entry after the handler returns when nothing
fired. The recorder's status flows into the fallback so the audit row
still reflects 200/206 vs 404 etc. No double logging for handlers that
already emit (write helpers, error paths, bulk delete).

Refs #9463

* fix(s3/audit): defensive nil checks on audit-tracking helpers

Address PR review: guard against nil request and nil *atomic.Bool stored
under the audit-tracking key. The conditions are unreachable today (the
key is private and we only ever store new(atomic.Bool)), but the checks
are free and keep the helpers safe if a future caller misbehaves.

* test(s3/audit): track() audit fallback coverage + stale comment cleanup (#9469)

test(s3/audit): cover track() fallback wiring + cleanup

Adds two unit tests in weed/s3api/stats_test.go that exercise the
audit-tracking flag set up by track(): one verifies the fallback path
fires when a handler writes the response directly (the GET/HEAD object
regression in #9463), the other verifies the flag is set when a handler
emits PostLog itself so the fallback is skipped.

To make the wiring observable without standing up fluent, PostLog now
marks the audit flag before short-circuiting on a nil Logger; production
behavior is unchanged (no logger, no posting) but the flag stays
consistent.

Also drops two stale comments in s3api_object_handlers.go that still
referenced proxyToFiler — that helper was removed when GET/HEAD started
streaming from volume servers directly.

Stacks on #9467.
 2026-05-13 09:24:59 -07:00
Chris Lu d5372f9eb7 feat(s3/lifecycle): apply cluster rate limit to walker dispatch (#9471) 
Phase 4b shipped the walker without plugging it into the cluster
rate.Limiter that processMatches honors. A walker hitting a large
bucket on the recovery branch could burst LifecycleDelete RPCs past
the cluster_deletes_per_second cap that streaming-replay respects.

WalkerDispatcher now takes a *rate.Limiter and waits on it before
each RPC, observing the wait time on S3LifecycleDispatchLimiterWaitSeconds
just like processMatches does. The handler passes the same limiter
to both paths so replay + walk share one budget; nil disables
throttling (unchanged default).

Tests pin: the limiter actually delays a dispatch when the burst
token is drained, and a ctx cancellation in Limiter.Wait surfaces
as an error without sending the RPC.
 2026-05-13 09:24:50 -07:00
Chris Lu 37e505b8fd refactor(s3/lifecycle): one meta-log subscription per dailyrun.Run pass (#9481) 
* refactor(s3/lifecycle): one meta-log subscription per dailyrun.Run pass

Per-shard Reader subscriptions multiplied filer load by len(cfg.Shards)
even though the same gRPC stream could serve every shard in a worker
process. Replace with one SubscribeMetadata stream covering all shards
in cfg.Shards: the Reader's ShardPredicate accepts the shard set, and
a fan-out goroutine routes events to per-shard channels by ev.ShardID.

drainShardEvents now reads from a passed-in channel; shards whose
persisted cursor is fresher than the global floor (runNow - maxTTL)
filter ev.TsNs <= startTsNs locally. The fan-out cancels the reader
when the first ev.TsNs > runNow arrives — meta-log order means the
rest of the stream is past the pass boundary too.

cfg.Workers no longer gates shard concurrency: with the shared
subscription, every shard goroutine must be live to drain its channel,
or the fan-out stalls. The field is retained for back-compat and
ignored. Dispatch throttling still goes through cfg.Limiter.

Filer load: 16x -> 1x SubscribeMetadata streams per pass.

* fix(s3/lifecycle): shared subscription floor is min(per-shard cursor)

The shared subscription used runNow - maxTTL as its starting TsNs, but
that's the cold-start floor. For shards whose persisted cursor sits
below the floor — exactly the case a rule with TTL == maxTTL produces,
where a pending event's PUT TsNs ends up at runNow - maxTTL — events
that the per-shard drain still needs are filtered out before the
Reader even forwards them.

Same regression I fixed in 6796ab6db for the per-shard subscription;
now applied at the shared level. computeGlobalStartTsNs loads every
shard's cursor and picks the minimum, falling back to the cold-start
floor only for shards with no persisted cursor.
 2026-05-13 02:13:11 -07:00
Chris LuandCopilot b1d59b04a8 fix(s3/lifecycle): walker dispatch uses entry.Path for ABORT_MPU (#9477) 
* fix(s3/lifecycle): WalkerDispatcher uses entry.Path for ABORT_MPU + shell announces load

Two CI-surfaced bugs caught by PR #9471's S3 Lifecycle Tests run on
master after PRs #9475 + #9466:

1. Walker dispatch for ABORT_MPU was sending entry.DestKey as
   req.ObjectPath. The server's ABORT_MPU handler
   (weed/s3api/s3api_internal_lifecycle.go) strips the .uploads/
   prefix to extract the upload id and reads the init record from
   that directory, so it expects the .uploads/<id> path verbatim.
   DestKey looks like a regular object path; the server's prefix
   check fails and the dispatch returns BLOCKED with
   "FATAL_EVENT_ERROR: ABORT_MPU object_path missing .uploads/
   prefix". The test fix renames TestWalkerDispatcher_MPUInitUsesDestKey
   to ...UsesUploadsPath and inverts the assertion to match the
   actual server contract.

   DestKey is still used for the WalkBuckets shard predicate and
   for rule-prefix matching in bootstrap.walker; both surfaces want
   the user's intended path, while DISPATCH wants the .uploads/<id>
   directory. The bootstrap test
   (TestLifecycleAbortIncompleteMultipartUpload) caught this when
   the walker's BLOCKED error surfaced as FATAL output.

2. test/s3/lifecycle/s3_lifecycle_empty_bucket_test.go asserts the
   shell command logs "loaded lifecycle for N bucket(s)" so a
   regression that produces half-shaped output (no load summary)
   is caught. The restored shell command (PR #9475) didn't print
   that line; add it back on the first pass that finds non-zero
   inputs.

* fix(s3/lifecycle): walker fires for walker-only buckets (empty replay path)

runShard's empty-replay sentinel (rsh == [32]byte{}) was returning
BEFORE the steady-state walker check. A bucket whose only lifecycle
rule was walker-only (ExpirationDate / ExpiredDeleteMarker /
NewerNoncurrent) would never have it dispatched because:

  - ReplayContentHash only hashes replay-eligible kinds, so
    walker-only-only snapshots produce rsh == empty.
  - The early-return persisted the empty cursor and exited before
    the steady-state walker block at the bottom of the function.

Move the walker invocation INTO the empty-replay branch so walker-
only rules dispatch on the same path as mixed-rule buckets.

TestLifecycleExpirationDateInThePast and
TestLifecycleExpiredDeleteMarkerCleanup were both timing out their
"object must be deleted" Eventually polls because of this. Caught
on PR #9471's S3 Lifecycle Tests run after PR #9475 restored the
shell entry point that exercises the integration tests.

* fix(s3/lifecycle): cold-start walker covers pre-existing objects

runShard only walked the bucket tree on the recovery branch (found
&& hash mismatch). For a fresh worker with no persisted cursor,
found=false, so the recovery walker never fired and the meta-log
replay only scanned runNow - maxTTL of events. Objects PUT before
that window — including pre-existing objects in a newly-rule-enabled
bucket — never matched the rule.

The streaming worker handled this with scheduler.BucketBootstrapper.
Daily-replay needed the equivalent: walk the live tree once on the
first run for each shard so pre-existing objects get evaluated even
when their PUT events are outside meta-log scan window.

Restructured the recovery branch to fire the walker on either
(found && mismatch) OR !found. On cold-start the cursor isn't
rewound — we keep TsNs=0 and let the drain below floor to
runNow - maxTTL like before; the walker just handles whatever the
sliding window can't reach.

TestLifecycleBootstrapWalkOnExistingObjects was the exact CI failure
this addresses (https://github.com/seaweedfs/seaweedfs/actions/runs/25777823522/job/75714014151).

* fix(s3/lifecycle): restore walker tag and null-version state

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(s3/lifecycle): parallelize shell shard sweeps

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix(s3/lifecycle): bound each runPass ctx + refresh in runLifecycleShard

Two CI bugs surfaced after PR #9466 deleted the streaming worker:

1. The shell command's -refresh loop never fires. runPass used the
   outer ctx (full -runtime), so dailyrun.Run blocked for the entire
   1800s s3tests window — the background worker only ran one pass
   and never re-loaded configs that tests created mid-run.
   test_lifecycle_expiration sees 6 objects when expecting 4 because
   expire1/* never reaches the worker's snapshot. Cap each pass to
   cadence+5s when cadence>0; one-shot (cadence=0) keeps the full ctx.

2. TestLifecycleExpiredDeleteMarkerCleanup's docstring says
   "pass 1 cleans v1; pass 2 removes the now-orphaned marker," but
   runLifecycleShard invoked with no -refresh — only one pass ran.
   The marker rule can't fire in the same pass that dispatches v1's
   delete because v1 is still in .versions/. Add -refresh 1s so the
   10s runtime gets multiple passes.

* fix(s3/lifecycle): persist cursor with fresh ctx after passCtx timeout

drainShardEvents only exits via ctx cancellation for an idle subscription
— that's the steady-state when all replayed events are already past.
Saving the cursor with the canceled passCtx silently drops every
advance, so the next pass re-subscribes from the same floor and
re-replays the same events. Symptom in s3tests: status=error shards=16
errors=16 on every pass, and 1/6 expire3/* dispatches lost to a race
between concurrent shard drains all retrying the same events.

Use a 5s timeout derived from context.Background for the save, and
treat passCtx Deadline/Canceled from drain as a clean end-of-pass —
not a shard-level error to log.

* fix(s3/lifecycle): trust persisted cursor; never bump past pending events

The drain freezes cursorAdvanceTo at the last pre-skip event so pending
matches (DueTime > runNow) re-enter the subscription next pass. Combined
with the new cursor persistence, the floor bump (runNow - maxTTL) then
orphans the very events the drain stopped at.

Concrete: a rule with TTL == maxTTL fires at runNow == PUT_TIME +
maxTTL, so floor (= runNow - maxTTL) lands exactly on PUT_TIME. If the
last advance saved a cursor right before the not-yet-due PUT (e.g.,
keep2/* between expire1/* and expire3/* on the same shard), the floor
bump on pass 9 skips past the expire3 event itself — the worker never
re-reads it. Test symptom: expire3/* never expires when worker shards
include other earlier no-match events.

Cold start (found=false) still subscribes from runNow - maxTTL. Steady
state honors the cursor verbatim.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
 2026-05-13 00:19:05 -07:00
Chris Lu 7b8647e8bc fix(shell): loop s3.lifecycle.run-shard so CI workflow stays alive (#9476) 
The s3tests workflow (.github/workflows/s3tests.yml) backgrounds
`weed shell -c 's3.lifecycle.run-shard -shards 0-15 -s3 ... -refresh 2s'`
and then runs `kill -0 $pid` to confirm the worker stayed alive.
The PR-9475 restore ran dailyrun.Run once and exited cleanly — even
faster when no buckets had lifecycle rules yet ("nothing to run").
The aliveness check then failed and the s3tests job died with
"lifecycle worker died on startup". Caught on
https://github.com/seaweedfs/seaweedfs/actions/runs/25772523143/job/75698413401.

Fix:

  - -refresh now drives an inter-pass loop. cadence=0 (default) is
    one-shot, matching the test/s3/lifecycle/ integration-test
    invocation that omits -refresh and expects synchronous return.
    cadence>0 (the CI case) keeps the command alive until -runtime
    expires, running a fresh dailyrun.Run on every tick.
  - Each iteration re-loads bucket configs via
    scheduler.LoadCompileInputs so rules created mid-run (the s3tests
    flow creates rules AFTER the worker starts) get picked up.
  - The "no rules; nothing to run" early return is gone — the
    command stays alive even with an empty initial snapshot, waiting
    for tests to add rules.
  - -dispatch, -checkpoint, -bootstrap-interval stay accepted-but-
    ignored (legacy streaming flags).
 2026-05-12 19:19:46 -07:00
Chris Lu 4ce027c2f3 fix(shell): restore s3.lifecycle.run-shard for CI/integration-test compatibility (#9475) 
fix(shell): restore s3.lifecycle.run-shard as a dailyrun.Run wrapper

PR #9466 deleted weed/shell/command_s3_lifecycle_run_shard.go on the
premise that it was a debug-only tool. It wasn't: the s3tests CI
workflow (.github/workflows/s3tests.yml) and the test/s3/lifecycle/
integration tests invoke it via `weed shell` to drive lifecycle
expirations on demand. Both started failing with
"unknown command: s3.lifecycle.run-shard".

This PR restores the command with the same flag set so existing
callers (CI scripts and integration tests) work unchanged. The
implementation no longer drives the streaming dispatcher.Pipeline +
scheduler.BucketBootstrapper (deleted) — instead it does one bounded
dailyrun.Run pass through the same daily-replay code path the
production worker exercises. The walker fires for walker-bound
rules just like in the worker.

Obsolete streaming flags (-dispatch / -checkpoint / -refresh /
-bootstrap-interval) are accepted-but-ignored so existing scripts
don't need to drop them.
 2026-05-12 18:29:18 -07:00
Chris Lu ce5768fab1 feat(s3/lifecycle): operator-declared meta-log retention activates PromotedHash (#9473) 
* feat(s3/lifecycle): operator-declared meta-log retention activates PromotedHash

dailyrun.Config.RetentionWindow has been wired since Phase 4b but the
handler never supplied a value, so runShard always fell back to
maxTTL and engine.PromotedHash hashed nothing. The partition-flip
recovery trigger was dormant by design "until the handler plumbs the
real meta-log retention here."

This PR plumbs it via a new admin form field:
  Meta-Log Retention (days) — 0 = unbounded (current behavior).

When set, ParseConfig converts days to a time.Duration on
cfg.MetaLogRetention. The handler passes it as
dailyrun.Config.RetentionWindow, which runShard then feeds to
engine.PromotedHash. Rules whose TTL exceeds the declared window
land in the walk partition; the next time an operator shrinks
retention so a previously replay-eligible rule slips past it,
PromotedHash mismatches → recovery branch fires → walker re-evaluates
the rule across the whole filer tree.

0 stays the default, so existing deployments see no behavior change.

* chore(s3/lifecycle): rephrase days->duration conversion

gemini-code-assist flagged the original form as a compile error,
which it wasn't (time.Duration is a named int64 and supports * with
other time.Durations — the test suite verified the value was
correct). The suggested form is more idiomatic regardless:
days*24 happens in int64 space before the lift to time.Duration,
so the unit is unambiguous.
 2026-05-12 18:26:52 -07:00
Chris Lu f51468cf73 Revert #9443 — heartbeat peer binding breaks hostname-based clusters (#9474) 
Revert "master: bind heartbeat claims to the connecting peer (#9443)"

This reverts commit f28c7ce6df.

The strict heartbeat-ip-vs-peer match in authorizeHeartbeatPeer rejects
every hostname-based deployment. In docker-compose / k8s the volume
server is started with -ip=<service-name> and the gRPC peer surfaces
as the container/pod IP, so the two never match and every heartbeat
fails with `heartbeat ip "volume" does not match peer "172.18.0.3"`.
The master therefore never learns about any volume, growth fails, and
fio writes against the mount return EIO.

After the #9440 revert merged (43a8c4fdc), the e2e workflow is still
failing for this reason; see
https://github.com/seaweedfs/seaweedfs/actions/runs/25767265775 .

Reverting to unblock e2e. A narrower re-do should accept the heartbeat
when heartbeat.Ip resolves (DNS) to the peer address, so the spoof
hardening can return without breaking hostname-based clusters.
 2026-05-12 18:22:21 -07:00
Chris Lu 43a8c4fdca Revert #9440 — volume admin fail-closed gate breaks multi-host clusters (#9472) 
* Revert "volume: fail closed in admin gRPC gate when no whitelist is configured (#9440)"

This reverts commit 21054b6c18.

The fail-closed gate broke any multi-host cluster: in compose / k8s /
remote-host deployments the master's IP isn't loopback, so every
master->volume admin RPC (AllocateVolume, BatchDelete, EC reroute,
vacuum, scrub, ...) is rejected with PermissionDenied unless the
operator manually configures -whiteList. The e2e workflow has been
failing since 10cc06333 with `not authorized: 172.18.0.2` on
AllocateVolume; downstream symptom is fio fsync EIO because zero
volumes can be grown.

The gate's intent was to lock down destructive admin tooling, but the
same RPCs are the master's normal mechanism for growing and managing
volumes. Reverting to restore cluster-internal operation; a narrower
re-do should distinguish operator/admin callers from the master peer
(e.g. trust IPs resolved from -master) before going back in.

* security: skip invalid CIDR in UpdateWhiteList so IsWhiteListed can't panic

The revert in the previous commit also rolled back an unrelated bug fix
that lived inside #9440: UpdateWhiteList logged on net.ParseCIDR error
but did not continue, so the nil *net.IPNet was stored in whiteListCIDR
and IsWhiteListed would panic dereferencing cidrnet.Contains(remote) on
the next gRPC admin check.

Restore the continue. Orthogonal to the fail-closed semantics this PR
is reverting.
 2026-05-12 16:00:44 -07:00
Chris Lu f28c7ce6df master: bind heartbeat claims to the connecting peer (#9443) 
SendHeartbeat used to accept whatever Ip/Port/Volumes the caller put on
the wire. Three changes tighten that:

- Reject heartbeats whose Ip does not match the gRPC peer's source
  address. Loopback peers are still trusted; operators behind a proxy
  can opt out with -master.allowUntrustedHeartbeat.
- Track which (ip, port) first claimed a volume id or an ec shard slot
  and drop foreign re-claims. Non-EC volume claims are bounded by the
  replica copy count so legitimate replicas still register. EC
  ownership is keyed by (vid, shard_id) so the same vid can legitimately
  be split across many peers as long as their EcIndexBits are disjoint;
  rejected bits are cleared from the bitmap and the parallel ShardSizes
  array is compacted in lock-step.
- Maintain reverse indexes owner -> volumes and owner -> ec shard slots
  so disconnect cleanup is O(M) in what that peer held rather than O(N)
  over the whole map.

Bindings are also released when a heartbeat reports that the peer no
longer holds an id, either via explicit Deleted{Volumes,EcShards}
entries or by omitting it from a full snapshot. Without this, a planned
rebalance that moved a vid or an ec shard from peer A to peer B would
leave B's heartbeats permanently filtered out until A disconnected,
breaking ec encode/decode flows that delete shards on the source as
soon as the move completes.

The (vid -> owners) binding still does not track which replica slot
each peer occupies, so the first N claims under the copy count win;
strict per-slot mapping is a follow-up.
 2026-05-12 15:38:52 -07:00
Chris Lu 10cc06333b cluster: restrict Ping RPC to known peers of the requested type (#9445) 
Ping previously dialled whatever host:port the caller asked for. Gate
each server's Ping handler on cluster membership: masters check the
topology, registered cluster nodes, and configured master peers; volume
servers only accept their seed/current masters; filers accept tracked
peer filers, the master-learned volume server set, and configured
masters.

Use address-indexed peer lookups to keep Ping target validation O(1):
- topology maintains a pb.ServerAddress -> *DataNode index alongside
  the dc/rack/node tree, kept in sync from doLinkChildNode and
  UnlinkChildNode plus the ip/port-rewrite branch in
  GetOrCreateDataNode. GetTopology now returns nil on a detached
  subtree instead of panicking, so the linkage hooks can no-op safely.
- vid_map tracks a refcount per volume-server address so
  hasVolumeServer answers without scanning every vid location. The
  add path skips empty-address entries the same way the delete path
  already does, so a zero-value Location cannot leak a permanent
  serverRefCount[""] bucket.
- masters reuse a cached master-address set from MasterClient instead
  of walking the configured peer slice on every request.
- volume servers compare against a pre-built seed-master set and
  protect currentMaster reads/writes with an RWMutex, fixing the
  data race with the heartbeat goroutine. The seed slice is copied
  on construction so external mutation cannot desync it from the
  frozen lookup set.
- cluster.check drops the direct volume-to-volume sweep; volume
  servers no longer carry a peer-volume list, and the note next to
  the dropped probe is reworded to make clear that direct
  volume-to-volume reachability is intentionally not validated by
  this command.

Update the volume-server integration tests that drove Ping through the
new admission gate: success-path coverage now targets the master peer
(the only type a volume server tracks), and the unknown/unreachable
path asserts the InvalidArgument the gate now returns instead of the
old downstream dial error.

Mirror the same admission gate in the Rust volume server crate: a
seed-master HashSet built once at startup plus a tokio RwLock over the
heartbeat-tracked current master, both consulted in is_known_ping_target
on every Ping, with InvalidArgument returned for any target that isn't
a recognised master.
 2026-05-12 13:00:52 -07:00
Chris Lu 5004b4e542 feat(s3/lifecycle): delete streaming algorithm path (Phase 5b) (#9466) 
* feat(s3/lifecycle): delete streaming algorithm path (Phase 5b)

Phase 5a (PR #9465) retired the algorithm flag and made daily_replay
the only execution path. The streaming-side code (scheduler.Scheduler,
scheduler.BucketBootstrapper, dispatcher.Pipeline, dispatcher.Dispatcher,
dispatcher.FilerPersister, and their tests) has had no in-tree caller
since then. This PR deletes it.

Net change: ~4800 lines removed, ~130 added (the scheduler/configload
tests' helper file the deleted bootstrap_test.go used to host).

Removed:
  - weed/s3api/s3lifecycle/scheduler/{bootstrap,bootstrap_test,
    scheduler,scheduler_test,pipeline_fanout_test,
    refresh_default,refresh_s3tests}.go
  - weed/s3api/s3lifecycle/dispatcher/{dispatcher,dispatcher_test,
    dispatcher_helpers_test,edge_cases_test,multi_shard_test,
    pipeline,pipeline_test,pipeline_helpers_test,toproto_test,
    dispatch_ticks_default,dispatch_ticks_s3tests}.go
  - weed/s3api/s3lifecycle/dispatcher/filer_persister_test.go
    (FilerPersister deleted; FilerStore tests don't need their own
    file)
  - weed/shell/command_s3_lifecycle_run_shard{,_test}.go
    (debug-only shell command that only ever wrapped the streaming
    pipeline; the production worker now exercises the same path
    every daily run)

Trimmed:
  - dispatcher/filer_persister.go down to FilerStore +
    NewFilerStoreClient — the small interface daily_replay's cursor
    persister (dailyrun.FilerCursorPersister) plugs into.

Kept (still consumed by daily_replay):
  - scheduler/configload.{go,_test.go} (LoadCompileInputs,
    AllActivePriorStates)
  - dispatcher/sibling_lister.{go,_test.go} (NewFilerSiblingLister,
    FilerSiblingLister)
  - dispatcher/filer_persister.go (FilerStore, NewFilerStoreClient)

scheduler/testhelpers_test.go restores fakeFilerClient, fakeListStream,
dirEntry, fileEntry — helpers the configload tests used to share with
the deleted bootstrap_test.go.

Updates the handler-package doc strings and one reader-package
comment that still named the streaming pipeline.

* fix(s3/lifecycle): hold lock through tree read in test filer client

gemini caught an inconsistency in scheduler/testhelpers_test.go:
LookupDirectoryEntry reads c.tree under c.mu, but ListEntries was
releasing the lock before reading c.tree. The map is effectively
static during tests so there's no actual race today, but matching
the convention keeps the helper safe if a future test mutates the
tree mid-run.
 2026-05-12 12:54:52 -07:00
Chris Lu 745e864bda feat(s3/lifecycle): retire algorithm flag, daily_replay is the only path (Phase 5a) (#9465) 
feat(s3/lifecycle): remove algorithm flag, daily_replay is the only path (Phase 5a)

With Phase 4b on master the daily_replay path covers every rule kind
and the streaming algorithm has no remaining responsibilities. This
PR retires the algorithm flag from the worker:

  - Drop the "Algorithm" enum field from AdminConfigForm and its
    DefaultValues entry.
  - Drop the if/else routing in Execute — every Execute call now
    routes straight into executeDailyReplay.
  - Drop the streaming-only worker fields (DispatchTick,
    CheckpointTick, RefreshInterval, BootstrapInterval) and their
    matching form fields. None of them are read by the daily_replay
    path; keeping them in the form would suggest tuning knobs that
    don't do anything.
  - Drop AlgorithmStreaming / AlgorithmDailyReplay constants and the
    Config.Algorithm field.

The streaming-path packages (s3lifecycle/scheduler, s3lifecycle/dispatcher)
remain on the tree; they're now reachable only by the
weed shell s3.lifecycle.run-shard debug command and the few helpers
(LoadCompileInputs, FilerStore, FilerSiblingLister) the daily_replay
worker still uses. Phase 5b deletes the dead code.

Tests prune the cadence-default assertions to the single remaining
field (max_runtime_minutes).
 2026-05-12 12:39:37 -07:00
Chris Lu 2f682303fb fix(s3/lifecycle): align walker dispatch error label to RPC_ERROR (#9464) 
Follow-up to PR #9459 (merged before this fix landed). The walker
dispatcher's RPC failure paths were labeled "TRANSPORT_ERROR" and
"NIL_RESPONSE"; streaming (dispatcher/dispatcher.go) and the replay
drain (processMatches in run.go via #9462) use "RPC_ERROR" for the
same condition. Aligning so a single Prometheus query covers all
three delete paths.

Folds nil-response under RPC_ERROR rather than a separate label —
operationally it's the same class of failure (server returned no
usable response).
 2026-05-12 12:38:52 -07:00
Chris Lu 21054b6c18 volume: fail closed in admin gRPC gate when no whitelist is configured (#9440) 
Add Guard.IsAdminAuthorized, a fail-closed variant of IsWhiteListed, and use
it to gate destructive volume admin RPCs. IsWhiteListed keeps its
allow-all-when-empty semantics for HTTP compatibility.

For TCP peers with an empty whitelist, off-host callers are rejected but
loopback (127.0.0.0/8, ::1) is still trusted. A volume server commonly
cohabits with the master/filer on a single host and in integration-test
clusters; the loopback exception keeps cluster-internal admin traffic
working without -whiteList while still locking out off-host attackers.

Non-TCP peers (in-process / bufconn / unix-socket) bypass the host check
entirely. When `weed server` runs master+volume+filer in a single process
the master dials the volume server in-process and the peer address surfaces
as "@", which has no parseable IP. Such a caller shares our OS process and
cannot be spoofed by a remote attacker, so we treat it as trusted by
construction.

The gate also tolerates a nil guard (developmental / embedded path) and only
enforces once a guard is wired up. UpdateWhiteList skips entries whose CIDR
fails to parse so the IP-iteration path can no longer hit a nil *net.IPNet.
 2026-05-12 12:35:27 -07:00
Chris Lu 495632730c feat(s3/lifecycle): daily-replay observability — metrics + summary log (Phase 6) (#9462) 
* feat(s3/lifecycle): daily-replay observability metrics + per-run summary log

Operators have no Prometheus signal today for the daily_replay path
beyond the cluster-rate-limiter wait histogram. Phase 6 adds the
three baseline questions: how long does a shard take, how many events
did it scan, and what did dispatch produce.

  - S3LifecycleDailyRunShardDurationSeconds (histogram, label=shard):
    wall-clock per shard. p95 climbing toward MaxRuntime means the
    shard is brushing its budget.
  - S3LifecycleDailyRunEventsScanned (counter, label=shard): meta-log
    events drainShardEvents processed. Pairs with the duration so a
    spike in events-per-shard correlates with a slow shard.
  - S3LifecycleDispatchCounter (existing, reused): processMatches now
    increments this with the outcome label, so streaming and
    daily_replay paths share one outcome view. Transport errors are
    counted under outcome="TRANSPORT_ERROR".

dailyrun.Run logs a per-run summary at V(0): status / shards /
errors / duration. The summary is the at-a-glance line operators read
in /var/log to confirm a run completed.

Test pins the dispatch-counter increment with a unique
bucket/kind/outcome triple so a refactor that drops the
instrumentation call surfaces as a test failure.

* fix(s3/lifecycle): align dispatch error label + clean test labels

Two PR-9462 review fixes from gemini:

1. processMatches' transport-failure label was "TRANSPORT_ERROR";
   streaming's dispatcher uses "RPC_ERROR" for the same condition
   (see dispatcher/dispatcher.go). Use "RPC_ERROR" here too so
   the same Prometheus query covers both delete paths.

2. The dispatch-counter assertion test now deletes its label row
   on exit so the in-process Prometheus registry doesn't accumulate
   per-test state across the suite.
 2026-05-12 12:15:20 -07:00
Chris Lu f954781169 feat(s3/lifecycle): Phase 4b — daily walker for recovery and steady state (#9459) 
* feat(s3/lifecycle): plumb RetentionWindow into dailyrun.Config

Adds a Config.RetentionWindow field that runShard threads into
engine.PromotedHash. Zero (the default) falls back to maxTTL, which
matches Phase 4a behavior — PromotedHash stays empty and the
partition-flip recovery trigger stays dormant.

Pure plumbing. The handler still passes zero so nothing changes at
runtime. The walker work (Phase 4b proper) sets a real retention from
the meta-log boundary and the partition-flip trigger starts firing.

* feat(s3/lifecycle): WalkerDispatcher adapter for the daily-run walker

Phase 4b prep. Implements bootstrap.Dispatcher on top of LifecycleClient
so the same LifecycleDelete RPC drives both the meta-log replay path
and the walker. No CAS witness — the server's identityMatches treats
nil ExpectedIdentity as a bootstrap call and rebuilds the witness from
the live entry, which is the right contract for a full-tree walk.

Adds VersionID to bootstrap.Entry so versioned-bucket walks address
the right version. MPU init uses DestKey for ObjectPath (matching the
prefix-match contract); rejecting empty DestKey keeps malformed init
records out of the dispatch path.

Not wired yet — runShard still doesn't invoke the walker. Follow-up
commits add the ListFunc adapter and the recovery-branch wiring.

* feat(s3/lifecycle): wire Walker hook into runShard's recovery branch

Adds a Config.Walker callback that fires on rule-content edit /
partition flip BEFORE the cursor rewinds, so already-due objects across
the rewritten rule set get caught instead of waiting on meta-log
replay alone. The callback receives engine.RecoveryView(snap) and the
per-shard ID; nil disables it (Phase 4a behavior preserved).

Decoupling the wiring from the implementation: the handler-side
WalkerFunc that drives bootstrap.Walk via the filer is the follow-up
commit, and tests can stub the callback without standing up the full
filer/client/lister harness.

Tests pin: walker fires exactly once on hash mismatch, walker error
propagates and leaves the cursor unchanged, nil Walker is a no-op.

* feat(s3/lifecycle): WalkBuckets composes ListFunc + Dispatcher per shard

Adds dailyrun.WalkBuckets — the composable driver the handler-side
WalkerFunc will call. Iterates a bucket list, wraps the supplied
bootstrap.ListFunc with a per-shard filter (Path for non-MPU, DestKey
for MPU init), and runs bootstrap.Walk per bucket using the supplied
Dispatcher. First bucket error wins; remaining buckets log and run to
completion so one filer flake doesn't kill the shard.

Composable rather than monolithic so callers and tests can swap parts:
production uses a filer-backed ListFunc + WalkerDispatcher; tests use
bootstrap.EntryCallback + a stub. The filer-backed ListFunc is the
next commit.

Tests pin: shard filter routes only matching entries, MPU shard uses
DestKey not the .uploads/<id> path, single-bucket error propagates
while other buckets still run, ctx cancellation short-circuits between
buckets, nil guards on view/list/dispatch.

* feat(s3/lifecycle): filer-backed ListFunc for the daily-run walker

Phase 4b: dailyrun.FilerListFunc returns a bootstrap.ListFunc that
streams entries under <bucketsPath>/<bucket> by paginated SeaweedList.
Recurses into regular directories; .versions/ and .uploads/ are
skipped at this stage so they don't surface as raw children — the
sibling expansion (versioned NoncurrentDays state, MPU init dispatch)
lands in the next commit.

listAll and isVersionsDir are ported from scheduler/bootstrap.go's
same-named helpers. Phase 5 deletes the scheduler copies along with
the streaming path.

Tests pin: flat listing, recursion through nested directories,
.versions/ and .uploads/ skipped, kill-resume via the start path
contract, nil-client error, attribute propagation (mtime / size /
IsLatest default).

* feat(s3/lifecycle): versioned-sibling expansion in FilerListFunc

Adds the .versions/<key>/ expansion to the daily-run's filer-backed
ListFunc. Each call emits one bootstrap.Entry per sibling (real
version files + the bare null version, when found) with the same
sibling state the streaming bootstrap injects via reader.Event:

  - Path = logical key (not the .versions/<file> physical path), so
    bootstrap.Walk's MatchPath uses the user's intended path.
  - VersionID per sibling (version_id or "null").
  - IsLatest resolved via parent's ExtLatestVersionIdKey, falling back
    to explicit-null-bare, falling back to newest-by-mtime.
  - NoncurrentIndex rank computed against the latest's position.
  - SuccessorModTime: SuccessorFromEntryStamp if stamped, else the
    previous-newer sibling's mtime (legacy derivation).
  - IsDeleteMarker from ExtDeleteMarkerKey.
  - NumVersions = len(siblings).

Two-pass walk so .versions/ dirs run before regular files; the bare
null-version path is recorded in skipBare so pass 2 doesn't emit it
twice.

expandVersionsDir and lookupNullVersion are ported from
scheduler/bootstrap.go. Sort order, latest resolution, and successor
derivation must agree with that path verbatim so streaming and walker
reach the same verdict on the same objects. Phase 5 deletes the
scheduler copy.

MPU init (.uploads/<id>) remains skipped — the dedicated commit emits
it with IsMPUInit and DestKey.

Tests pin: pointer-wins latest resolution, no-pointer newest-sibling
fallback, explicit-null-is-latest with skipBare suppression of the
bare emission, coincidentally-named .versions folder recursing as a
regular subdir, delete-marker propagation.

* feat(s3/lifecycle): emit MPU init records from FilerListFunc

Last gap in the filer-backed ListFunc. A directory at .uploads/<id>
carrying ExtMultipartObjectKey is the MPU init record; emit one
bootstrap.Entry with IsMPUInit=true and DestKey set to the user's
intended path. The walker's MatchPath uses DestKey for prefix
matching; the WalkerDispatcher uses it for the LifecycleDelete RPC's
ObjectPath. .uploads/<id> directories without the extended key are
mid-write before metadata landed and stay skipped.

isMPUInitDir is upgraded from the path-shape-only stub to the full
shape + extended-attr check that mirrors router.mpuInitInfo and
scheduler/bootstrap.go's same-named helper.

Tests pin: valid init record emits with the right DestKey, missing
ExtMultipartObjectKey skips the directory.

* feat(s3/lifecycle): wire walker into executeDailyReplay

Activates the recovery-branch walker. The handler composes the three
Phase 4b building blocks — FilerListFunc + WalkerDispatcher + WalkBuckets
— into a dailyrun.WalkerFunc and passes it via Config.Walker. The
bucket list is derived from the compiled inputs so it matches the
engine snapshot exactly.

Effect on master behavior: when a worker observes a RuleSetHash or
PromotedHash mismatch on its persisted cursor (rule content edited /
partition flip), runShard now walks the live filer tree under the
RecoveryView before rewinding the cursor. Already-due objects across
the rewritten rule set fire immediately instead of waiting on the
sliding meta-log replay.

Still scoped to replay-eligible action kinds because
checkSnapshotForUnsupported continues to reject walker-bound rules
(ExpirationDate / ExpiredDeleteMarker / NewerNoncurrent) and
scan_only-promoted rules at the top of Run. The follow-up commit
relaxes the gate once the steady-state walker over RulesForShard's
walk view is wired so those rules fire every day, not just on rule
edits.

* feat(s3/lifecycle): steady-state walker + drop unsupported-rule gate

Adds the second walker invocation in runShard. After the recovery
check passes, runShard derives the walk view via snap.RulesForShard
(using the same retentionWindow PromotedHash used, so the partition
is consistent) and runs the walker over it. The view holds
walker-bound action kinds (ExpirationDate / ExpiredDeleteMarker /
NewerNoncurrent) plus any replay-eligible rules promoted to walk by
retention shortage; an empty view skips the call so non-versioned,
replay-only deployments don't pay an O(N) bucket walk per run.

With the walker now servicing every rule kind, checkSnapshotForUnsupported
and its UnsupportedRuleError type are obsolete. router.Route gates
replay on Mode == ModeEventDriven, so walker-bound and scan_only
rules are silently dropped by replay and picked up by the walker
instead — no double-dispatch. Drop the gate, delete replayability.go
+ replayability_test.go, and remove the handler's redundant
IsUnsupportedRule branch.

* fix(s3/lifecycle): walker dispatcher nil-response guard + retention-comment

Two PR-review fixes on 9459:

1. WalkerDispatcher.Delete used to panic on a (nil, nil) RPC return —
   add a defensive nil-response check so the walk halts cleanly
   instead. Spotted by coderabbit.

2. The retentionWindow=maxTTL comment in runShard claimed PromotedHash
   "stays empty" in fallback mode, which gemini correctly pointed out
   is only true once rules are active. During bootstrap (rules
   compiled but IsActive=false) MaxEffectiveTTL is 0 while
   PromotedHash counts every non-disabled rule, so promoted becomes
   non-empty and the next post-activation run hits the recovery
   branch. That's the intended bootstrap walk — rewrite the comment
   to explain it rather than misstate the invariant.

Test: pins nil-response → error path on WalkerDispatcher.

* fix(s3/lifecycle): explicit stale-pointer fallback in versioned expansion

Reviewer caught a structural bug in expandVersionsDir's latest
resolution: when ExtLatestVersionIdKey was set but no scanned sibling
carried that id (stale pointer), the code left latestPos at the
default 0 without ever entering the no-pointer fallback. Today the
two paths yield the same value (newest sibling wins), but the
implicit fall-through makes the intent unclear and would break
silently if the no-pointer branch ever did anything more than
latestPos=0.

Track a pointerResolved flag explicitly so the no-pointer branch
(including the explicit-null-bare check) re-runs on a stale pointer.
Behavior unchanged today.

Test pins: stale pointer + two real versions falls back to
newest-sibling (vnew, not vold).

* feat(s3/lifecycle): walker-side dispatch metrics in WalkerDispatcher

Mirrors the Phase 6 instrumentation already on the replay side
(processMatches) onto the walker's Delete dispatch. Every walker
dispatch now bumps S3LifecycleDispatchCounter with the resolved
outcome (or TRANSPORT_ERROR / NIL_RESPONSE for the failure paths) so
streaming, daily_replay's replay drain, and daily_replay's walker
share a single per-(bucket, kind, outcome) counter view.

Lands together with the rest of Phase 4b — no new metric, just an
extra observation site for the existing one.
 2026-05-12 11:39:15 -07:00
Chris Lu 69da20bdae volume: gate FetchAndWriteNeedle behind admin auth and refuse internal endpoints (#9441) 
volume: require admin auth and refuse loopback endpoints in FetchAndWriteNeedle

Gate the RPC behind checkGrpcAdminAuth for parity with the rest of the
destructive volume-server RPCs, and reject cluster-internal remote S3
endpoints (loopback / link-local / IMDS / RFC 1918 / CGNAT) before
dialing. Pin the validated address against DNS rebinding by routing the
AWS SDK through an HTTP transport whose DialContext re-resolves the host
and re-applies the deny list on every dial, so an endpoint that resolves
to a public IP at validate-time and then flips to 127.0.0.1 at connect
time is refused. Operators that legitimately fetch from private hosts
can opt out with -volume.allowUntrustedRemoteEndpoints.
 2026-05-12 10:11:20 -07:00
Chris Lu 5e8f99f40a filer: require admin-signed JWT on the IAM gRPC service (#9442) 
Every IAM RPC (CreateUser, PutPolicy, CreateAccessKey, ...) now requires
a Bearer token in the authorization metadata, signed with the filer
write-signing key. The service refuses to register on a filer that has
no jwt.filer_signing.key set, so the unauthenticated default is gone:
operators who use these RPCs must configure the key and attach a token
on every call.

Bearer scheme matching is case-insensitive (RFC 6750), every handler
nil-checks req before dereferencing it, and tests now cover the
expired-token path.
 2026-05-12 10:11:08 -07:00
Chris Lu 05ed5c9ae8 filer: scope JWT allowed_prefixes to path components (#9439) 
The allowed_prefixes check used a literal byte-prefix match, so a token
scoped to /tenant1 also matched /tenant1234, /tenant1-old, and similar
sibling paths. Match on /-separated path components after path.Clean
normalisation instead.
 2026-05-12 10:10:48 -07:00
Chris Lu 18677a8430 fix(storage): refuse to load .vif-only entry as regular volume when .ecx exists (#9448) (#9461) 
fix(storage): refuse to load .vif-only entry as regular volume when .ecx exists

Defensive root-cause fix for issue #9448. The detection-side guard in
the EC plugin worker already breaks the infinite re-encode loop, but
the underlying volume-server state — `.vif` preserved next to EC shards
when the source replica is destroyed — can still re-arm a phantom
regular volume via the MountVolume / LoadVolume path.

In loadExistingVolume, the existing `.ecx`-present check is gated on
the caller passing `skipIfEcVolumesExists=true`. The startup-scan path
(concurrentLoadingVolumes) does pass that flag and correctly skips
the `.vif`. The LoadVolume → loadExistingVolume(…, false, …) path
used by VolumeMount does NOT, so it falls through to NewVolume, which
calls v.load with createDatIfMissing=true and creates a phantom
empty `.dat`. The master then reports the volume as regular and EC
detection re-proposes it.

Hoist the `.ecx`-present check so it runs unconditionally for `.vif`
entries: if the EC index is on the disk, the `.vif` belongs to those
EC shards, never to a regular volume that should be resurrected.
Pure OSS clusters never reach this exact state today (OSS deletes
`.vif` unconditionally during Destroy), but the guard hardens the
load path against any future path that leaves the same state.

Test:
- TestLoadExistingVolumeSkipsVifWhenEcxPresent builds the exact
  post-#9448 disk layout (`.vif` + `.ecx`, no `.dat`) and asserts
  loadExistingVolume(skipIfEcVolumesExists=false) returns false,
  does not create a placeholder `.dat`, and does not register a
  phantom volume in l.volumes.
 2026-05-12 09:30:42 -07:00
Chris Lu d221a64262 fix(ec): skip re-encode when EC shards already exist for the volume (#9448) (#9458) 
* fix(ec): skip re-encode when EC shards already exist for the volume (#9448)

When an earlier EC encoding succeeded but the post-encode source-delete
left a regular replica behind on one of the servers, the next detection
cycle proposes the same volume again. The new encode tries to redistribute
shards to targets that already have them mounted, the volume server
returns `ec volume %d is mounted; refusing overwrite`, the task fails,
and detection re-queues the volume. The cycle repeats forever — issue
#9448.

The existing `metric.IsECVolume` skip catches the case where the canonical
metric is reported on the EC-shard side of the heartbeat, but when the
master sees BOTH a regular replica AND its EC shards in the same volume
list, the canonical metric we pick is the regular replica and
IsECVolume is false. Add a second guard that checks the topology
directly via `findExistingECShards` (already present and indexed) and
skip the volume when any shards exist, logging a warning that points
the admin at the stuck source.

This breaks the loop. Auto-cleanup of the orphaned replica is left as
follow-up work — deleting a source replica from inside the detector is
only safe with a re-verification step right before the delete, plus a
config opt-in, and is best done in its own change.

* fix(ec): #9448 guard only fires when EC shard set is complete

The first version of the #9448 guard tripped on `len(existingShards) > 0`,
which is broader than necessary. The existing recovery branch in the
encode arm (around the `existingECShards` block, ~line 216) is designed
to fold partial leftover shards from a previously failed encode into
the new task as cleanup sources. Skipping unconditionally on any
existing shards made that branch dead code, regressing the recovery
behavior Gemini flagged in the review of af09e1ec7.

Two corrections:

  1. New helper `countExistingEcShardsForVolume` walks each disk's
     `EcIndexBits` bitmap and ORs the results into a `ShardBits`,
     returning the distinct-shard popcount. This is the right unit:
     a single `VolumeEcShardInformationMessage` can carry several
     shards, so `len(EcShardInfos)` is not the same as the number
     of present shards. Per Gemini's "use helper functions that walk
     the actual shard bitmap" note.
  2. The guard now fires only when `shardCount >= totalShards`.
     Partial shard sets fall through to the existing recovery branch,
     unchanged.

Tests:
  - TestDetectionSkipsWhenECShardsAlreadyExist: complete shards →
    no proposal (the regression test for #9448 itself, unchanged
    intent, rewritten on top of new helpers).
  - TestDetectionAllowsRegularReplicaWhenShardsPartial: partial
    shards → guard does NOT swallow the volume; the encode arm
    still gets a chance.
  - TestCountExistingEcShardsForVolume: the helper walks the
    bitmap correctly even when one info entry packs multiple
    shards on one disk.

The dangerous `volume.delete` hint in the warning is unchanged for
now — it gets fixed in the next commit.

* fix(ec): drop dangerous shell-command hint from #9448 warning

The previous warning told operators to run `volume.delete -volumeId=%d`
in the SeaweedFS shell to clean up the orphaned source replica. That
command is cluster-wide — it deletes every replica of the volume,
including the EC shards, which share the same volume id. Running it
in the state the message describes would cause the data loss the
guard exists to prevent.

Replace it with explicit guidance that the cleanup must be a targeted
VolumeDelete RPC against the source server only, and that the
shell command is the exact wrong thing to use here. The next two
commits add the plumbing and the auto-execution of that targeted
delete so most operators never see this hint at all.

Per Gemini comment on af09e1ec7.

* feat(worker): plumb grpc dial option through ClusterInfo

Add ClusterInfo.GrpcDialOption (optional) and set it in the
erasure_coding plugin handler. Lets the detector make targeted
gRPC calls during detection — used by the follow-up commit to
auto-clean orphan source replicas via VolumeDelete RPCs.

Zero-value safe: existing detectors that don't need RPC access
get a nil DialOption and ignore the field.

* feat(ec): auto-clean orphan source replica via targeted VolumeDelete

Builds on the previous commits: the guard now identifies the
#9448 stuck-source state and a gRPC dial option is available on
ClusterInfo. When both are true, detection auto-cleans the
orphaned regular replica instead of just warning the operator.

New helper `cleanupOrphanSourceReplicas`:

  1. Re-verifies the EC shard set is still complete via
     `countExistingEcShardsForVolume` against the live topology
     snapshot. If the count dropped between detection start and
     the cleanup decision (a volume server going down mid-cycle),
     it aborts — the source replica is the only complete copy and
     deleting it without a healthy shard set would be data loss.
  2. Issues targeted VolumeDelete RPCs to each regular-replica
     server via `operation.WithVolumeServerClient`. That RPC only
     touches the regular volume on the targeted server; EC shards
     live in a separate store path and are not affected. This is
     the safe alternative to the cluster-wide `volume.delete`
     shell command we previously warned against.

If the cleanup partially fails (one replica delete errors, others
succeed), detection logs the failure and continues to skip the
volume. The next detection cycle will try again. We deliberately
don't fall back to a re-encode because that would just collide
with the mounted shards on the targets again.

When no dial option is available the existing warning still
points operators at the safe manual procedure.
 2026-05-11 23:12:57 -07:00
Chris Lu 644664bbee feat(s3/lifecycle): swap daily_run to engine hash APIs (Phase 4a) (#9457) 
* feat(s3/lifecycle): swap daily_run to engine hash APIs (Phase 4a)

Replace the local replay-content-hash / max-effective-TTL helpers in
dailyrun with the engine package's canonical versions (ReplayContentHash,
MaxEffectiveTTL, PromotedHash) that landed with the Phase 4 view surface.

Adds PromotedHash to the cursor's recovery triggers: a partition flip
(rule moving between replay and walk because retention shifted) now
fires the rule-change branch alongside RuleSetHash mismatch. The
retentionWindow is set to MaxEffectiveTTL today, which keeps the
promoted set empty and the trigger dormant; Phase 4b will plumb the
real meta-log retention boundary so true scan_only promotions are
detected.

Cursor schema is unchanged — PromotedHash was already persisted as
the zero hash in Phase 2.

* docs(s3/lifecycle): note the one-time cursor rewind on hash format change

gemini-code-assist flagged that swapping localReplayContentHash for
engine.ReplayContentHash changes the persisted RuleSetHash byte layout
(sort order + tagged-field encoding). Phase-2 cursors mismatch on first
post-upgrade run and drop into the rule-change branch.

Going with option 3 (document the intentional one-time rewind). The
rewind is bounded to runNow - maxTTL (not time-zero), self-healing on
the next save, and daily_replay is off by default so the affected
population is limited to early adopters of the algorithm flag. A
migration shim or a hash-compat layer would carry the legacy encoder
forever for one bounded re-scan; not worth it.

Comment in runShard makes the trade explicit so a future reader doesn't
hunt for the "why does my cursor rewind once after upgrade" mystery.

* chore(s3/lifecycle): trim verbose comments in dailyrun

Cut multi-paragraph headers and narration that just described what the
code does. Kept the small WHY notes (per-match skip vs per-rule, the
one-time post-upgrade cursor rewind, scan_only rejection rationale).
Same behavior, ~150 fewer lines of comment.

* fix(s3/lifecycle): persist PromotedHash on the successful runShard save

The comment-trim pass dropped the field alongside a "stays empty in
Phase 2" comment. Harmless today (promoted is always zero), but Phase 4b
turns promoted into a real value — and a save that writes zero would
make the next run falsely detect drift and rewind. Spotted by
gemini-code-assist on PR 9457.

Other save paths (recovery, drain-error) already persisted it; the
success path is the only one that was missing it. Now consistent.
 2026-05-11 21:18:19 -07:00
Chris Lu 532b088262 fix(ec): preserve source disk type across EC encoding (#9423) (#9449) 
* fix(ec): carry source disk type on VolumeEcShardsMount (#9423)

When EC shards land on a target whose disk type differs from the
source volume's, master heartbeats wrongly reported under the target
disk's type. Add source_disk_type to VolumeEcShardsMountRequest; the
target server applies it to the in-memory EcVolume via SetDiskType so
the mount notification and steady-state heartbeat both carry the
source's disk type. Empty value falls back to the location's disk
type (used by disk-scan reload paths).

The override is not persisted with the volume — disk type stays an
environmental property and .vif remains portable.

* fix(ec): plumb source disk type through plugin worker (#9423)

Add source_disk_type to ErasureCodingTaskParams (field 8; 7 reserved),
populate it from the metric the detector already collects, thread it
through ec_task into the MountEcShards helper, and forward it on the
VolumeEcShardsMount RPC.

* fix(ec): mirror source disk type plumbing in rust volume server (#9423)

The volume_ec_shards_mount handler now forwards source_disk_type into
mount_ec_shard → DiskLocation::mount_ec_shards. When non-empty it
overrides ec_vol.disk_type (and each mounted shard's disk_type) via
the new set_disk_type method; empty value keeps the location's disk
type, so disk-scan reload and reconcile paths are unchanged.

Also picks up two pre-existing proto drifts that 'make gen' synced
from weed/pb (LockRingUpdate in master.proto, listing_cache_ttl_seconds
in remote.proto).

* feat(ec): bias placement toward preferred disk type (#9423)

Add DiskCandidate.DiskType and PlacementRequest.PreferredDiskType.
When PreferredDiskType is non-empty, SelectDestinations partitions
suitable disks into matching/fallback tiers and runs the rack/server/
disk-diversity passes on the matching tier first; the fallback tier
is only consulted if the matching pool can't satisfy ShardsNeeded.
PlacementResult.SpilledToOtherDiskType lets callers warn on spillover.

Empty PreferredDiskType keeps the existing single-pool behavior.

* fix(ec): plumb source disk type into placement planner (#9423)

diskInfosToCandidates now copies DiskInfo.DiskType into the placement
candidate, and ecPlacementPlanner.selectDestinations forwards
metric.DiskType as PreferredDiskType so EC shards land on disks
matching the source volume's disk type when possible. A glog warning
fires when placement had to spill to other disk types.

* test(ec): integration coverage for source-disk-type plumbing (#9423)

store_ec_disk_type_test exercises Store.MountEcShards end-to-end: a
shard physically lives on an HDD location, MountEcShards is called
with sourceDiskType="ssd", and the test asserts that the in-memory
EcVolume, the mounted shard, the NewEcShardsChan notification, and
the steady-state heartbeat all report under the source's disk type.
A companion test pins the empty-source path so disk-scan reload
keeps the location's disk type.

detection_disk_type_test exercises the worker plumbing: with a
cluster of nodes carrying both HDD and SSD disks, planECDestinations
must place every shard on SSD when metric.DiskType="ssd"; with only
one SSD node and 13 HDD nodes it must still satisfy a 10+4 layout
via spillover (and log a warning).

* revert(ec): drop unrelated proto drift in seaweed-volume/proto (#9423)

make gen pulled two pre-existing OSS changes into the rust proto
tree (LockRingUpdate / by_plugin in master.proto,
listing_cache_ttl_seconds in remote.proto). Reviewers flagged it as
scope creep — none of the rust EC fix references those fields.
Restore both files to origin/master so this branch only touches
EC-related symbols.

* fix(ec placement): treat empty disk type as hdd and skip used racks on spill (#9423)

partitionByDiskType used raw string comparison, so a PreferredDiskType
of "hdd" never matched candidates whose DiskType is "" (the
HardDriveType sentinel that weed/storage/types uses). EC encoding of
an HDD source would spill onto any HDD reporting "" even when the
cluster has plenty of matching capacity. Normalize both sides
through normalizeDiskType, which lowercases and folds "" → "hdd",
mirroring types.ToDiskType without taking a dependency on it.

selectFromTier's rack-diversity pass also kept revisiting racks the
preferred tier had already used when running on the fallback tier,
which negated PreferDifferentRacks on spillover. Skip racks already
in usedRacks so fallback placements still spread onto new racks.

* fix(ec): empty-source remount must not clobber existing disk type (#9423)

mount_ec_shards_with_idx_dir runs more than once per vid (RPC mount,
disk-scan reload, orphan-shard reconcile). After an RPC sets the
source-derived disk type, any later call passing source_disk_type=""
was resetting ec_vol.disk_type back to the location's value, which
reintroduces the heartbeat drift this PR is meant to fix. Only
default to the location's disk type when the EC volume is fresh
(no shards mounted yet); otherwise leave the recorded type alone so
empty-source reloads preserve whatever the original mount RPC set.
 2026-05-11 20:21:50 -07:00
Chris Lu 884b0bcbfd feat(s3/lifecycle): cluster rate-limit allocation (Phase 3) (#9456) 
* feat(s3/lifecycle): cluster rate-limit allocation (Phase 3)

Admin computes a per-worker share of cluster_deletes_per_second at
ExecuteJob time and ships it to the worker via
ClusterContext.Metadata. The worker reads the share, constructs a
golang.org/x/time/rate.Limiter, and passes it to dailyrun.Run via
cfg.Limiter (Phase 2 already plumbed the field). Phase 5 deletes the
streaming path; until then streaming ignores the cap.

Why allocate at admin: the cluster cap is a single knob operators
care about. Dividing it locally per worker would either need
out-of-band coordination or accept N× the configured budget. Admin
is the only party that knows how many execute-capable workers there
are, so it owns the math.

Admin side (weed/admin/plugin):
- Registry.CountCapableExecutors(jobType) returns the number of
  non-stale workers with CanExecute=true.
- New file cluster_rate_limit.go: decorateClusterContextForJob clones
  the input ClusterContext and injects two metadata keys for
  s3_lifecycle. cloneClusterContext duplicates Metadata so per-job
  decoration doesn't race shared base state.
- executeJobWithExecutor calls the decorator after loading the admin
  config; other job types pass through unchanged.

Worker side (weed/worker/tasks/s3_lifecycle):
- New cluster_rate_limit.go declares the constants both sides agree
  on (admin-config field names, metadata keys). Plain strings on the
  admin side keep weed/admin/plugin free of a dependency on the
  s3_lifecycle worker package; the two sets of constants are pinned
  to identical values and a mismatch would silently disable rate
  limiting.
- handler.go executeDailyReplay reads ClusterContext.Metadata,
  builds a rate.Limiter, and passes it into dailyrun.Config{Limiter}.
  Missing/empty/non-positive values → no limiter (legacy unlimited
  behavior). burst defaults to 2 × rate, clamped to ≥1 to avoid a
  bucket that never refills.
- Admin form gains two fields under "Scope": cluster_deletes_per_second
  (rate, 0 = unlimited) and cluster_deletes_burst (0 = 2 × rate).

Metric:
- New S3LifecycleDispatchLimiterWaitSeconds histogram observes how
  long each Limiter.Wait blocks before a LifecycleDelete RPC.
  Operators tune the cap by reading p95 — near-zero means the cap
  isn't binding, a long tail at 1/rate means it is.

Tests:
- weed/admin/plugin/cluster_rate_limit_test.go: 9 cases covering
  pass-through for non-allocator job types, rps=0 / no-executors
  skip, even sharing, burst sharing, burst=0 omit (worker default
  kicks in), burst floor of 1, no mutation of input metadata, nil
  input.
- weed/worker/tasks/s3_lifecycle/cluster_rate_limit_test.go: 7 cases
  covering nil/empty/missing metadata, non-positive/invalid rate,
  positive rate builds correctly, burst missing defaults to 2× rate,
  tiny rate clamps burst to ≥1.

Build clean. Phase 2 (#9446) and Phase 4 engine (#9447) are the
parents; this branch stacks on Phase 2 since it consumes
dailyrun.Config{Limiter} which lands there.

* fix(s3/lifecycle): divide cluster budget by active workers, not all capable

gemini pointed out that s3_lifecycle has MaxJobsPerDetection=1
(handler.go:189) — it's a singleton job, only one worker is ever active.
Dividing the cluster_deletes_per_second budget by the count of capable
executors gave the single active worker just 1/N of the configured cap.

Pass adminRuntime.MaxJobsPerDetection through to the decorator. Divisor
is now min(executors, maxJobsPerDetection), clamped to >=1. For
s3_lifecycle (maxJobs=1) the active worker gets the full budget; for a
hypothetical parallel-dispatch job (maxJobs>1) the budget divides
across the running-set.

Tests swap the SharedEvenly case for two pinned scenarios:
  - SingletonJobGetsFullBudget: maxJobs=1 across 4 executors => 100/1
  - SharedEvenlyWhenParallelLimited: maxJobs=4 across 4 executors => 25/worker
  - MaxJobsExceedsExecutors: maxJobs=10 across 4 executors => divisor 4

* feat(s3/lifecycle): drop Worker Count knob from admin config form

The "Worker Count" admin field controlled in-process pipeline goroutines
across the 16-shard space — per-worker tuning, not a cluster-wide scope
concern. Operators looking at the form alongside Cluster Delete Rate
reasonably misread it as the number of workers in the cluster.

Drop the form field and DefaultValues entry. cfg.Workers is now hardcoded
to shardPipelineGoroutines (=1) inside ParseConfig; the rest of the
plumbing through dailyrun.Config.Workers stays so a future need can
re-introduce it as a worker-local knob (or just bump the constant).

handler_test.go pins that "workers" must NOT appear in the form so the
removal doesn't silently regress.
 2026-05-11 19:17:06 -07:00
Chris Lu 3f4cb6d2fb feat(s3/lifecycle/engine): daily-replay view surface (Phase 4 engine) (#9447) 
* feat(s3/lifecycle/engine): daily-replay view surface (Phase 4 engine)

Adds the engine-side API the new daily-replay worker reaches for:
per-view snapshot construction (RulesForShard, RecoveryView), the two
cursor hashes that gate recovery (ReplayContentHash, PromotedHash),
and the cursor sliding-window helper (MaxEffectiveTTL). CurrentSnapshot
is a stub keyed on a package-level atomic that the worker startup wiring
populates.

Views return new *Snapshot instances holding cloned *CompiledAction
values so per-clone active/Mode never leak across partitions. Replay
clones force Mode=ModeEventDriven to rehabilitate any persistent
ModeScanOnly carried over from PriorState; walk and recovery clones
preserve Mode as-is. Disabled actions are excluded from all views.

No production caller is wired here — Phase 4's walker/dailyrun
integration is the follow-up. dailyrun's local helpers
(localReplayContentHash, localMaxEffectiveTTL) become one-line
redirects to these exports.

API surface:
- CurrentSnapshot() *Snapshot — stub until Phase 4 wiring.
- SetCurrentEngine(*Engine) — Phase 4 wiring entry point.
- Snapshot.RulesForShard(shardID, retentionWindow) (replay, walk *Snapshot)
- RecoveryView(s *Snapshot) *Snapshot — force-active over the full set.
- ReplayContentHash(s *Snapshot) [32]byte — partition-independent.
- PromotedHash(s *Snapshot, retentionWindow) [32]byte — partition-flip.
- MaxEffectiveTTL(s *Snapshot) time.Duration — over active replay only.

30 unit tests covering clone isolation, Mode rewrite, partition
membership including the multi-action-kind XML rule split,
RecoveryView activating pre-BootstrapComplete actions,
ReplayContentHash partition-independence, PromotedHash sensitivity to
promotion in either direction, MaxEffectiveTTL aggregation. Build +
race-tests green.

* refactor(s3/lifecycle/engine): consolidate hash helpers; clarify shardID semantics

Addresses PR #9447 review feedback. Three medium-priority items from
gemini, all code-quality refinements (no behavior change):

1. Duplicated sort comparator between ReplayContentHash and
   PromotedHash. Extract sortHashItems shared helper so the two
   hashes use the same ordering by construction — if one drifted, the
   cursor could see a spurious "rule changed" on a no-op snapshot
   rebuild.

2. Duplicated writeField/writeInt closures. Extract hashWriter struct
   holding the sha256 running hash + lenbuf, with method helpers.
   Same allocation profile (one Hash, one tiny stack buffer per
   helper); just deduplicates ~20 lines.

3. shardID parameter on RulesForShard is unused. Per the design's
   open question, every shard sees every rule today (shard filter
   runs at the entry-iteration site, not view construction). Keep
   the parameter for API stability — removing it now would force
   a breaking change when bucket-shard ownership lands — and update
   the doc comment to explain why it's reserved.

go build ./... clean; engine test suite green.
 2026-05-11 18:07:54 -07:00
Chris Lu 122ca7c020 feat(s3/lifecycle): daily-replay worker behind algorithm flag (Phase 2) (#9446) 
* docs(s3lifecycle): design for daily-replay worker

Captures the algorithm and dev plan iterated on in PR #9431 and the
discussion leading up to it: per-shard daily meta-log replay, walker
as a per-day pass for ExpirationDate/ExpiredDeleteMarker/NewerNoncurrent
plus a recovery branch over engine.RecoveryView(snap), explicit
retention-window input to RulesForShard, two cursor hashes
(ReplayContentHash + PromotedHash) that together detect every
invalidation case. Implementation phases are sequenced so each can
ship independently — Phase 1 (noncurrent_since stamp) just landed.

* feat(s3/lifecycle): daily-replay worker behind algorithm flag (Phase 2)

New weed/s3api/s3lifecycle/dailyrun package implementing the bounded
daily meta-log scan from the design doc. One pass per Execute per
shard: load cursor, scan events forward, route each through router.Route,
dispatch any due Match, advance the cursor on success. Halt-on-failure
keeps the cursor at the last fully-processed event so tomorrow resumes
from the same point — head-of-line blocking is the deliberate failure
signal.

Replay-only in this phase. Phase 4 wires the walker for ExpirationDate,
ExpiredDeleteMarker, NewerNoncurrent, and scan_only-promoted rules.
Until then a typed UnsupportedRuleError refuses runs on those buckets:
operators see the rejection in the activity log rather than silently
losing rules.

Behavior:
- Per-shard cursor {TsNs, RuleSetHash, PromotedHash} JSON-persisted
  under /etc/s3/lifecycle/daily-cursors/. PromotedHash always-empty in
  Phase 2; Phase 4 turns it on.
- Rule-change branch rewinds cursor to now - max_ttl when the
  replay-content hash mismatches. Cold start uses the same floor.
- Transport errors retry 3x with exponential backoff capped at 5s;
  server outcomes (RETRY_LATER / BLOCKED) halt the run without retry.
- Empty-replay sentinel: cursor TsNs=0 when no replay-eligible rules
  exist, only the hash gates a future addition.

Worker shape:
- New admin config field "algorithm" with enum streaming|daily_replay,
  default streaming. Existing deployments are unaffected.
- handler.Execute branches on the flag: streaming routes through the
  current scheduler.Scheduler, daily_replay routes through
  dailyrun.Run.
- dispatcher.NewFilerSiblingLister exported so both paths share the
  same .versions/ + null-bare lookup.

Engine integration:
- Local replayContentHash + maxEffectiveTTL helpers in dailyrun. Phase
  4's engine surface (ReplayContentHash, MaxEffectiveTTL) will replace
  them with one-line redirects; the local versions hash the same
  fields so the cursor stays valid across the swap.

Tests cover cursor persistence, unsupported-rule rejection,
hash stability under rule reordering, hash sensitivity to TTL edits,
max-TTL aggregation, dispatch retry budget, and request shape
including the identity-CAS witness.

Includes the design doc at weed/s3api/s3lifecycle/DESIGN.md so reviewers
and future phases share the same spec.

* feat(s3/lifecycle): default to daily_replay; streaming becomes the fallback knob

The streaming dispatcher hasn't shipped to users yet, so there's no
backward-compat surface to preserve. Flip the algorithm default from
streaming to daily_replay so the new path is the standard from day
one. Streaming stays as an explicit opt-in escape hatch during the
Phase 4 walker rollout; Phase 5 deletes both the flag and the
streaming code.

Buckets whose lifecycle rules require walker-bound dispatch
(ExpirationDate, ExpiredDeleteMarker, NewerNoncurrent, scan_only)
will fail the daily_replay run with the existing
UnsupportedRuleError until Phase 4 walker integration ships. Operators
hitting that case can set algorithm=streaming until the follow-up
lands.

Updates the test for the default value and renames the
unknown-value-fallback case to reflect the new default.

* fix(s3/lifecycle/dailyrun): drop per-rule done flag — it suppressed due matches

The done map was keyed by ActionKey = {Bucket, RuleHash, ActionKind}.
That's only safe when each event produces at most one match per
ActionKey with a single deterministic due-time formula —
ExpirationDays and AbortMPU fit that shape because due_time
= ev.TsNs + r.days is monotonic in event TsNs.

But NoncurrentDays paired with NewerNoncurrentVersions > 0 (allowed
in Phase 2 since it compiles to ActionKindNoncurrentDays) routes
through routePointerTransitionExpand, which emits matches for every
noncurrent sibling — each with its own SuccessorModTime taken from
the demoting event for that specific sibling. A single event can
therefore produce two matches for the same ActionKey on different
objects with wildly different DueTimes.

With the old code, a not-yet-due sibling encountered first would set
done[ActionKey] = true and then the next sibling — even though its
DueTime had already passed — would be skipped. Future events for the
same rule would also be suppressed for the rest of the run. Objects
that should have been deleted weren't.

Fix: drop the early-stop optimization. Process every match
independently. A future-DueTime match is now silently skipped without
affecting any later match. The performance hit is small (Phase 2 is a
single bounded daily pass, and the rate limiter is the real
throughput governor); the correctness gain is non-negotiable.

Also fixes the inverted comment in processMatches that described the
old check as "due_time is past now" when it actually checked
DueTime.After(now) (i.e., NOT yet due).

Adds four targeted tests:
- not-yet-due match first in slice does not suppress two later
  due matches for the same rule;
- reversed slice ordering produces identical dispatch;
- BLOCKED outcome halts the loop before later due matches are sent;
- empty match slice is a no-op.

Phase 4's walker-and-recovery integration can revisit a
per-(rule, object) memoization if profiling argues for it.

* fix(s3/lifecycle/dailyrun): address PR review — cursor advance, mode gate, ctx cancel, snapshot consistency

Addresses PR #9446 review feedback. Eight distinct fixes:

1. CURSOR ADVANCEMENT (gemini, critical). The old code advanced the
   persisted cursor to lastOK = TsNs of the last event processed,
   including events whose matches were skipped as not-yet-due. Those
   skipped matches would never be re-scanned, so objects under
   long-TTL rules would never expire.

   Track a "stuck" flag in drainShardEvents: the first event with a
   skipped (future-DueTime) match stops cursorAdvanceTo from rising,
   but the loop keeps processing later events to dispatch any that ARE
   due. The persisted cursor sits at the last fully-processed event so
   tomorrow's run re-scans from the skipped event onward and the
   future-due matches get re-evaluated when they age in.

   processMatches now returns (skippedAny, halted, err) so the drain
   loop can tell apart "event fully drained" from "event had pending
   future-due matches."

2. MODE GATE (gemini). checkSnapshotForUnsupported only checked the
   ActionKind. A replay-eligible kind with Mode != ModeEventDriven
   (e.g. ModeScanOnly via retention promotion) passed the check but
   then got silently ignored by router.Route, which gates dispatch
   on Mode == ModeEventDriven. Reject loudly with the typed error
   so admin sees the rejection in the activity log.

3. WORKERS CONFIG (gemini). The handler hardcoded 16 concurrent shard
   goroutines regardless of cfg.Workers. Add a Workers field to
   dailyrun.Config and gate the goroutine fan-out on a semaphore of
   that size; the handler now passes cfg.Workers through.

4. SINGLE SNAPSHOT PER RUN (coderabbit). Run() validated against one
   snapshot but runShard() pulled a fresh cfg.Engine.Snapshot() per
   shard. Mid-run Compile would let shards process different rule
   sets. Capture snap at the top of Run, pass it down to every shard.

5. FROZEN runNow (coderabbit). drainShardEvents and processMatches
   accepted a `now func() time.Time` and called it multiple times.
   DueTime comparisons would slip as the run wore on. Capture runNow
   once at the top of Run and thread it through as a time.Time value.

6. CTX CANCELLATION (coderabbit). The drain loop's <-ctx.Done() case
   broke out of the loop and returned nil, marking interrupted runs as
   successful. Return ctx.Err() instead so the caller propagates the
   interrupt; cursorAdvanceTo carries whatever progress was made.

7. CURSOR LOAD VALIDATION (coderabbit + gemini). The persister silently
   accepted empty files, mismatched shard_ids, and hash slices shorter
   than 32 bytes (copy() would zero-pad). Each now returns a typed
   error so the run halts and an operator investigates rather than
   silently re-scanning from time zero or persisting a zero-padded
   hash that masks corruption forever.

8. DEAD BRANCH (coderabbit). The "lastOK < startTsNs → keep persisted"
   guard in runShard was unreachable because drainShardEvents
   initialized lastOK := startTsNs and only ever raised it. Removed
   along with the new cursor-advancement semantics that handle the
   "no events processed" case implicitly.

Plus markdown lint: DESIGN.md fenced code blocks now carry a `text`
language identifier to satisfy MD040.

Skipped from the review:
- gemini's "maxTTL == 0 incorrectly skips immediate expirations":
  actions with Days <= 0 don't compile to a CompiledAction (see
  weed/s3api/s3lifecycle/action_kind.go: `if rule.X > 0`). The new
  empty-replay sentinel uses `rsh == [32]byte{}` for clarity per
  gemini's suggested form, but the behavior is equivalent.

Tests added/updated:
- TestProcessMatches_AllDueNoSkippedFlag pins skippedAny=false when
  all matches are past their DueTime.
- TestCheckSnapshotForUnsupported_NonEventDrivenModeRejected pins
  the new Mode check.
- TestFilerCursorPersister_EmptyFileReturnsError,
  _ShardIDMismatchReturnsError, _HashLengthMismatchReturnsError pin
  the new validation rules.
- Existing process-matches tests reshaped for the
  (skippedAny, halted, err) return tuple.

Full build clean. Dailyrun + worker test packages green.
 2026-05-11 18:07:17 -07:00
Chris Lu b2d24dd54f volume: require admin auth on BatchDelete (#9438) 
Run BatchDelete through checkGrpcAdminAuth like the other destructive
volume-server RPCs (VolumeDelete, DeleteCollection, vacuum, EC, ...),
so a whitelist-configured server denies non-admin callers.
 2026-05-11 13:50:48 -07:00
Chris Lu 2b21d19e4c volume: require admin auth on ReadAllNeedles and VolumeNeedleStatus (#9437) 
Both RPCs hand out raw needle bytes / cookies. Run them through
checkGrpcAdminAuth like the rest of the volume-server admin handlers.
 2026-05-11 13:50:19 -07:00
Chris Lu 46bb70d93e feat(s3): stamp noncurrent_since on versioned demotions (#9431) 
* feat(s3): stamp noncurrent_since on versioned demotions

A version's noncurrent TTL clock starts when the next version is
written, not at its own mtime. Today the lifecycle engine derives
that moment from the next-newer sibling's mtime — a heuristic that
drifts if the sibling is later modified and is unavailable when
the demoting event sits outside meta-log retention.

Stamp Seaweed-X-Amz-Noncurrent-Since-Ns on the demoted entry at
the two places where a PUT flips the latest pointer:
updateLatestVersionInDirectory and
updateIsLatestFlagsForSuspendedVersioning. Timestamp source is
time.Now().UnixNano() captured once per demotion — the documented
Phase 1 fallback until the filer write API surfaces its own TsNs.

Engine reads the stamp on both the bootstrap walker path and the
event-driven router; missing/zero falls back to the legacy
sibling-mtime derivation, so pre-stamp entries keep working.

Prerequisite for the daily-replay lifecycle worker (Phase 2+).

* fix(s3): address CI failure and PR review feedback

- Backdating tests must move both clocks: the lifecycle integration
  tests backdate version mtimes to simulate aging, but my earlier
  commit made the engine prefer the explicit demotion stamp over
  sibling mtime, so a real-now stamp dominated a backdated mtime and
  the rule never fired. Update backdateVersionedMtime to also rewrite
  Seaweed-X-Amz-Noncurrent-Since-Ns when the entry already carries it.
  This is a test simplification — production stamps record when the
  successor was written, not the demoted version's own mtime — but the
  resulting clock is correctly old enough.

- Refactor stamp parsing into one shared helper. Per gemini-code-assist:
  the parsing logic for ExtNoncurrentSinceNsKey was duplicated in
  router/router.go and scheduler/bootstrap.go. Move it to a new
  weed/s3api/s3lifecycle/noncurrent_since.go as exported
  SuccessorFromEntryStamp; both call sites now go through it.

- Make the parser ordering test deterministic. Per coderabbitai:
  time.Now().UnixNano() drops the monotonic clock component, so
  two back-to-back calls can decrease if the wall clock steps
  backward — the prior test was exercising OS clock behavior rather
  than the parser. Replace with fixed nanosecond values.

- Close a suspended-versioning race. Per coderabbitai: the prior
  putSuspendedVersioningObject called updateIsLatestFlagsForSuspendedVersioning
  after putToFiler returned, i.e. after the object write lock released.
  A concurrent PUT could promote a newer latest version, which we'd
  then wipe — leaving the older "null" object incorrectly current.
  Move the cleanup into the afterCreate callback so the null write and
  the .versions pointer clear (including the new demotion stamp) run
  atomically under the same lock. Best-effort logging is preserved.

* fix(s3/lifecycle): clear noncurrent_since stamp on test backdate

Backdating a version's mtime in tests is not a coherent claim about
when it became noncurrent — production stamps record the successor's
PUT time, which the test doesn't manipulate. The prior commit rewrote
the stamp to the backdated instant, but for TestLifecycleNewerNoncurrent
that creates an inconsistent state: v3's stamp says "demoted 30 days
ago" while v4's mtime (the supposed demoter) is real-now. With both
NewerNoncurrentVersions and NoncurrentDays in the same rule, the
NoncurrentDays floor passes against the backdated stamp and the
rank-based check then deletes v3 via the meta-log historical replay
that misranks against current state.

Clearing the stamp instead lets the lifecycle engine fall back to the
sibling-mtime derivation the tests were originally written against:
the legacy code path is preserved end-to-end while the new explicit-
stamp path is exercised by the unit tests in s3lifecycle/noncurrent_since_test.go
and the bootstrap-walker integration in scheduler/bootstrap_test.go.

The deeper interaction — historical meta-log replay ranking against
current state inside routePointerTransitionExpand — is pre-existing
and is no longer masked by the freshly-PUT successor's mtime once the
stamp is read. Tracked separately; not blocking this PR.

* fix(s3): stamp noncurrent_since before the .versions/ pointer flip

The pointer-flip on the .versions/ directory emits a meta-log event that
the lifecycle router consumes via routePointerTransition. The router
then calls LookupVersion on the demoted version's id. With the prior
ordering — pointer flip first, stamp second — the router could read
the demoted entry before markVersionNoncurrent landed and fall back to
the legacy sibling-mtime derivation.

Versioned COPY is the clean break: the new latest version keeps the
source object's mtime instead of recording the moment v_old was
demoted, so the fallback's successor clock can be arbitrarily wrong.
Reorder both updateLatestVersionInDirectory and
updateIsLatestFlagsForSuspendedVersioning so the stamp is written
first; the pointer flip then emits an event into a state where the
stamp is already present.

Failure of the stamp write remains non-fatal — lifecycle still falls
back to the legacy derivation in that case, with the same caveats as
before the PR but no race window.
 2026-05-11 13:41:33 -07:00
Chris Lu 6d12ebeefe fix(mount): fall through to filer when cached dir misses a tracked inode (#9436) 
lookupEntry returned ENOENT whenever the metaCache had the parent marked
cached but the child entry was absent. That's only correct when the
kernel has no record of the path either — when inodeToPath still maps
it, the three layers disagree (#9139). Triggers in practice under bursts
of concurrent metadata ops and after delete/rename events from another
mount drop the local entry without clearing the inode mapping; the test
flake fixed in b94ad8247 was the same shape on a smaller scale.

Trust the filer in that case: fall through to the existing
GetEntry path, which already loudly logs (Warningf with layer state)
when the filer also returns ErrNotFound, and otherwise serves the live
entry. Drop the Warningf from the cached-dir miss branch; it fires
thousands of times under 16-task rclone imports while the real error
path downstream covers the genuine-drift signal.
 2026-05-11 11:50:37 -07:00
Chris Lu 514ba7a233 fix(master): route ec shard vids to NewEcVids on initial subscribe (#9435) 
* fix(master): route ec shard vids to NewEcVids on initial subscribe

ToVolumeLocations appended EC shard volume IDs to NewVids, so a freshly
subscribing master client registered them in the regular-volume map via
addLocation instead of addEcLocation until the next heartbeat-driven
delta arrived. Append to NewEcVids to match the incremental path.

Fixes #9429

* fix(master): dedupe ec vids in initial subscribe snapshot

dn.GetEcShards returns per-(vid, diskId) entries, so a single EC volume
spread across multiple physical disks on one DataNode emitted the same
vid multiple times in NewEcVids. Dedupe so the snapshot carries each
vid once.
 2026-05-11 10:56:26 -07:00
Chris Lu defe047e1a perf(volume): stream-count the gzip size when no Content-MD5 is set (#9433) 
ParseUpload runs util.DecompressData on every gzipped multipart upload
just to record OriginalDataSize. The decompress materializes the full
uncompressed slice via bytes.Buffer.ReadFrom inside util.GunzipStream;
for a 64 MiB chunk that's a ~128 MiB heap spike per call (geometric
grow). On 6-way concurrent UploadPartCopy the spike dominated the
remaining heap profile after #9420/#9421/#9422/#9424/#9425.

When no Content-MD5 verification is requested the uncompressed bytes
aren't needed — only the length is. Stream the gunzip through
io.Discard and count: the pooled gzip.Reader's working set replaces
the materialized slice.

Unlike the previous attempt in #9426 the size still comes from the
real bytes, not from a client-set header.

  TotalAlloc per call, 4 MiB uncompressed body:
    materialize (was, still runs when MD5 is set):  ~16.8 MiB
    stream-count (no MD5):                             ~28 KiB

Refs #6541, #9426 (reverted in #9432).
 2026-05-11 10:56:11 -07:00
Chris Lu 6001d65206 perf(volume): stream-count the gzip size when no Content-MD5 is set (#9433) 
ParseUpload runs util.DecompressData on every gzipped multipart upload
just to record OriginalDataSize. The decompress materializes the full
uncompressed slice via bytes.Buffer.ReadFrom inside util.GunzipStream;
for a 64 MiB chunk that's a ~128 MiB heap spike per call (geometric
grow). On 6-way concurrent UploadPartCopy the spike dominated the
remaining heap profile after #9420/#9421/#9422/#9424/#9425.

When no Content-MD5 verification is requested the uncompressed bytes
aren't needed — only the length is. Stream the gunzip through
io.Discard and count: the pooled gzip.Reader's working set replaces
the materialized slice.

Unlike the previous attempt in #9426 the size still comes from the
real bytes, not from a client-set header.

  TotalAlloc per call, 4 MiB uncompressed body:
    materialize (was, still runs when MD5 is set):  ~16.8 MiB
    stream-count (no MD5):                             ~28 KiB

Refs #6541, #9426 (reverted in #9432).
 2026-05-11 10:55:56 -07:00
Chris Lu a64483885c fix(pb): skip Unix-socket gRPC registration on Windows (#9430) (#9434) 
The same-host gRPC fast path registered /tmp/...sock paths and dialed
them with net.Listen("unix", ...) / net.Dial("unix", ...). Those paths
are POSIX-only, so on Windows the listener failed at startup and every
local gRPC call routed through the registered port lost its transport.

Gate RegisterLocalGrpcSocket to return early on Windows. ServeGrpcOnLocalSocket
and resolveLocalGrpcSocket already short-circuit when no socket is registered,
so all same-host RPCs fall back to TCP without touching any callsite.
 2026-05-11 10:25:37 -07:00
Chris Lu ac65c6c2ca revert(volume): drop X-Seaweedfs-Original-Size hint (#9432) 
Reverts #9426. The header had the volume server record OriginalDataSize
from a value set by the multipart upstream — a client-controlled
metadata field. On a volume server that isn't JWT-protected, a caller
can lie and the needle stores the lie; bounds-checking the value
doesn't change the trust shape, only the magnitude of the lie. Derive
the size from the bytes again.

The optimization only fired on multipart Content-Encoding: gzip parts
(the s3 chunk-copy fast path), a narrow case that doesn't justify the
trust dependency. A future change can attack the same heap profile by
stream-decompressing to count bytes instead of materializing the
uncompressed slice — no client-trust surface.

Refs https://github.com/seaweedfs/seaweedfs/pull/9426#issuecomment-4417862793
 2026-05-11 10:08:55 -07:00
Chris Lu 8efa32258a feat(volume): X-Seaweedfs-Original-Size hint skips redundant gunzip (#9426) 
* feat(volume): X-Seaweedfs-Original-Size hint skips redundant gunzip

The full-chunk gzip pass-through (#9425) fixed source-volume
decompression but moved the cost to the destination volume:
parseUpload still ran util.DecompressData on the forwarded gzipped
bytes, just to learn the uncompressed length so it could record
OriginalDataSize in the needle metadata. For 6-way concurrent 64 MiB
UploadPartCopy that decompress-and-discard pass dominated the
remaining heap profile after the streaming chain landed (~297 MiB
inuse via bytes.Buffer.ReadFrom inside util.GunzipStream).

Add an X-Seaweedfs-Original-Size header on the multipart part. When
the upstream sets it (the s3 chunk-copy fast path always knows the
uncompressed size — it's the source chunk's logical size) and no
Content-MD5 verification is requested (which would require
decompressed bytes to compute against), parseUpload uses the hint
directly and skips the decompress.

Header is X-* prefixed (not Seaweed-*) so it doesn't get auto-stored
as a needle pair by PairNamePrefix.

Backward compatible:
- old s3 servers don't set the header, parseUpload decompresses as
  before
- new s3 servers talking to old volumes: header is ignored, volume
  decompresses
- bad header values (non-numeric, negative, garbage) fall back to the
  existing decompress path

End-to-end repro impact (512 MiB src, 6 parallel UploadPartCopy,
post-#9420/#9421/#9422/#9424/#9425 baseline):

  RSS, round 2:        1149 MiB → 594 MiB
  heap inuse_space:     545 MiB → 349 MiB
  HeapSys:             1.35 GiB → 777 MiB
  TotalAlloc cum:        ~9 GiB → 3.5 GiB

Total reduction from pre-#9420 baseline: 3134 → 594 MiB (-81%).

Test exercises the four matrix corners (hint+no-MD5,
hint+part-MD5, hint+req-MD5, no-hint, garbage-hint) and bounds
allocation per case so a regression that re-introduces the
unconditional decompress fails the hint-present-no-MD5 case.

* review: bound X-Seaweedfs-Original-Size by sizeLimit and uint32

CodeQL on PR 9426 traced a new taint flow: the strconv.Atoi(hint)
value flows into pu.OriginalDataSize -> originalSize -> the existing
uint32(originalSize) cast in volume_server_handlers_write.go:73. The
cast was always there but its input was previously bounded by the
ParseUpload read path (capped at sizeLimit). Adding a user-controlled
hint bypassed that bound, so a malicious header could overflow the
uint32 silently.

Bound the hint at parse time by sizeLimit (the largest needle this
volume will accept anyway) and by math.MaxUint32 (belt-and-suspenders
in case sizeLimit is configured > 4 GiB).
 2026-05-10 15:57:07 -07:00
Chris Lu 9a70bbfcc6 feat(s3api): full-chunk gzip pass-through skips volume-side decompress (#9427) 
Building on the io.Pipe streaming chunk copy: when a copy operation
covers an entire source chunk (the common case for Harbor's
part-size = chunk-size assemble pattern), ask the source volume for
compressed bytes via Accept-Encoding: gzip and forward them to the
destination as-is.

This trades a Range fetch (where the volume decompresses the chunk
internally to satisfy the byte range) for a full-chunk fetch that
returns whatever wire bytes the chunk is stored as. For gzipped
chunks the source volume avoids the decompression entirely; we never
allocate a chunk-sized decompress buffer.

Implementation: build the source GET directly instead of going
through ReadUrlAsStream, because that helper auto-decompresses gzip
responses (which would defeat the point). Trust the response's
Content-Encoding header over caller hints — for partial ranges the
volume always returns raw bytes regardless of how the chunk is
stored, so labeling those as gzip would corrupt subsequent reads.

End-to-end repro impact (512 MiB src, 6 parallel UploadPartCopy):
  + #9420/#9421/#9422       : 2236 MiB
  + io.Pipe streaming       : 1521 MiB
  + this commit             : 1149 MiB  (round 2 RSS, perfectly flat)

Round 3 now completes (was hitting volume-full before, since
chunks took up uncompressed space on disk; we now store the gzipped
chunks the volume gives us, which fit in the test's 8 GiB volume
budget).

Heap inuse_space (after force GC):
  before all: ~1.5 GiB
  this PR:    266 MiB

Volume-side bytes.Buffer.ReadFrom inuse:
  before:     611 MiB
  streaming:  571 MiB
  this PR:    297 MiB (now in destination-volume parseUpload's
                       size-hint decompression — separate
                       optimization opportunity for a hint header)
 2026-05-10 14:55:59 -07:00
Chris Lu 4a04594826 feat(s3api): stream chunk copy via io.Pipe to cut peak working set (#9424) 
* fix: cap pool retention so chunk-copy buffers don't hoard memory

Two pool-retention sites kept the runaway-RSS pattern in #6541 visible
even after #9420 and #9421:

* weed/util/buffer_pool: SyncPoolPutBuffer dropped a buffer back into
  sync.Pool regardless of how big it had grown. After a 64 MiB chunk
  upload through volume.PostHandler -> needle.ParseUpload, the pool
  hoarded a 64 MiB byte array per cached entry for the rest of the
  process's lifetime. Cap retention at 4 MiB; oversized buffers are
  dropped so GC can reclaim the backing array.

* weed/s3api/...copy.go: uploadChunkData left UploadOption.BytesBuffer
  unset, so operation.upload_content fell back to the package-global
  valyala/bytebufferpool. That pool also retains high-water buffers
  forever, and concurrent UploadPartCopy filled it with one chunk-sized
  buffer per concurrent upload. Provide a fresh per-call bytes.Buffer
  pre-sized to chunk + multipart framing; it's GC'd as soon as the
  upload returns.

Tests:
- weed/util/buffer_pool/sync_pool_test.go: pin the cap (oversized
  buffers don't round-trip), the inverse (right-sized buffers do), and
  nil-safety.
- weed/s3api/...copy_chunk_upload_test.go: extract newChunkUploadOption
  and pin that BytesBuffer is always non-nil and pre-sized, and that
  each call gets a distinct buffer.

* feat(s3api): stream chunk copy via io.Pipe to cut peak working set

Final piece for #6541. The buffered chunk-copy path holds two
chunk-sized buffers per copy in flight (download buffer + multipart-
encoded upload buffer). Under concurrent UploadPartCopy that put a
floor on RSS at concurrency × 2 × chunk_size — about 768 MiB for the
6-way / 64 MiB Harbor-style assemble repro, even after the previous
pool/retention fixes.

Replace the buffered path with an io.Pipe between the source GET and
the destination POST: ReadUrlAsStream pumps data into the pipe via a
multipart.Writer, the http.Client reads from the pipe end and POSTs
the body. In-flight per copy is now ~32 KiB (pipe hand-off + http
buffers), regardless of chunk size.

The streaming path is gated by canStreamCopyChunk: only used when no
in-transit transformation is needed (no per-chunk CipherKey, no SSE).
SSE-C / SSE-KMS / SSE-S3 paths still go through the buffered path,
which already handles re-encryption correctly.

Benchmarks (Apple M4, httptest source/dest, B/op = bytes per copy):

  Buffered  1 MiB:   6.0 MB B/op,  443 MB/s
  Streamed  1 MiB:   374 KB B/op,  727 MB/s
  Buffered  8 MiB:    56 MB B/op,  559 MB/s
  Streamed  8 MiB:   379 KB B/op, 1138 MB/s
  Buffered 64 MiB:   455 MB B/op,  718 MB/s
  Streamed 64 MiB:   304 KB B/op, 1387 MB/s

End-to-end repro (512 MiB src, 6 parallel UploadPartCopy):
  pre-#9420 RSS round 2: 3134 MiB
  + #9420/#9421/#9422  : 2236 MiB
  + this PR            : 1521 MiB
  heap inuse_space     :  350 MiB (was 1422 / 1187 MiB)
  HeapSys (MemStats)   : 1.74 GiB (was 2.49 GiB)

* review: surface shouldRetry, add int32 guard, drop redundant drains

Address review on PR 9424:

* coderabbit (HIGH, line 122): ReadUrlAsStream can set shouldRetry=true
  with readErr=nil. Before this fix, that fell through to mw.Close()
  and the destination POST succeeded against a possibly-truncated
  multipart body. Mirror downloadChunkData's explicit check and
  surface shouldRetry as a producer error so the dst POST aborts.
* gemini (line 98): chunk size is int64 but ReadUrlAsStream takes int.
  Reject sizes above MaxInt32 up front so the int(size) cast can't
  truncate negative on 32-bit platforms — same guard downloadChunkData
  uses.
* gemini (line 151): util_http.CloseResponse already drains the body
  (io.Copy(io.Discard, ...) inside the helper) before closing, so the
  manual io.Copy drains we added are redundant. Drop them.

* review: cancel source GET when destination POST fails

Address coderabbit review (line 165 / second pass on PR 9424): when
the POST leg fails or returns an error status, closing pipeReader
only fails the producer's *writes*. ReadUrlAsStream's own read loop
runs under the parent ctx, so it keeps draining the source body in
the background until EOF — wasting source-volume bandwidth and CPU
on a copy that's already failed.

Wrap streamCopyChunkRange in a child context cancelled on return.
ReadUrlAsStream checks ctx.Done() per 256 KiB tick, so the in-flight
read aborts on the next iteration once the function returns. The POST
also moves to streamCtx so the in-flight request can be cancelled the
same way if the producer fails first.

Defer-cancel runs after both legs return, so the success path still
sends EOF cleanly through pipeWriter.Close before cancellation.
 2026-05-10 14:29:39 -07:00
Chris Lu d8bbc1d855 fix: cap pool retention so chunk-copy buffers don't hoard memory (#9422) 
Two pool-retention sites kept the runaway-RSS pattern in #6541 visible
even after #9420 and #9421:

* weed/util/buffer_pool: SyncPoolPutBuffer dropped a buffer back into
  sync.Pool regardless of how big it had grown. After a 64 MiB chunk
  upload through volume.PostHandler -> needle.ParseUpload, the pool
  hoarded a 64 MiB byte array per cached entry for the rest of the
  process's lifetime. Cap retention at 4 MiB; oversized buffers are
  dropped so GC can reclaim the backing array.

* weed/s3api/...copy.go: uploadChunkData left UploadOption.BytesBuffer
  unset, so operation.upload_content fell back to the package-global
  valyala/bytebufferpool. That pool also retains high-water buffers
  forever, and concurrent UploadPartCopy filled it with one chunk-sized
  buffer per concurrent upload. Provide a fresh per-call bytes.Buffer
  pre-sized to chunk + multipart framing; it's GC'd as soon as the
  upload returns.

Tests:
- weed/util/buffer_pool/sync_pool_test.go: pin the cap (oversized
  buffers don't round-trip), the inverse (right-sized buffers do), and
  nil-safety.
- weed/s3api/...copy_chunk_upload_test.go: extract newChunkUploadOption
  and pin that BytesBuffer is always non-nil and pre-sized, and that
  each call gets a distinct buffer.
 2026-05-10 13:34:25 -07:00
Chris Lu c917110124 fix(volume): pre-size ParseUpload buffer to request ContentLength (#9421) 
* fix(volume): pre-size ParseUpload buffer to request ContentLength

The volume server's PostHandler reads the multipart upload body via
bytes.Buffer.ReadFrom inside parseUpload. The buffer comes from a
sync.Pool and may have cap=0 when the pool dropped the prior entry,
which makes ReadFrom geometric-grow on each chunk: a 64 MiB upload
allocates roughly 1+2+4+...+64 ≈ 128 MiB just to receive the body.
Under concurrent uploads (every s3 chunk-copy lands here on the
destination volume) this is one of the main contributors to the
runaway-RSS pattern in #6541 — pprof shows ~458 MiB cum in
parseUpload's bytes.Buffer.ReadFrom under Harbor-style assemble load.

Grow the buffer once up front, bounded by the existing sizeLimit so
a misreported Content-Length can't over-allocate. The receive then
fills in place.

Add a regression test that drives ParseUpload with a 16 MiB multipart
body and bounds TotalAlloc at 1.5x the chunk size (pre-fix measures
~4x, so the bound trips deterministically).

* fix(volume): guard ParseUpload pre-grow against int overflow on 32-bit

Address PR review feedback: r.ContentLength is int64, and on 32-bit
platforms int is 32 bits wide, so int(r.ContentLength) for a value above
math.MaxInt32 wraps negative and bytes.Buffer.Grow panics with
"bytes.Buffer.Grow: negative count". Skip the pre-grow optimization in
that range; the existing geometric-grow path remains correct, just
slightly more allocator pressure for that one call.

64-bit platforms (math.MaxInt == math.MaxInt64) are unaffected — the
guard only kicks in for 32-bit builds with very large sizeLimit.

* fix(volume): cap ParseUpload pre-grow at 4 MiB to bound DoS surface

Address PR review: pre-growing the receive buffer to r.ContentLength
trusts the header before any body bytes arrive. A bad header or slow /
idle client could declare a large Content-Length up to sizeLimit (256
MiB by default for volume writes) and force per-request preallocation
without sending data, turning many concurrent slow connections into
avoidable memory pressure.

Cap eager pre-grow at maxEagerPreGrow (4 MiB). Larger uploads still
benefit from the higher starting cap and fall back to ReadFrom's grow
path for the remainder. Per-request waste from a misreported
Content-Length is now bounded at 4 MiB regardless of sizeLimit.

Extract the policy as eagerPreGrow so the unit test can exercise the
gates structurally — replaces the prior TotalAlloc bound (which became
uninformative once savings were capped at 4 MiB).
 2026-05-10 12:08:24 -07:00
Chris Lu 926a8e9351 fix(s3api): cap copy-chunk receive buffer to avoid append-grow blowup (#9420) 
* fix(s3api): cap copy-chunk receive buffer to avoid append-grow blowup

downloadChunkData accumulated the streamed chunk into a nil []byte via
`chunkData = append(chunkData, data...)`. ReadUrlAsStream pumps in 256 KiB
ticks, so a 64 MiB chunk grew the slice geometrically (256K → 512K →
1M → ... → 64M), allocating ~2x the chunk size for every transferred
byte. Combined with the 4-way per-request concurrency and any number of
in-flight UploadPartCopy calls (Harbor multipart assemble), this is what
produces the runaway-RSS pattern reported in #6541.

Pre-size the receive buffer to the known sizeInt so the callback fills
in place. Add a regression test that downloads a 16 MiB chunk through
httptest and asserts TotalAlloc stays under 1.5x the chunk size — the
pre-fix code allocates ~5x and trips the bound.

Local repro (weed 4.23, 6 parallel UploadPartCopy on a 512 MiB source):

  before:  baseline 96 MiB → peak 3124 MiB, never reclaimed
  pprof:   650 MiB inuse in bytes.growSlice + 461 MiB in
           downloadChunkData.func1

* test(s3api): assert downloaded chunk content matches payload

Address PR review feedback: the allocation-bound check alone would still
pass if a future regression silently truncated or corrupted the chunk.
Compare the returned bytes against the source payload (after the
TotalAlloc measurement window so bytes.Equal doesn't pollute it).
 2026-05-10 12:08:06 -07:00
Chris Lu ca12934834 docs(s3/lifecycle): reflect shipped reader, obsolete Phase 6 (#9419) 
* test(s3/lifecycle/engine): pin delay-group dedup across buckets

Compile a 100-bucket × 5-rule snapshot where the five Days values
include duplicates (1, 1, 7, 7, 30) and assert:

- snap.actions has 500 entries — every (bucket, rule) compiles to its
  own ActionKey, no collapse.
- snap.originalDelayGroups has exactly 3 entries — the routing index
  is keyed by Delay, so same-day rules across all buckets share a
  group. This is the property that lets the dispatcher index by
  delay group rather than per-rule.
- Per-group key count = (rules with that day) × buckets, so every
  action is reachable from its group entry.

* docs(s3/lifecycle): reflect shipped reader, obsolete Phase 6

The reader didn't end up building per-filer-shard cursors,
RetainedLogRangePerShard / EarliestRetainedPositionPerShard probes,
tail_drained_streams GC, or CollectLogFileRefs heap-merge. The
filer's SubscribeMetadata already aggregates persisted + in-memory
logs across peer filers, so a single global cursor on a single
subscription is sufficient.

Mark the per-shard architecture and lost-log GC pseudocode as
implementation-note + obsoleted, rewrite Phase 6 to call out what
was descoped and why, point readers at the narrow ResumeFromDiskError
auto-degrade follow-up that is the only residual gap, and update
Layer 4 to note no multi-filer test harness is needed.

* docs(s3/lifecycle): drop stale per-shard cursor wording in Phase 3

The "one sweep per delay group" bullet still described
last_processed_original / last_processed_predicate as per-shard maps,
contradicting the implementation note added a few lines above. The
shipped reader stores one global (ts_ns, offset) position per delay
group; rewrite the bullet to match.
 2026-05-10 10:40:33 -07:00
Chris Lu 82648cca53 test(s3/lifecycle/engine): pin delay-group dedup across buckets (#9418) 
Compile a 100-bucket × 5-rule snapshot where the five Days values
include duplicates (1, 1, 7, 7, 30) and assert:

- snap.actions has 500 entries — every (bucket, rule) compiles to its
  own ActionKey, no collapse.
- snap.originalDelayGroups has exactly 3 entries — the routing index
  is keyed by Delay, so same-day rules across all buckets share a
  group. This is the property that lets the dispatcher index by
  delay group rather than per-rule.
- Per-group key count = (rules with that day) × buckets, so every
  action is reachable from its group entry.
 2026-05-10 10:36:54 -07:00
Chris Lu 1b1d4aa814 refactor(s3/lifecycle): extract entryUsesMetadataOnlyDelete predicate (#9417) 
* test(s3/lifecycle): integration coverage for versioning + filters

First integration-test bundle building on the existing single-test
backdating harness. Each scenario follows the same shape: create
bucket, set lifecycle, PUT object, backdate mtime via filer
UpdateEntry, run the shell command for one shard sweep, assert
S3-side state.

Five new tests:

- TestLifecycleVersionedBucketCreatesDeleteMarker: Expiration on a
  versioned bucket must produce a delete marker (latest after worker
  runs is a marker) AND keep the original version directly addressable
  by versionId. ListObjectVersions confirms IsLatest=true on the
  marker.

- TestLifecycleNoncurrentVersionExpiration: NoncurrentVersionExpiration
  fires only on demoted versions. PUT v1, PUT v2 (so v1 → noncurrent),
  backdate v1, run worker. v1 must be gone, v2 still current.

- TestLifecycleExpiredDeleteMarkerCleanup: combined rule (noncurrent +
  expired-delete-marker) cleans up a sole-survivor marker. PUT v1,
  DELETE (creates marker), backdate both, run worker. Every version
  AND marker must be gone for the key.

- TestLifecycleDisabledRuleSkipsObject: rule with Status=Disabled
  must not produce dispatches even on a backdated match. Negative
  test for the engine's enabled-status gate.

- TestLifecycleTagFilter: rule with And{Prefix, Tag} only matches
  objects carrying the tag. Two backdated objects (one tagged, one
  not) — only the tagged one is removed.

Helpers extracted to keep each test focused: putVersioningEnabled,
putNoncurrentExpirationLifecycle, putExpiredDeleteMarkerLifecycle,
backdateVersionedMtime (ages a specific .versions/v_<id> entry),
runLifecycleShard (one-shot shell invocation with FATAL guard).

* test(s3/lifecycle): tighten noncurrent expiration diagnostics

Local run showed TestLifecycleNoncurrentVersionExpiration failing
with a bare 404 on HEAD(latest), not enough to tell whether v2 was
deleted, the bare-key pointer was removed, or a delete marker was
synthesized. Strengthen the test to:

- HEAD by versionId=v2 first, so we pin "v2 file still on disk"
  separately from "the latest pointer resolves to v2"
- on HEAD(latest) failure, log ListObjectVersions output (versions +
  markers, with IsLatest) so the next failure shows which side the
  bug is on rather than just NotFound

* test(s3/lifecycle): integration coverage for AbortIncompleteMultipartUpload

Exercises the lifecycleAbortMPU handler path that the prefix-based
expiration tests can't reach — routing keys off of .uploads/<id>/
directory events, not regular object events, and the dispatcher uses
a different RPC path (rm on the .uploads/<id>/ folder).

Setup: AbortIncompleteMultipartUpload rule with DaysAfterInitiation=1,
CreateMultipartUpload, UploadPart (so the directory carries the
right shape), backdate the .uploads/<uploadID>/ directory entry 30
days, run the worker. The upload must drop out of
ListMultipartUploads.

Helpers added: putAbortMPULifecycle, backdateUploadDir.

* test(s3/lifecycle): integration coverage for NewerNoncurrentVersions

NewerNoncurrentVersions=N keeps the N most recent noncurrent versions
and expires the rest. Distinct from per-version NoncurrentDays —
depends on per-version rank, not just per-version age — and routes
through routePointerTransition's "needs full expansion" path.

Setup: PUT v1, v2, v3, v4 on a versioned bucket (v4 current; v1-v3
noncurrent), backdate v1+v2+v3 so all satisfy the NoncurrentDays>=1
floor, run the worker. Expect v1+v2 expired (older noncurrent),
v3 (newest noncurrent within keep=1) and v4 (current) preserved.

Helper added: putNewerNoncurrentLifecycle.

* test(s3/lifecycle): integration coverage for suspended-versioning Expiration

Suspended versioning takes a distinct code path in lifecycleDispatch:
the VersioningSuspended branch first deletes the null version (via
deleteSpecificObjectVersion(versionId="null")) and then writes a
fresh delete marker on top. Other branches (Enabled → only writes a
marker; Off → straight rm) miss this two-step.

Setup: enable versioning, PUT v1 (real versionId), suspend
versioning, PUT again (creates the null version, demotes v1 to
noncurrent), set the Expiration rule, backdate the null at the
bare path. Expect: latest is now a fresh delete marker, the
"null" version is gone from ListObjectVersions, and v1 (noncurrent
under Enabled) still addressable directly — suspended Expiration
must only touch the null, not other versions.

Helper added: putVersioningSuspended.

* test(s3/lifecycle): integration coverage for multi-bucket sweep

A single shell-driven shard sweep must process every bucket carrying
lifecycle config, not just the first one alphabetically. Pinned
because the scheduler iterates the buckets directory and a regression
that returns early after the first match would silently disable
lifecycle for every later bucket.

Two buckets, each with their own prefix-expiration rule and a
backdated object. Both must be expired after the same sweep.

* test(s3/lifecycle): integration coverage for ObjectSizeGreaterThan filter

ObjectSizeGreaterThan is a strict > gate (filterAllows uses
ev.Size <= rule.FilterSizeGreaterThan to reject). Pinned at the
boundary: an object whose size equals the threshold must remain;
only an object strictly larger expires. Catches a > vs >= flip.

Two backdated objects on the same prefix, sizes 100 and 150 with
threshold=100 — boundary survives, larger expires.

* test(s3/lifecycle): scrub bucket lifecycle config + versions on cleanup

Tests share one weed mini server. Two pollution modes were producing
order-dependent failures:

- A later test's shard sweep would still load the prior test's
  lifecycle config (the worker reads every bucket's XML from filer
  state, and DeleteBucket alone doesn't drop lifecycle config
  cleanly on this codebase).
- Versioned-bucket tests left versions + delete markers behind that
  ListObjectsV2 can't see, so the existing best-effort empty-then-
  delete didn't actually empty those buckets.
- The AbortMPU test intentionally leaves an in-flight upload; without
  an explicit AbortMultipartUpload the bucket DELETE hits NotEmpty.

Cleanup now runs DeleteBucketLifecycle, ListObjectVersions →
DeleteObject(versionId), ListObjectsV2 → DeleteObject (catches what
ListObjectVersions missed), ListMultipartUploads → AbortMultipartUpload,
then DeleteBucket. Best-effort throughout so a half-torn-down bucket
doesn't fail the cleanup chain.

* test(s3/lifecycle): backdate both versions for NoncurrentDays clock

Per codex review: NoncurrentDays is clocked from the SUCCESSOR
version's mtime (when the displaced version became noncurrent), not
from the displaced version's own mtime. Backdating only v1 left the
clock (v2's mtime) at "now" and the rule never fired — the test was
wrong, not the production path.

Backdate v1=31d and v2=30d so v1 sits past the 1-day threshold
relative to v2, the noncurrent rule fires, and v2 stays current.

* test(s3/lifecycle): assert specific NotFound on multi-bucket deletion

Per codex review: TestLifecycleMultipleBucketsInOneSweep treated any
HeadObject error as "deleted", which lets a transport failure or
dead endpoint mask a real bug. Recognize NoSuchKey/NotFound/HTTP-404
specifically via a small isS3NotFound helper so the assertion
actually proves deletion happened, not just that the call broke.

* test(s3/lifecycle): gofmt size-filter test

* test(s3/lifecycle): integration coverage for Object Lock skip

Object Lock retention must override the lifecycle rule. The handler's
enforceObjectLockProtections check (s3api_internal_lifecycle.go:47)
returns an error when retention is active; the dispatcher then
classifies the outcome as SKIPPED_OBJECT_LOCK and the object stays.
No existing integration test reaches that outcome.

Setup: bucket created with ObjectLockEnabledForBucket=true, expiration
rule on prefix "lock/", two backdated objects under the same prefix —
one with GOVERNANCE retention until 1h from now, one without. After
the worker runs, the unlocked object expires (positive control); the
locked one survives.

Custom cleanup uses BypassGovernanceRetention so the test can drop
the locked version when the test finishes — otherwise the retention
window keeps the bucket from being deleted.

* test(s3/lifecycle): integration coverage for config update between sweeps

An operator changes the lifecycle rule between two shell-driven
sweeps. The second sweep must respect the NEW rule, not a cached
copy of the old one. Each runLifecycleShard invocation spawns a
fresh weed shell subprocess, so cached engine state from a previous
sweep doesn't persist — but a regression that caches rules across
PutBucketLifecycleConfiguration calls within the S3 server itself
would still surface here.

Sweep 1: rule prefix="first/", PUT + backdate firstKey, run worker
→ firstKey expires.

Update rule to prefix="second/", PUT + backdate secondKey AND a
new key under the OLD prefix ("first/post-update.txt"). Sweep 2
must expire only the second-prefix object; the post-update old-
prefix one must survive — config replacement, not merge.

* test(s3/lifecycle): integration coverage for ExpirationDate (past)

Rules with Expiration{Date: <past>} route through ScanAtDate in the
engine (decideMode's ActionKindExpirationDate case) — a separate
compile + dispatch branch from the EventDriven delay-group path the
Days-based tests exercise.

Past date + in-prefix object → must expire. Out-of-prefix object →
must remain. Object also backdated as defense-in-depth so the
assertion doesn't depend on whether the dispatcher consults
MinTriggerAge for date kinds.

* test(s3/lifecycle): integration coverage for bootstrap walk on existing objects

Production scenario: operator enables lifecycle on a bucket that
already holds objects from before the policy. The worker must
discover them via the bootstrap walk (BucketBootstrapper) — there
were no meta-log events to observe because the objects predate the
rule. Without the bootstrap path, only NEW writes would ever match.

Setup: PUT 5 objects (no lifecycle config yet) + 1 out-of-prefix
survivor, backdate all, THEN set the Expiration rule, run the
worker. Every in-prefix pre-existing object must be expired; the
out-of-prefix one must remain.

* test(s3/lifecycle): integration coverage for DeleteBucketLifecycle stops dispatching

Operator UX: after DeleteBucketLifecycle, the worker must observe the
removal on the next sweep and stop expiring objects under the now-gone
rule. A regression that caches old configs across
PutBucketLifecycleConfiguration → DeleteBucketLifecycle would keep
silently dropping objects.

Setup: positive control (rule active, backdated obj expires) →
DeleteBucketLifecycle → PUT + backdate a fresh object → second
sweep. The fresh object must remain.

* test(s3/lifecycle): integration coverage for empty bucket sweep no-op

A bucket carrying lifecycle config but no objects must produce a
successful sweep — no hangs, no errors, no dispatches. Pinned
because the bootstrap walker iterates bucket directories, and an
empty directory is a corner of that traversal that's easy to break
(slice-bounds bug on the first listing returning zero entries).

Asserts: worker logs "loaded lifecycle for" and "shards 0-15
complete", no FATAL output, bucket still exists after the sweep.

* test(s3/lifecycle): fix Object Lock backdate path + skip unwired ScanAtDate

ObjectLock: enabling Object Lock on a bucket implicitly enables
versioning, so PUT objects land at .versions/v_<id>, not at the bare
key. The test was calling backdateMtime (bare path) and failing in
the helper with "filer: no entry is found". Switch to
backdateVersionedMtime with the versionId returned by PutObject.

ExpirationDate: ScanAtDate dispatch path isn't wired to the run-shard
shell command yet — the bootstrap walker explicitly skips actions in
ModeScanAtDate (walker.go:141 says "SCAN_AT_DATE runs its own date-
triggered bootstrap" but no such bootstrap exists in the scheduler or
shell). Skip with a t.Skip + explanation so the test activates the
moment the date-triggered path lands.

* fix(s3/lifecycle): wire ExpirationDate dispatch through bootstrap walker

The walker explicitly skipped ModeScanAtDate actions on the comment
"SCAN_AT_DATE runs its own date-triggered bootstrap" — but no such
bootstrap exists in the scheduler or shell layer. The result: rules
with Expiration{Date: ...} compiled correctly, populated the
snapshot's dateActions map, and were never dispatched.
ExpirationDate is silently a no-op in production.

EvaluateAction already handles ActionKindExpirationDate correctly
(rejects when now.Before(rule.ExpirationDate), otherwise emits
ActionDeleteObject). The walker just needed to fall through instead
of skipping. Pre-date walks become no-ops via EvaluateAction's date
check; post-date walks expire eligible objects.

Un-skip TestLifecycleExpirationDateInThePast — it now exercises the
fixed path end-to-end.

* test(s3/lifecycle): integration coverage for multiple rules per bucket

A single bucket carries two independent Expiration rules with disjoint
prefix filters and different Days thresholds. Each rule must fire
only on its prefix; objects outside both prefixes must survive.

Pinned because Compile builds one CompiledAction per rule per kind
all sharing the same bucket index — a bug that lets one rule's
prefix or threshold leak into another (e.g. last-write-wins on a
shared map) would silently expire wrong objects.

Setup: rule A with prefix=logs/ Days=1, rule B with prefix=tmp/
Days=7. Three backdated objects: logs/access.log, tmp/scratch.bin,
data/keep.bin. After the worker runs, logs/ + tmp/ are gone;
data/ — outside both rule prefixes — survives.

* fix(s3/lifecycle): mark ScanAtDate actions active in Compile

Two layers were silently filtering ScanAtDate actions out of routing:
the walker's mode skip (fixed in e785f59d6) and Compile only marking
ModeEventDriven actions active. MatchPath / MatchOriginalWrite both
require IsActive() to emit a key, so a ScanAtDate action that's never
marked active never reaches a dispatch path even after the walker
falls through.

ScanAtDate's only dispatch path is the bootstrap walk's MatchPath
call — there's no bootstrap-completion rendezvous to wait on. Make
the active flag include ModeScanAtDate alongside the
EventDriven+BootstrapComplete combination.

ExpirationDate-based rules now actually fire end-to-end. The
TestLifecycleExpirationDateInThePast integration test exercises this.

* fix(s3/lifecycle): route date kinds via ComputeDueAt

ExpirationDate has MinTriggerAge=0, so router computed
dueTime = info.ModTime + 0 = info.ModTime. For a backdated entry
that mtime is BEFORE rule.ExpirationDate, so EvaluateAction's
now.Before(rule.ExpirationDate) check returned ActionNone and the
date rule never fired through the event-driven path.

ComputeDueAt already knows the per-kind shape — rule.ExpirationDate
for date kinds, ModTime+Days for the rest — so use it as the
single source of truth for dueTime in Route's main loop.

* test(s3/lifecycle): pin bootstrap walker date dispatch

The original TestWalk_DateActionsSkipped pinned the pre-e785f59d6
behavior that the regular walker skipped ExpirationDate. That
walker was rewired to fire date rules whose date has passed (the
SCAN_AT_DATE bootstrap was never wired); update the test to match.

Split into two: post-date entries dispatch, pre-date entries don't.

* test(s3/lifecycle): drop unused putExpiredDeleteMarkerLifecycle

The helper was never called — TestLifecycleExpiredDeleteMarkerCleanup
constructs a combined noncurrent + expired-marker rule inline, which
the helper doesn't cover. The blank-assignment workaround was just
hiding dead code; remove both.

* test(s3/lifecycle): tighten HeadObject termination check to typed not-found

Generic err != nil also passes on transport/auth/timeouts, letting
the test go green without proving the lifecycle action actually
fired. Switch the three Eventuallyf HeadObject predicates to
isS3NotFound, matching the pattern already in the multi-bucket and
expiration-date tests.

* test(s3/lifecycle): guard ListObjectVersions diagnostic against nil

When ListObjectVersions errors, listOut is nil and the diagnostic
log path panics on listOut.Versions before the real assertion fires.
Branch on (listErr != nil || listOut == nil) so the failure log is
robust whatever ListObjectVersions returned.

* refactor(s3/lifecycle): extract entryUsesMetadataOnlyDelete predicate

The metadata-only delete decision (entry.Attributes.TtlSec > 0) was
inlined in lifecycleDispatch with no direct test. Lift it into a
named predicate with the rationale comment moved onto the function
and pin the four edge cases: nil entry, nil attributes, TtlSec=0,
TtlSec>0, plus a defensive check that TtlSec<0 doesn't flip the
path on.
 2026-05-10 09:39:05 -07:00
Chris Lu c7b01c72b2 test(s3/lifecycle): integration coverage for versioning + filters (#9415) 
* test(s3/lifecycle): integration coverage for versioning + filters

First integration-test bundle building on the existing single-test
backdating harness. Each scenario follows the same shape: create
bucket, set lifecycle, PUT object, backdate mtime via filer
UpdateEntry, run the shell command for one shard sweep, assert
S3-side state.

Five new tests:

- TestLifecycleVersionedBucketCreatesDeleteMarker: Expiration on a
  versioned bucket must produce a delete marker (latest after worker
  runs is a marker) AND keep the original version directly addressable
  by versionId. ListObjectVersions confirms IsLatest=true on the
  marker.

- TestLifecycleNoncurrentVersionExpiration: NoncurrentVersionExpiration
  fires only on demoted versions. PUT v1, PUT v2 (so v1 → noncurrent),
  backdate v1, run worker. v1 must be gone, v2 still current.

- TestLifecycleExpiredDeleteMarkerCleanup: combined rule (noncurrent +
  expired-delete-marker) cleans up a sole-survivor marker. PUT v1,
  DELETE (creates marker), backdate both, run worker. Every version
  AND marker must be gone for the key.

- TestLifecycleDisabledRuleSkipsObject: rule with Status=Disabled
  must not produce dispatches even on a backdated match. Negative
  test for the engine's enabled-status gate.

- TestLifecycleTagFilter: rule with And{Prefix, Tag} only matches
  objects carrying the tag. Two backdated objects (one tagged, one
  not) — only the tagged one is removed.

Helpers extracted to keep each test focused: putVersioningEnabled,
putNoncurrentExpirationLifecycle, putExpiredDeleteMarkerLifecycle,
backdateVersionedMtime (ages a specific .versions/v_<id> entry),
runLifecycleShard (one-shot shell invocation with FATAL guard).

* test(s3/lifecycle): tighten noncurrent expiration diagnostics

Local run showed TestLifecycleNoncurrentVersionExpiration failing
with a bare 404 on HEAD(latest), not enough to tell whether v2 was
deleted, the bare-key pointer was removed, or a delete marker was
synthesized. Strengthen the test to:

- HEAD by versionId=v2 first, so we pin "v2 file still on disk"
  separately from "the latest pointer resolves to v2"
- on HEAD(latest) failure, log ListObjectVersions output (versions +
  markers, with IsLatest) so the next failure shows which side the
  bug is on rather than just NotFound

* test(s3/lifecycle): integration coverage for AbortIncompleteMultipartUpload

Exercises the lifecycleAbortMPU handler path that the prefix-based
expiration tests can't reach — routing keys off of .uploads/<id>/
directory events, not regular object events, and the dispatcher uses
a different RPC path (rm on the .uploads/<id>/ folder).

Setup: AbortIncompleteMultipartUpload rule with DaysAfterInitiation=1,
CreateMultipartUpload, UploadPart (so the directory carries the
right shape), backdate the .uploads/<uploadID>/ directory entry 30
days, run the worker. The upload must drop out of
ListMultipartUploads.

Helpers added: putAbortMPULifecycle, backdateUploadDir.

* test(s3/lifecycle): integration coverage for NewerNoncurrentVersions

NewerNoncurrentVersions=N keeps the N most recent noncurrent versions
and expires the rest. Distinct from per-version NoncurrentDays —
depends on per-version rank, not just per-version age — and routes
through routePointerTransition's "needs full expansion" path.

Setup: PUT v1, v2, v3, v4 on a versioned bucket (v4 current; v1-v3
noncurrent), backdate v1+v2+v3 so all satisfy the NoncurrentDays>=1
floor, run the worker. Expect v1+v2 expired (older noncurrent),
v3 (newest noncurrent within keep=1) and v4 (current) preserved.

Helper added: putNewerNoncurrentLifecycle.

* test(s3/lifecycle): integration coverage for suspended-versioning Expiration

Suspended versioning takes a distinct code path in lifecycleDispatch:
the VersioningSuspended branch first deletes the null version (via
deleteSpecificObjectVersion(versionId="null")) and then writes a
fresh delete marker on top. Other branches (Enabled → only writes a
marker; Off → straight rm) miss this two-step.

Setup: enable versioning, PUT v1 (real versionId), suspend
versioning, PUT again (creates the null version, demotes v1 to
noncurrent), set the Expiration rule, backdate the null at the
bare path. Expect: latest is now a fresh delete marker, the
"null" version is gone from ListObjectVersions, and v1 (noncurrent
under Enabled) still addressable directly — suspended Expiration
must only touch the null, not other versions.

Helper added: putVersioningSuspended.

* test(s3/lifecycle): integration coverage for multi-bucket sweep

A single shell-driven shard sweep must process every bucket carrying
lifecycle config, not just the first one alphabetically. Pinned
because the scheduler iterates the buckets directory and a regression
that returns early after the first match would silently disable
lifecycle for every later bucket.

Two buckets, each with their own prefix-expiration rule and a
backdated object. Both must be expired after the same sweep.

* test(s3/lifecycle): integration coverage for ObjectSizeGreaterThan filter

ObjectSizeGreaterThan is a strict > gate (filterAllows uses
ev.Size <= rule.FilterSizeGreaterThan to reject). Pinned at the
boundary: an object whose size equals the threshold must remain;
only an object strictly larger expires. Catches a > vs >= flip.

Two backdated objects on the same prefix, sizes 100 and 150 with
threshold=100 — boundary survives, larger expires.

* test(s3/lifecycle): scrub bucket lifecycle config + versions on cleanup

Tests share one weed mini server. Two pollution modes were producing
order-dependent failures:

- A later test's shard sweep would still load the prior test's
  lifecycle config (the worker reads every bucket's XML from filer
  state, and DeleteBucket alone doesn't drop lifecycle config
  cleanly on this codebase).
- Versioned-bucket tests left versions + delete markers behind that
  ListObjectsV2 can't see, so the existing best-effort empty-then-
  delete didn't actually empty those buckets.
- The AbortMPU test intentionally leaves an in-flight upload; without
  an explicit AbortMultipartUpload the bucket DELETE hits NotEmpty.

Cleanup now runs DeleteBucketLifecycle, ListObjectVersions →
DeleteObject(versionId), ListObjectsV2 → DeleteObject (catches what
ListObjectVersions missed), ListMultipartUploads → AbortMultipartUpload,
then DeleteBucket. Best-effort throughout so a half-torn-down bucket
doesn't fail the cleanup chain.

* test(s3/lifecycle): backdate both versions for NoncurrentDays clock

Per codex review: NoncurrentDays is clocked from the SUCCESSOR
version's mtime (when the displaced version became noncurrent), not
from the displaced version's own mtime. Backdating only v1 left the
clock (v2's mtime) at "now" and the rule never fired — the test was
wrong, not the production path.

Backdate v1=31d and v2=30d so v1 sits past the 1-day threshold
relative to v2, the noncurrent rule fires, and v2 stays current.

* test(s3/lifecycle): assert specific NotFound on multi-bucket deletion

Per codex review: TestLifecycleMultipleBucketsInOneSweep treated any
HeadObject error as "deleted", which lets a transport failure or
dead endpoint mask a real bug. Recognize NoSuchKey/NotFound/HTTP-404
specifically via a small isS3NotFound helper so the assertion
actually proves deletion happened, not just that the call broke.

* test(s3/lifecycle): gofmt size-filter test

* test(s3/lifecycle): integration coverage for Object Lock skip

Object Lock retention must override the lifecycle rule. The handler's
enforceObjectLockProtections check (s3api_internal_lifecycle.go:47)
returns an error when retention is active; the dispatcher then
classifies the outcome as SKIPPED_OBJECT_LOCK and the object stays.
No existing integration test reaches that outcome.

Setup: bucket created with ObjectLockEnabledForBucket=true, expiration
rule on prefix "lock/", two backdated objects under the same prefix —
one with GOVERNANCE retention until 1h from now, one without. After
the worker runs, the unlocked object expires (positive control); the
locked one survives.

Custom cleanup uses BypassGovernanceRetention so the test can drop
the locked version when the test finishes — otherwise the retention
window keeps the bucket from being deleted.

* test(s3/lifecycle): integration coverage for config update between sweeps

An operator changes the lifecycle rule between two shell-driven
sweeps. The second sweep must respect the NEW rule, not a cached
copy of the old one. Each runLifecycleShard invocation spawns a
fresh weed shell subprocess, so cached engine state from a previous
sweep doesn't persist — but a regression that caches rules across
PutBucketLifecycleConfiguration calls within the S3 server itself
would still surface here.

Sweep 1: rule prefix="first/", PUT + backdate firstKey, run worker
→ firstKey expires.

Update rule to prefix="second/", PUT + backdate secondKey AND a
new key under the OLD prefix ("first/post-update.txt"). Sweep 2
must expire only the second-prefix object; the post-update old-
prefix one must survive — config replacement, not merge.

* test(s3/lifecycle): integration coverage for ExpirationDate (past)

Rules with Expiration{Date: <past>} route through ScanAtDate in the
engine (decideMode's ActionKindExpirationDate case) — a separate
compile + dispatch branch from the EventDriven delay-group path the
Days-based tests exercise.

Past date + in-prefix object → must expire. Out-of-prefix object →
must remain. Object also backdated as defense-in-depth so the
assertion doesn't depend on whether the dispatcher consults
MinTriggerAge for date kinds.

* test(s3/lifecycle): integration coverage for bootstrap walk on existing objects

Production scenario: operator enables lifecycle on a bucket that
already holds objects from before the policy. The worker must
discover them via the bootstrap walk (BucketBootstrapper) — there
were no meta-log events to observe because the objects predate the
rule. Without the bootstrap path, only NEW writes would ever match.

Setup: PUT 5 objects (no lifecycle config yet) + 1 out-of-prefix
survivor, backdate all, THEN set the Expiration rule, run the
worker. Every in-prefix pre-existing object must be expired; the
out-of-prefix one must remain.

* test(s3/lifecycle): integration coverage for DeleteBucketLifecycle stops dispatching

Operator UX: after DeleteBucketLifecycle, the worker must observe the
removal on the next sweep and stop expiring objects under the now-gone
rule. A regression that caches old configs across
PutBucketLifecycleConfiguration → DeleteBucketLifecycle would keep
silently dropping objects.

Setup: positive control (rule active, backdated obj expires) →
DeleteBucketLifecycle → PUT + backdate a fresh object → second
sweep. The fresh object must remain.

* test(s3/lifecycle): integration coverage for empty bucket sweep no-op

A bucket carrying lifecycle config but no objects must produce a
successful sweep — no hangs, no errors, no dispatches. Pinned
because the bootstrap walker iterates bucket directories, and an
empty directory is a corner of that traversal that's easy to break
(slice-bounds bug on the first listing returning zero entries).

Asserts: worker logs "loaded lifecycle for" and "shards 0-15
complete", no FATAL output, bucket still exists after the sweep.

* test(s3/lifecycle): fix Object Lock backdate path + skip unwired ScanAtDate

ObjectLock: enabling Object Lock on a bucket implicitly enables
versioning, so PUT objects land at .versions/v_<id>, not at the bare
key. The test was calling backdateMtime (bare path) and failing in
the helper with "filer: no entry is found". Switch to
backdateVersionedMtime with the versionId returned by PutObject.

ExpirationDate: ScanAtDate dispatch path isn't wired to the run-shard
shell command yet — the bootstrap walker explicitly skips actions in
ModeScanAtDate (walker.go:141 says "SCAN_AT_DATE runs its own date-
triggered bootstrap" but no such bootstrap exists in the scheduler or
shell). Skip with a t.Skip + explanation so the test activates the
moment the date-triggered path lands.

* fix(s3/lifecycle): wire ExpirationDate dispatch through bootstrap walker

The walker explicitly skipped ModeScanAtDate actions on the comment
"SCAN_AT_DATE runs its own date-triggered bootstrap" — but no such
bootstrap exists in the scheduler or shell layer. The result: rules
with Expiration{Date: ...} compiled correctly, populated the
snapshot's dateActions map, and were never dispatched.
ExpirationDate is silently a no-op in production.

EvaluateAction already handles ActionKindExpirationDate correctly
(rejects when now.Before(rule.ExpirationDate), otherwise emits
ActionDeleteObject). The walker just needed to fall through instead
of skipping. Pre-date walks become no-ops via EvaluateAction's date
check; post-date walks expire eligible objects.

Un-skip TestLifecycleExpirationDateInThePast — it now exercises the
fixed path end-to-end.

* test(s3/lifecycle): integration coverage for multiple rules per bucket

A single bucket carries two independent Expiration rules with disjoint
prefix filters and different Days thresholds. Each rule must fire
only on its prefix; objects outside both prefixes must survive.

Pinned because Compile builds one CompiledAction per rule per kind
all sharing the same bucket index — a bug that lets one rule's
prefix or threshold leak into another (e.g. last-write-wins on a
shared map) would silently expire wrong objects.

Setup: rule A with prefix=logs/ Days=1, rule B with prefix=tmp/
Days=7. Three backdated objects: logs/access.log, tmp/scratch.bin,
data/keep.bin. After the worker runs, logs/ + tmp/ are gone;
data/ — outside both rule prefixes — survives.

* fix(s3/lifecycle): mark ScanAtDate actions active in Compile

Two layers were silently filtering ScanAtDate actions out of routing:
the walker's mode skip (fixed in e785f59d6) and Compile only marking
ModeEventDriven actions active. MatchPath / MatchOriginalWrite both
require IsActive() to emit a key, so a ScanAtDate action that's never
marked active never reaches a dispatch path even after the walker
falls through.

ScanAtDate's only dispatch path is the bootstrap walk's MatchPath
call — there's no bootstrap-completion rendezvous to wait on. Make
the active flag include ModeScanAtDate alongside the
EventDriven+BootstrapComplete combination.

ExpirationDate-based rules now actually fire end-to-end. The
TestLifecycleExpirationDateInThePast integration test exercises this.

* fix(s3/lifecycle): route date kinds via ComputeDueAt

ExpirationDate has MinTriggerAge=0, so router computed
dueTime = info.ModTime + 0 = info.ModTime. For a backdated entry
that mtime is BEFORE rule.ExpirationDate, so EvaluateAction's
now.Before(rule.ExpirationDate) check returned ActionNone and the
date rule never fired through the event-driven path.

ComputeDueAt already knows the per-kind shape — rule.ExpirationDate
for date kinds, ModTime+Days for the rest — so use it as the
single source of truth for dueTime in Route's main loop.

* test(s3/lifecycle): pin bootstrap walker date dispatch

The original TestWalk_DateActionsSkipped pinned the pre-e785f59d6
behavior that the regular walker skipped ExpirationDate. That
walker was rewired to fire date rules whose date has passed (the
SCAN_AT_DATE bootstrap was never wired); update the test to match.

Split into two: post-date entries dispatch, pre-date entries don't.

* test(s3/lifecycle): drop unused putExpiredDeleteMarkerLifecycle

The helper was never called — TestLifecycleExpiredDeleteMarkerCleanup
constructs a combined noncurrent + expired-marker rule inline, which
the helper doesn't cover. The blank-assignment workaround was just
hiding dead code; remove both.

* test(s3/lifecycle): tighten HeadObject termination check to typed not-found

Generic err != nil also passes on transport/auth/timeouts, letting
the test go green without proving the lifecycle action actually
fired. Switch the three Eventuallyf HeadObject predicates to
isS3NotFound, matching the pattern already in the multi-bucket and
expiration-date tests.

* test(s3/lifecycle): guard ListObjectVersions diagnostic against nil

When ListObjectVersions errors, listOut is nil and the diagnostic
log path panics on listOut.Versions before the real assertion fires.
Branch on (listErr != nil || listOut == nil) so the failure log is
robust whatever ListObjectVersions returned.
 2026-05-10 09:30:50 -07:00
Chris Lu 2840980c7d test(s3/lifecycle): final unit-test cleanup before integration suite (#9414) 
* test(s3/lifecycle): final unit-test cleanup before integration suite

Closes the residual coverage gaps in the lifecycle packages so the
next track (Layer 3 integration tests) starts from a clean baseline.
Big coverage lifts: lifecycletest 88.7→100.0, engine 81.7→95.1,
s3lifecycle 87.8→95.0, dispatcher 60.3→67.6, router 86.1→88.8,
bootstrap 90.7→92.6. Remaining sub-100% surfaces (reader.Run,
Pipeline.Run, scheduler.Run, multi-step bootstrap orchestration)
need a live filer and belong with the integration suite.

router/helpers_test.go (formerly #9409, now stale on master because
9410-9413 absorbed adjacent surface): direct tests for the pure
helpers Route exercises indirectly — successorModTimeFromContainer
(missing/empty/non-numeric/non-positive/positive round-trip),
logicalKeyFromVersionPath (extracts logical, rejects non-.versions
parent / root-level / no-slashes / bare container), isVersionsContainerKey
(table over container forms), isVersionFolderPath (table over child
forms), isDeleteMarkerEntry (only literal "true" matches),
extractTags (nil/empty, AmzObjectTagging-prefixed only, no-tag
returns nil), hasActiveEventDrivenAction (matches only active+
event-driven, scan-only rejected, unknown skipped). Plus engine
Snapshot accessors: BucketVersioned (compiled flag, unknown bucket
false), BucketActionKeys (full list, unknown nil), Action (unknown
nil), AllActions (every kind), SnapshotID (strictly monotonic).

s3lifecycle/final_cleanup_test.go: ActionKind.String default branch
(unspecified + future-unknown render "unspecified" rather than
empty); HashExtended direct from the lifecycle package (covers it
in this package's coverage report, not just the s3api one) including
nil/empty produces no bytes and identical content hashes the same.

bootstrap/has_prefix_test.go: thin wrapper around strings.HasPrefix
exported by the package; trivial but at 0% pre-fix.

lifecycletest/eventbuilder_old_entry_test.go: pins the OldEntry
fall-through path on Delete events for WithModTime / WithTtlSec /
WithVersionID / WithExtended / WithChunks (existing tests cover
Create events that hit NewEntry only). Adds WithBootstrapVersion
across all three event shapes. Defensive: every With* option is a
no-op on a degenerate event with neither entry populated.

* test(s3/lifecycle): address coderabbit nitpicks on final cleanup

- eventbuilder empty-event test now exercises WithBootstrapVersion
  too, with an honest claim about its scope: it targets the event
  itself (not an entry), so it sets BootstrapVersion regardless of
  whether NewEntry/OldEntry are populated. Renamed the test from
  AllAreNoOpsOnEmptyEvent to NoPanicOnEmptyEvent since the original
  name overstated the contract.
- HashExtended stability check uses a 3-key map with different
  literal orders so the helper's sort path actually does work; a
  single-key check can't catch an iteration-order regression.
- HasPrefix test refactored to table-driven so adding a new edge
  case is one row instead of two assertion lines.
 2026-05-09 22:32:49 -07:00
Chris Lu b740e22e63 test(s3/lifecycle): bundle dispatcher + engine edge-case coverage (#9413) 
* test(s3/lifecycle): bundle dispatcher + engine edge-case coverage

Two-package bundle covering uncovered branches in production code that
the existing happy-path tests don't reach. Dispatcher 58.1% → 60.2%
and engine 81.0% → 81.7% (engine lift modest because most branches
were already hit; the nil-rule defensive case is otherwise unreachable
from a Compile flow).

dispatcher (4 tests):
- FilerPersister.Load with nil Store errors with a "nil Store"
  message rather than panicking at the Read call.
- FilerPersister.Save with nil Store same.
- FilerPersister.Load with a non-NotFound transport error wraps the
  shard ID into the message AND keeps the underlying error
  recoverable via errors.Is.
- FilerPersister.Load with successful empty []byte returns an empty
  map, not a JSON-decode error — pinning that an existing-but-empty
  cursor file is treated as "no entries".
- Tick initializes the retries map on first call without panic so a
  freshly-constructed Dispatcher works.
- Tick with already-canceled ctx re-queues the popped Match, returns
  zero, and never invokes the LifecycleDelete client — the Match
  must not be lost across worker restart.

engine (4 tests):
- rulePredicateSensitive(nil) returns false rather than panicking on
  the FilterTags dereference. The non-nil paths run through Compile,
  but a defensive nil-rule arrival isn't reachable that way.
- rule with no FilterTags / empty FilterTags map returns false (the
  check is len(FilterTags) > 0, so empty must classify as
  non-sensitive — pinning catches a flipped >= comparison).
- rule with a populated FilterTags returns true.

* fix(s3/lifecycle): Tick must requeue every drained Match on shutdown

Per codex review on #9413: Tick called Schedule.Drain to pop ALL due
matches at once, then iterated. If ctx canceled mid-loop, only the
current Match was re-added — everything past that index was silently
lost across the worker restart. With N due matches, up to N-1 were
dropped.

Fix: on cancellation, re-add due[i:] (current + remaining) before
returning. Matches already dispatched (due[:i]) stay processed; the
schedule is left exactly as it would be if Drain had returned only
the dispatched prefix.

Strengthen the existing test to enqueue three due matches and assert
sched.Len()==3 after a pre-canceled Tick. Pre-fix the test would have
seen Len()==1 because only the first popped Match was re-added.
 2026-05-09 22:02:17 -07:00
Chris Lu ad77362be3 test(s3/lifecycle): bundle reader + scheduler helper coverage (#9412) 
* test(s3/lifecycle): bundle reader + scheduler helper coverage

Bundles direct tests for previously-uncovered helpers in two
packages. Bumps reader 73.2% → 79.2% and scheduler 71.6% → 73.6%.

Reader Event predicates (4):
- IsCreate: NewEntry-only event classifies as create
- IsDelete: OldEntry-only event classifies as delete
- both entries (update): neither IsCreate nor IsDelete (strict
  exclusivity so router routes updates through their own path)
- no entries (degenerate): neither (so a metadata-only filer event
  with no payload doesn't trigger spurious dispatches)

Reader LogStartup (4): exercises both shape branches (single-shard
ShardID vs ShardPredicate), the explicit-StartTsNs override path, and
the Cursor.MinTsNs fallback when StartTsNs=0. Side-effect-only
function; tests pin compile-time shape and visit each code path.

Scheduler pipelineFanout.InjectEvent (5):
- nil event silently absorbed (no follow-up panic in receiving
  pipeline)
- unknown shard returns nil (forward-compat for future shard-mapping
  gaps)
- known shard succeeds
- ctx cancellation propagates when underlying pipeline's buffer fills
- routes to the correct pipeline among multiple, with cross-pipeline
  isolation proven via per-pipeline buffer state

* test(s3/lifecycle): rename canceled to canceledCtx in fanout test

Per gemini review on #9412: a bare 'canceled' identifier reads like
a bool. Rename to canceledCtx so the type is obvious at the call site.
 2026-05-09 22:02:09 -07:00
Chris Lu 7996dc1d67 test(s3/lifecycle): bundle dispatcher pipeline helper coverage (#9411) 
* test(s3/lifecycle): bundle dispatcher pipeline helper coverage

Five untested helpers in dispatcher/pipeline.go and one accessor in
dispatcher.go that previously sat at 0% coverage. Bumps the dispatcher
package from 58.8% → 64.7%.

observeScheduleDepth: gauge tracks Schedule.Len; pinned for empty,
populated, and post-Drain states with a unique shard label so the
test doesn't bleed into other parallel tests sharing the global
counter.

ensureEventsChan: positive EventBuffer sizes the channel; zero and
negative fall back to defaultEventBuffer so a misconfigured operator
doesn't end up with a zero-cap channel that blocks every InjectEvent;
sync.Once contract holds under 32-goroutine concurrent invocation.

InjectEvent: delivers the same pointer to p.events (no defensive
copy); a canceled ctx propagates context.Canceled rather than
silently dropping the event; allocates the channel via
ensureEventsChan so a caller can inject before Run starts.

isCtxShutdown: nil → false; context.Canceled / DeadlineExceeded →
true; gRPC-wrapped Canceled / DeadlineExceeded codes → true (so
shutdown isn't misclassified as transport failure when filer/S3
RPCs unwrap as status); other errors (Unavailable, Internal, plain)
→ false.

cursorFileName: shard 0 / 1 / 9 / 10 / 15 all pad to two digits so
on-disk filenames stay sorted by shard ID — pinned so a refactor that
drops %02d doesn't break existing cursor files.

* test(s3/lifecycle): drop ScheduleDepthGauge label series at test end

Per gemini review on #9411: even with a unique shard label, the gauge
series persists in the global Prometheus registry across test runs in
the same process. Add a defer to DeleteLabelValues so the registry
stays clean.
 2026-05-09 22:02:01 -07:00
Chris Lu ca95d33092 test(s3/lifecycle): bundle dispatcher + engine accessor coverage (#9410) 
* test(s3/lifecycle): bundle dispatcher + engine accessor coverage

Two-package bundle covering pure helpers and snapshot read-side
accessors that the router and dispatcher reach for at runtime. None
were directly tested; regressions previously surfaced only as
downstream Tick / Match / Compile failures.

dispatcher (10 tests):
- keyOf: derives every retryKey field from the Match; equal Match
  values produce equal keys (so the second dispatch hits the first's
  retry counter); distinct VersionIDs and ActionKinds produce
  distinct keys (so a noisy version can't starve a healthy one,
  and two kinds on the same object don't share a budget).
- budget(): configured value when set; defaultRetryBudget when zero
  or negative — pins the >0 guard against a flipped comparison.
- backoff(): same pattern as budget for RetryBackoff.

engine snapshot accessors (8 tests):
- OriginalDelayGroups exposes the compiled per-delay groups; rules
  with multiple kinds at different cadences land in distinct entries;
  scan-only actions don't leak into delay groups so the dispatcher
  doesn't try to drive them event-driven.
- PredicateActions populated for tag-sensitive rules, empty for non-
  tag-sensitive ones (so MatchPredicateChange doesn't route
  irrelevant kinds).
- DateActions surfaces ExpirationDate verbatim for date kinds; empty
  for non-date rules.
- MarkActive on an unknown key is a no-op (durable bootstrap-complete
  write races a recompile that drops the rule; panic here would crash
  the worker).
- MarkActive flips a fresh-no-prior-state action from inactive to
  active.
- BucketActionKeys covers every kind RuleActionKinds reports.

* test(s3/lifecycle): strengthen snapshot accessor content assertions

Per gemini review on #9410: assertions previously only checked counts
and non-empty status. Verify the specific ActionKeys land where
expected so an indexing regression that produces the right number of
items with wrong kinds gets caught.

OriginalDelayGroups: each delay group's slice asserts.Contains the
specific (bucket, rule_hash, kind) ActionKey instead of just
NotEmpty.

PredicateActions: assert.Contains the expected key instead of just
NotEmpty.

BucketActionKeys: every key.Bucket must equal the test bucket (catches
cross-bucket leak), and ElementsMatch pins kinds against
RuleActionKinds.
 2026-05-09 22:01:54 -07:00
Chris Lu 0955d1aa08 test(s3/lifecycle): direct prefixMatches + filterAllows coverage (#9408) 
Both helpers were exercised indirectly through MatchOriginalWrite /
MatchPath; pinning them directly catches a regression at the helper
level so a Match-test failure isn't the first signal of a broken
filter.

prefixMatches: empty prefix fast path; exact-prefix match; non-match
rejection; path shorter than prefix.

filterAllows: no-filter accepts any event; FilterSizeGreaterThan is
strictly > (boundary value rejected); FilterSizeLessThan is strictly
<; zero-size thresholds mean "not set" (must let any size through —
a regression treating 0 as a real threshold would reject everything);
required tag present accepts; missing key, empty tags map, wrong
value, and missing-among-multiple all reject; size + tag filters are
AND'd so either failing rejects.
 2026-05-09 20:47:35 -07:00
Chris Lu edbe7ab140 test(s3/lifecycle): meta-log Event builder + monotonic clock fixture (#9406) 
* test(s3/lifecycle): meta-log Event builder + monotonic clock fixture

Several test files build *reader.Event ad-hoc; consolidate the common
shape into the lifecycletest package as task #12 spec calls out
("fixture meta-log generator"). New tests using the builder don't
have to thread Mtime / ShardID / leaf-name semantics by hand, and
existing helpers can migrate over time without churning this PR.

NewCreate / NewDelete / NewUpdate cover the three event shapes;
WithSize / WithModTime / WithTtlSec / WithVersionID / WithExtended /
WithChunks / WithBootstrapVersion / WithShardID compose deterministic
overrides. ShardID defaults to s3lifecycle.ShardID(bucket, key) so
events route through the same shard the production reader would.

MetaLogClock issues monotonic timestamps with a configurable step
(default 1s); concurrent-safe so fan-out fixtures don't have to lock
externally.

15 unit tests pin every option, the IsCreate/IsDelete/IsUpdate
discriminators, leaf-name extraction for nested keys, ShardID
derivation, option-ordering semantics, the concurrent clock contract
under -race, and a Peek-doesn't-advance check.

* test(s3/lifecycle): address review comments on event builder

- leafOf strips trailing slashes before splitting so directory-key
  fixtures (e.g. "folder/") get the slashless leaf "folder" — pre-fix
  it returned "" which would break router tests for directory markers.
- NewUpdate now seeds OldEntry.Attributes.Mtime with the event ts
  (matching NewDelete), so a downstream router that compares mtimes
  doesn't see a synthetic 1970 epoch on the pre-update state.
- New WithOldSize / WithOldChunks / WithOldModTime options let Update
  events configure pre-update state independently. The unprefixed
  variants still target NewEntry on Update events; the With Old*
  options are no-ops on Create (no OldEntry to mutate) and never bleed
  into NewEntry.

5 new tests pin: directory-key + multi-slash leaf extraction; OldEntry
mtime default on Update; the WithOld* targeting + Create-event
no-bleed contract.
 2026-05-09 20:47:27 -07:00
Chris Lu 9d20e71883 test(s3/lifecycle): cover worker handler lookupBucketsPath (#9407) 
Three branches: gRPC error from GetFilerConfiguration must propagate
(else Execute would proceed to dial S3 with an empty buckets path
and never dispatch); a non-empty DirBuckets is honored verbatim so
operators with a non-default layout aren't force-routed to /buckets;
an empty DirBuckets falls back to the documented "/buckets" default
rather than returning empty (which would route to root). stubFilerConfigClient
embeds filer_pb.SeaweedFilerClient so methods other than the one
under test panic if called — keeps the surface narrow.
 2026-05-09 20:41:09 -07:00
Chris Lu 1aa55f5bf9 test(s3/lifecycle): direct decideMode + RuleMode.String coverage (#9405) 
Compile tests cover decideMode indirectly; these direct tests pin
every branch so a regression in the classifier itself can't slip
behind a more elaborate Compile failure.

Pinned: nil rule and Disabled status both → Disabled; ExpirationDate
→ ScanAtDate without consulting retention; metaLogRetention=0 means
unbounded so any horizon → EventDriven; horizon within retention →
EventDriven; horizon exceeding retention → ScanOnly; bootstrapLookback
adds to horizon (not retention) so a near-threshold case is still
gated; zero horizon (rule field unset) skips the gate. RuleMode.String
must render the documented names for every variant; an unknown value
collapses to "unspecified" rather than empty or panic.
 2026-05-09 20:35:34 -07:00
Chris Lu 619cb39827 test(s3/lifecycle): pin Schedule edge cases beyond happy path (Phase 15 slice) (#9403) 
* test(s3/lifecycle): pin Schedule edge cases beyond happy path

Pre-existing schedule_test covered the happy path (ordered Drain,
empty schedule, duplicates, boundary-inclusive). Five new tests pin
edge cases the dispatcher relies on:

- Drain at a time before any DueTime returns nil and leaves the heap
  intact, so the dispatcher can't accidentally consume future-due
  matches.
- NextDue after partial Drain points to the next earliest, catching
  a Drain that forgets the heap invariant.
- Add after Drain bubbles a fresh earlier DueTime to the front, so
  late-arriving high-priority matches don't sit behind older ones.
- Drain returns Matches in ascending DueTime order regardless of
  insert order — explicit pinning of the documented contract.
- Concurrent Add+Drain across 64 goroutines under -race.

* test(s3/lifecycle): actually exercise Drain in AddAfterDrain test

Per coderabbit review on #9403: the test name promised "after Drain"
but the previous body only Add'd both items without ever calling
Drain in between. Insert a real Drain (popping "drain_me") before
the second Add, so the heap-invariant-across-Drain-then-Add path is
actually pinned. Bumps the after-Drain Match's DueTime out of the
way so the Drain in step 3 returns it deterministically.
 2026-05-09 20:35:22 -07:00
Chris Lu 435ef7f94f test(s3/lifecycle): pin toProtoActionKind + toProtoIdentity converters (#9404) 
test(s3/lifecycle): pin toProtoActionKind + toProtoIdentity

The two converters are the worker-side wire to LifecycleDelete; a
miss in toProtoActionKind sends ACTION_KIND_UNSPECIFIED that the
server rejects FATAL, and a wrong toProtoIdentity flips the CAS
witness so every dispatch comes back NOOP_RESOLVED with STALE_IDENTITY
even though the entry hasn't changed.

10 tests pin: every listed s3lifecycle.ActionKind maps to its proto
counterpart (table-driven, one subtest per kind); ActionKindUnspecified
and a future unknown kind both collapse to ACTION_KIND_UNSPECIFIED
(forward compat); nil EntryIdentity stays nil (preserves the no-CAS
sentinel); a populated identity copies every field; a zero-valued
identity still produces a non-nil output so the server treats it as
a real CAS witness rather than no-CAS.
 2026-05-09 20:35:04 -07:00
Chris Lu 1350e681c9 test(s3/lifecycle): pin Pipeline.Run dependency + shard validation (Phase 15 slice) (#9402) 
* test(s3/lifecycle): pin Pipeline.Run dependency + shard validation

Pre-existing TestPipelineRunRequiresDependencies only checked that an
empty Pipeline errors; it didn't pin which specific dependency must be
present. A refactor that makes one nilable accidentally would slip
through.

8 new tests pin every validation branch in Pipeline.Run: missing
Engine / Persister / Client / FilerClient each error with "missing
required dependency"; missing BucketsPath errors with its own
distinct message so operators can spot the missing wiring; ShardID =
-1 / ShardCount errors with a range message (covers the half-open
[0, ShardCount) boundary so a < to <= refactor can't introduce a
one-past-the-end shard); and a multi-shard config with one
out-of-range entry refuses the whole run rather than silently
disabling the rest.

* test(s3/lifecycle): refactor Pipeline.Run validation tests as table-driven

Per gemini review on #9402: collapse the eight per-branch tests into
TestPipelineRunValidation with a slice of (name, mutate, wantErr)
cases. Same coverage, ~30 fewer lines, idiomatic Go pattern that
makes adding a new validation case trivial.
 2026-05-09 20:34:51 -07:00
Chris Lu cb6e498e0b test(s3/lifecycle): pin Descriptor structural invariants (#9401) 
* test(s3/lifecycle): pin Descriptor structural invariants

Pre-existing handler tests covered Capability and Detect; Descriptor
was previously untested. A drift between the form fields it advertises
and the defaults config.go reads silently breaks the admin UI in two
ways: the form renders blank (admin can't tune) or the worker clamps
to a hardcoded fallback ignoring the admin's edits. The new tests
catch both directions.

Pinned: jobType / DisplayName / Description / DescriptorVersion;
AdminConfigForm exposes a workers field whose default matches
defaultWorkers; WorkerConfigForm has a default and a field for every
cadence knob ParseConfig reads (dispatch_tick / checkpoint_tick /
refresh_interval / bootstrap_interval / max_runtime); AdminRuntime-
Defaults hits a daily cadence with bounded detection timeout and
single job per detection.

* test(s3/lifecycle): tighten Descriptor invariant assertions

Per gemini review on #9401: pin DetectionTimeoutSeconds to its exact
value (60) instead of ">0" so an accidental tweak is caught, and
assert WorkerConfigForm fields are INT64 (matching ParseConfig's
readInt64) so a STRING-type drift can't silently make the worker
ignore admin edits.
 2026-05-09 20:34:17 -07:00
Chris Lu 6f9668c20b test(s3/lifecycle): pin lifecycleDispatch validation early-returns (#9400) 
Three pure-validation paths in lifecycleDispatch return BLOCKED before
any filer call; without coverage a refactor could let them fall
through to a real delete. ABORT_MPU at dispatch time is a defensive
catch (the route bypass should never happen, but if it does the
fallthrough must not become a default-case rm). Unknown ActionKind
gets the same treatment for forward-compatibility with new proto
values. Empty version_id on noncurrent / EXPIRED_DELETE_MARKER kinds
must be rejected before deleteSpecificObjectVersion is called, so a
malformed event can't silently delete the latest pointer.
 2026-05-09 20:11:08 -07:00
Chris Lu af2a359e45 feat(s3/lifecycle): metadata_only_total Prometheus counter (#9399) 
Operator-visible signal for the metadata-only delete path landed in
PR 9390. Increment seaweedfs_s3_lifecycle_metadata_only_total{bucket,
rule_hash} after each successful unversioned or noncurrent / expired-
marker delete that took the skip-chunk path. Suspended-versioning
null delete is intentionally not counted: that path's nil err can
mean "deleted" or "NotFound", so a count there would over-report.
rule_hash is hex-encoded for label safety; nil bytes collapse to
"". DeleteBucketMetrics tears the new series down alongside the
existing lifecycle counters when a bucket is removed.
 2026-05-09 20:02:26 -07:00
Chris Lu c0cf1417f1 test(s3/lifecycle): cover worker handler Execute validation paths (#9398) 
7 tests pin the Execute early-return surface that runs without a
filer or S3 dial: nil request / nil Job / nil sender all error;
foreign JobType errors with the offending name in the message; no
S3 endpoints in cluster context errors (Execute is stricter than
Detect — the admin shouldn't have routed the job there); missing
filer_grpc_address parameter errors (proposal must have been tampered
with or dropped); empty JobType is accepted as broadcast routing and
flows through to the next validation step. The dial path itself is
intentionally not covered here — those tests would need an in-process
gRPC server and belong with the integration suite.
 2026-05-09 19:51:31 -07:00
Chris Lu 284d37c3b6 test(s3/lifecycle): cover InMemoryPersister deep-copy contract (#9397) 
* test(s3/lifecycle): cover InMemoryPersister deep-copy contract

8 tests pin the persister contract other lifecycle tests rely on for
cursor checkpointing: Load on an unknown shard returns an empty map
(not an error); Save then Load roundtrips; Save copies the input so
caller-side mutation doesn't bleed into stored state; Load returns a
copy so caller-side mutation of the snapshot doesn't bleed back; Save
replaces (not merges) prior state so stale resume points don't survive
restart; different shards stay isolated; saving an empty map clears
state; concurrent Save+Load is race-free under -race. A regression on
any of these silently corrupts downstream tests.

* test(s3/lifecycle): assert.NotContains for InMemoryPersister key absence

assert.Empty on a map[K]V index returns true when the value is the
zero value, which would mask a key that leaked through with int64(0).
Use assert.NotContains so the assertion fails on key presence
regardless of the stored value.
 2026-05-09 19:47:16 -07:00
Chris Lu 62e04623ce test(s3/lifecycle): cover worker handler Detect + helpers (#9396) 
* test(s3/lifecycle): cover worker handler Detect + helpers

13 tests pin the worker-handler surface that runs without a live filer
or S3 server. Pure helpers: clusterS3Endpoints (nil context, empty
list, filter empty entries while preserving order, all-valid
passthrough); readString (missing key, nil ConfigValue, wrong kind
falls back, string returned). Capability advertises jobType with
single-job concurrency caps. Detect: nil request / nil sender / wrong
JobType all error; no S3 endpoints emits a 'skipped' activity and
completes with success; no filer addresses behaves the same; the
happy path proposes one job parameterized with the first filer
address; empty JobType is accepted (broadcast detect); a
SendProposals failure propagates without firing complete.

* test(s3/lifecycle): cover SendComplete error propagation in worker Detect

The recordingSender already supported forcing an err on SendComplete
via errOn, but no case exercised it. A SendComplete failure must
propagate so the admin learns the completion signal never landed;
proposals went out before the failure so they remain recorded.
 2026-05-09 19:46:57 -07:00
Chris Lu 551e700e64 test(s3/lifecycle): cover scheduler configload surface (#9395) 
* test(s3/lifecycle): cover scheduler configload surface

LoadCompileInputs is the bridge between the filer's bucket directory
and the engine snapshot the scheduler compiles every refresh; a missed
or misclassified bucket silently disables lifecycle for that prefix
until the next refresh. Tests pin: empty bucket dir, files at the
bucket level skipped, buckets without the lifecycle XML extended key
skipped, empty-bytes XML skipped, valid XML becomes a CompileInput,
versioning attr propagates to CompileInput.Versioned, malformed XML
surfaces as a ParseError without aborting the walk, and pagination
across the 1024 page boundary preserves bucket order.

Also covers the IsBucketVersioned (case + whitespace tolerance,
rejection of garbage values) and AllActivePriorStates (one entry per
(bucket, ruleHash, actionKind), bucket-keyed isolation) helpers.

* test(s3/lifecycle): tighten configload pagination boundary check

Switch the bucket-count check to require.Len so a regression that
returns the wrong number of buckets fails fast before the boundary
asserts panic on out-of-range index. Add explicit assertions on the
last entry of page 1 (b01023) and the first entry of page 2 (b01024)
so a pagination-loop bug that drops or duplicates the seam is caught
directly rather than only via the count check.
 2026-05-09 19:46:40 -07:00
Chris Lu 6021a88606 test(s3/lifecycle): cover CompareVersionIds tiebreak surface (#9394) 
* test(s3/lifecycle): cover CompareVersionIds tiebreak surface

13 tests pin every documented branch of the version-id comparator and
its helpers (isNewFormatVersionId, getVersionTimestamp): equality and
short-circuit paths, null sorting last, both-new-format with smaller-
=-newer ordering, both-old-format with larger-=-newer ordering, mixed-
format compared by parsed timestamp, mixed-format with synthesized
equal timestamps, length / null / non-hex rejection, the strictly-
greater-than threshold boundary at 0x4000000000000000, and the
inverted-value invariant the comparator relies on. Getting any axis
wrong silently inverts retention rankings, which would resurrect
deleted versions or evict live ones.

* test(s3/lifecycle): use plain assert.Equal in mixed-format compare test

The previous local require := assert.New(t) shadowed testify's require
package while actually returning an assert.Assertions (continue-on-fail
semantics, not fail-fast). Use plain assert.Equal(t, ...) calls so the
behavior matches the variable's name and the rest of the file.
 2026-05-09 19:03:31 -07:00
Chris Lu 7781eef429 test(s3/lifecycle): cover dispatcher filerSiblingLister surface (Phase 14 slice) (#9392) 
* test(s3/lifecycle): cover dispatcher's filerSiblingLister surface

Tests pin the four routing-critical filer interactions on the
filerSiblingLister: Survivors (count cap, LoneEntry semantics,
null-version detection across regular files and directory-key
markers, error propagation in both list and lookup paths),
ListVersions (NotFound collapse, dir/missing-id filtering,
1024-page boundary, error propagation), LookupNullVersion (regular
file, explicit-null flag, directory-key marker accept, plain-dir
reject, NotFound collapse, error propagation), and LookupVersion
(empty version-id no-op, v_ prefix, NotFound collapse, error
propagation).

The fake SeaweedFilerClient mirrors the real filer's NotFound
shape — gRPC succeeds at stream creation and the first Recv()
surfaces filer_pb.ErrNotFound — which is what the lister's
errors.Is check depends on. NewFullPath strips a trailing slash
before splitting so directory-key markers are stored under their
slashless Name.

* test(s3/lifecycle): gofmt sibling_lister_test.go

Trailing comment alignment.
 2026-05-09 18:55:50 -07:00
Chris Lu 8cf42a5abb test(s3/lifecycle): assert per-goroutine errors in fake-server concurrent test (#9393) 
test(s3/lifecycle): assert per-goroutine errors in concurrent fake test

The previous TestFake_ConcurrentCallsSerializeWithoutDeadlock dropped
the err return from each LifecycleDelete call, so a regression in the
concurrent path could pass the length-only assertion. Capture each
err on a buffered channel and require.NoError after wg.Wait().
 2026-05-09 18:54:15 -07:00
Chris Lu ddfb219ec3 test(s3/lifecycle): fake LifecycleDelete server (Phase 12 slice) (#9391) 
* test(s3/lifecycle): fake LifecycleDelete server for component tests

A reusable double for SeaweedS3LifecycleInternalServer with per-key
FIFO outcome queues, a fallback Default, and recorded request capture.
Tests of the worker pipeline that need to hit the proto boundary can
queue up DONE/NOOP/RETRY/FATAL/SKIPPED_OBJECT_LOCK responses per
(bucket, objectPath, versionId) and assert dispatch order against
Recorded(). SetError flips the server into transport-failure mode
without polluting the request log.

* test(s3/lifecycle): use struct map key for FakeLifecycleServer queues

Bucket / object path / version-id are user-supplied strings that can
contain "/" or "@", which would collide if the queue map were keyed by
"<bucket>/<object>@<version>". Switch to a struct key so the
components stay separate.

* test(s3/lifecycle): deep-copy recorded LifecycleDelete requests

Tests that mutate a Recorded() entry — or a request pointer they
already passed in — were able to corrupt the fake's bookkeeping
because the slice carried shared pointers. Clone with proto.Clone at
both record and read time so the fake holds an independent snapshot
of every arriving request and hands callers an independent snapshot
back. Tightened TestFake_VersionIDPartOfKey error checks while there.
 2026-05-09 18:38:52 -07:00
Chris Lu bb0c7c779f feat(s3/lifecycle): metadata-only delete when entry TtlSec > 0 (Phase 2b) (#9390) 
* refactor(s3): thread metadataOnly into delete helpers

Add a metadataOnly bool to deleteUnversionedObjectWithClient and
deleteSpecificObjectVersion. When true the helper sends IsDeleteData=
false to the filer's DeleteEntry RPC so per-chunk DeleteFile RPCs are
skipped — the volume server reclaims chunks on its own at TTL drop.
Non-lifecycle callers (DELETE handlers, batch delete) pass false to
preserve today's eager-chunk-delete behavior; only the lifecycle
handler in the next commit will pass true.

* feat(s3/lifecycle): metadata-only delete when entry TtlSec > 0

Per-write TTL stamping (PR 9377) sets Attributes.TtlSec on every
lifecycle-fitting entry. When the live entry the LifecycleDelete
handler fetched carries TtlSec > 0 the volume server is guaranteed
to reclaim chunks at TTL drop, so the filer can skip per-chunk
DeleteFile RPCs and just remove the entry record. lifecycleDispatch
now computes metadataOnly from the live entry and threads it through
the unversioned, suspended-null, and noncurrent/expired-marker delete
paths. createDeleteMarker is unaffected — it creates a marker, never
deletes chunks.
 2026-05-09 18:38:38 -07:00
Chris Lu 255e9cd0f7 test(s3/lifecycle): cover reader cursor + Run validation contracts (#9389) 
* test(s3/lifecycle): cover reader cursor + Run validation contracts

Layer 2 tests pinning four reader-package contracts the dispatcher
pipeline depends on: MinTsNs anchors at frozen positions, Snapshot
returns a deep copy in both directions, Restore replaces (not merges),
and Run validates ShardID/Events/BucketsPath before subscribing.

* test(s3/lifecycle): tighten cursor composition assertions

Snapshot deep-copy: also assert cursor doesn't see keys added to the
returned map. Restore replace: freeze before second Restore and assert
IsFrozen returns false after, pinning the contract that Restore wipes
frozen state alongside the value map. Run validation: bound the call
with a 5s context timeout so a regression that lets Run reach the nil
client surfaces as a failure instead of a hang.
 2026-05-09 14:32:11 -07:00
Chris Lu aa280443e7 test(s3/lifecycle): Layer 2 multi-shard composition for the dispatcher (#9387) 
* test(s3/lifecycle): Layer 2 multi-shard composition for the dispatcher

The existing dispatcher unit tests cover individual outcomes
(DONE / RETRY_LATER / BLOCKED / etc.) on a single shard, and
pipeline_test.go has only one end-to-end happy-path assertion.
Multi-shard composition — the contract Pipeline.Run wires up at
runtime — was untested at the component level.

Add four Layer 2 tests in dispatcher/multi_shard_test.go:

  Two events for two shards land in different schedules, dispatch
  independently, and each cursor advances only for its own event
  (no cross-contamination on the action-key map).

  A poison event on shard 0 returns BLOCKED and freezes shard 0's
  cursor; shard 1's normal event continues to dispatch and its
  cursor advances. Per-shard isolation contract.

  Save/Load round-trips a per-shard cursor snapshot through the
  Persister: a fresh dispatcher restores the same TsNs map. Pins
  the contract Pipeline.Run drives on the checkpoint ticker.

  RETRY_LATER respects RetryBackoff against the wall clock — a
  Tick within the backoff window doesn't re-dispatch; a Tick past
  the new DueTime does. Guards against premature retries from
  refresh ticks landing inside the backoff.

Pipeline.Run itself can't run here (it builds a real reader.Reader),
so the tests share the same fakeClient pattern dispatcher_test.go
uses and drive Tick directly.

* test(s3/lifecycle): drop unused snapshot helper and addAndTick parameter
 2026-05-09 14:12:21 -07:00
Chris Lu 1854101125 feat(s3/lifecycle): bootstrap re-walk cadence + operator hooks (Phase 8) (#9386) 
* feat(s3/lifecycle): bootstrap re-walk cadence + operator hooks (Phase 8)

scan_only actions only fire from the bootstrap walk: the engine
classifies a rule as scan_only when its retention horizon exceeds the
meta-log retention, so event-driven routing can't be trusted. Today
each bucket walks once per process, so a long-running worker never
revisits — scan_only retention only catches up when the worker
restarts.

Replace BucketBootstrapper.known (set) with BucketBootstrapper.lastWalk
(name -> completion time). KickOffNew now re-walks a bucket whose
last walk completed more than BootstrapInterval ago. Zero interval
preserves the legacy walk-once-per-process behavior so existing
deployments don't change cadence by default. walkBucket re-stamps
on success and clears the stamp on failure (via MarkDirty), so the
next KickOffNew picks failed walks back up.

Add MarkDirty / MarkAllDirty operator hooks for forced re-walks, and
a Now func() for testable time travel.

weed shell run-shard grows --bootstrap-interval (cadence knob) and
--force-bootstrap (drop in-memory state at startup so every bucket
walks again immediately, useful when a config change should take
effect without a restart).

Tests: cadence respected (skip inside interval, re-walk past it);
zero interval keeps once-per-process; MarkDirty forces re-walk
under a 24h interval; MarkAllDirty resets every record. The
fakeClock helper guards the test clock with a mutex so race-detector
runs are clean.

* fix(s3/lifecycle): split walk state, thread BootstrapInterval through worker, drop dead flag

Three issues with the Phase 8 cadence work as it landed:

1. lastWalk did double duty as both completed-walk timestamp and
   in-flight debounce. A walk that took longer than BootstrapInterval
   would have a fresh KickOffNew start a duplicate goroutine on the
   next refresh tick because the stamp from KickOffNew looked stale
   against the interval. Split into lastCompleted (set on success)
   and inFlight (set on dispatch, cleared after the walk goroutine
   returns success or failure). KickOffNew skips inFlight buckets
   regardless of cadence.

2. The cadence knob existed on `weed shell` but not on the production
   path: scheduler.Scheduler constructed BucketBootstrapper without
   BootstrapInterval, and weed/worker/tasks/s3_lifecycle/Config had
   no field for it. Add Scheduler.BootstrapInterval, parse
   `bootstrap_interval_minutes` in ParseConfig (zero = legacy walk-
   once-per-process; negative clamps to zero), and forward it from
   the handler. Tests cover default, override, clamp, and explicit-zero.

3. --force-bootstrap was a no-op: BucketBootstrapper is freshly
   allocated at command start, so MarkAllDirty on empty state does
   nothing, and the flag couldn't influence an already-running
   process anyway. Remove it; a real runtime trigger (SIGHUP, control
   RPC) is a separate change.

In-flight regression: a blockingInjector pins the first walk in
progress while the test advances the clock past the interval. The
second KickOffNew is a no-op (inFlight check). After release, the
post-completion KickOffNew within the interval is also a no-op.

* test(s3/lifecycle): wait for lastCompleted stamp before advancing fake clock

The cadence test polled listedN to know "the walk happened" — but
that fires once both list passes are issued, while the success-stamp
lands later, after walkBucketDir returns. A clock.Advance(30m)
between those two events would record the stamp at clock+30m
instead of T0; the next assertion would then see now.Sub(last) < 1h
and skip the expected re-walk. Tight in practice but exposed under
-race / load.

Add a waitForCompleted helper that polls b.lastCompleted directly,
and use it before each clock advance in both the cadence and zero-
interval tests.

* fix(s3/lifecycle): expose bootstrap interval in worker UI; honor MarkDirty during walks

Two follow-ups on Phase 8.

The worker config descriptor had no bootstrap_interval_minutes field,
so the production operator UI couldn't enable the cadence — only the
internal ParseConfig + Scheduler wiring knew about it. Add the field
to the cadence section (MinValue=0 since 0 is the legacy default) and
include the default in DefaultValues so existing deployments see the
knob with the right preset.

MarkDirty / MarkAllDirty silently lost their effect when a walk was
in flight: the methods cleared lastCompleted, but the walk's success
path then wrote a fresh timestamp, hiding the operator's invalidation.
Track a pendingDirty set; the walk goroutine consumes the flag on
exit and skips the success stamp, so the next KickOffNew picks the
bucket up immediately.

Regression: pin a walk in progress with a blockingInjector, MarkDirty
the bucket, release the walk, and assert lastCompleted stayed empty
plus the next KickOffNew triggers a new walk inside the
BootstrapInterval window.

* refactor(s3/lifecycle): drop unused MarkDirty / MarkAllDirty + pendingDirty

These methods were the operator-hook half of Phase 8, but the only
caller (--force-bootstrap on the shell command) was removed when it
turned out to be a no-op against a freshly-allocated bootstrapper.
Nothing in production calls them anymore.

Strip the dead surface: MarkDirty, MarkAllDirty, the pendingDirty
set, the dirty-suppression branch in walkBucket, and the three tests
that only exercised those methods. BootstrapInterval-driven
re-bootstrap is the live mechanism. A real runtime trigger (SIGHUP,
control RPC) is a separate change with a real call site.
 2026-05-09 13:42:31 -07:00