mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-19 05:36:58 +00:00
2212cc8a5f27b1ee7712429cc3323d94cedff735
13814
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Parviz MiriyevandGitHub
|
2212cc8a5f |
shell: expose retention flags on mq.topic.configure (#9416)
* shell: expose retention flags on mq.topic.configure
The ConfigureTopicRequest proto already carries a TopicRetention message
({retention_seconds, enabled}), and GetTopicConfigurationResponse / the
admin UI both surface it — but the `mq.topic.configure` shell command
sent Retention: nil unconditionally, leaving CLI and IaC users with no
way to set retention without going through the admin UI. Add three
flags so the shell matches the existing surface:
-retention <duration> Go duration string (e.g. 168h, 30m)
-retentionSeconds <int> raw seconds (mutually exclusive with -retention)
-retentionEnabled bool toggle for retention enforcement
Behavior:
- If none of the retention flags are set, the request omits Retention
(`nil`) — preserving the prior "leave server-side state alone"
semantics for callers that only care about partition count.
- Setting any retention flag populates TopicRetention with both fields
so existing operators keep working when only one is provided.
- -retention and -retentionSeconds together is a hard error (ambiguous);
negative values are rejected.
The new behavior is detected via flag.FlagSet.Visit so default values
of 0 / false are distinguishable from "not provided".
Help text updated with examples for both setting a TTL and disabling
retention on an existing topic. No proto / server changes needed; this
is a CLI-only patch.
* broker, shell: address review for mq.topic.configure retention
Three follow-up fixes for the review comments on the original commit:
1. Server-side: detect retention changes in the early-return path.
Previously ConfigureTopic returned without persisting when
partitionCount + schema were unchanged, even if the request supplied
a different Retention. Now the early-return branch checks both
schema and retention, persists either or both when they differ, and
logs which fields actually changed.
2. Server-side: preserve existing retention when request.Retention is
nil in the fresh-allocation path. Capture the prior resp.Retention
before overwriting `resp = &mq_pb.ConfigureTopicResponse{}`, and
only overwrite the new resp.Retention with `request.Retention` when
the request actually supplies one. Otherwise carry the previous
retention forward, so partition-count changes (or any path that
bypasses the early-return branch) don't accidentally clear retention.
3. CLI: switch the `-retention` / `-retentionSeconds` mutual-exclusion
check from value-based (`!= 0`) to flag.FlagSet.Visit-based, so an
explicit `-retention=0 -retentionSeconds=N` is also rejected.
4. CLI: when any retention flag is set, fetch the current
GetTopicConfiguration and fill in the fields the user didn't
explicitly provide. This means `-retentionEnabled` alone no longer
zeros the existing duration, and `-retention 24h` alone preserves
the existing enabled flag. When the topic doesn't exist yet, the
GetTopicConfiguration error is treated as "no current state" and
we proceed with just the user-supplied values.
Build / vet / gofmt / `go test ./weed/shell/... ./weed/mq/broker/...`
clean.
|
||
 Lars LehtonenandGitHub
|
89608e5499 | chore(weed/util/log_buffer): remove unused functions (#9444) | ||
 Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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.
|
||
|
Chris LuandGitHub
|
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.
|
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
  |
bd687a2d7a |
build(deps): bump google.golang.org/api from 0.274.0 to 0.278.0 (#9451)
Bumps [google.golang.org/api](https://github.com/googleapis/google-api-go-client) from 0.274.0 to 0.278.0. - [Release notes](https://github.com/googleapis/google-api-go-client/releases) - [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md) - [Commits](https://github.com/googleapis/google-api-go-client/compare/v0.274.0...v0.278.0) --- updated-dependencies: - dependency-name: google.golang.org/api dependency-version: 0.278.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
fe0d533b9d |
build(deps): bump github.com/klauspost/compress from 1.18.5 to 1.18.6 (#9452)
Bumps [github.com/klauspost/compress](https://github.com/klauspost/compress) from 1.18.5 to 1.18.6. - [Release notes](https://github.com/klauspost/compress/releases) - [Commits](https://github.com/klauspost/compress/compare/v1.18.5...v1.18.6) --- updated-dependencies: - dependency-name: github.com/klauspost/compress dependency-version: 1.18.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
91957d6919 |
build(deps): bump cloud.google.com/go/kms from 1.30.0 to 1.31.0 (#9453)
Bumps [cloud.google.com/go/kms](https://github.com/googleapis/google-cloud-go) from 1.30.0 to 1.31.0. - [Release notes](https://github.com/googleapis/google-cloud-go/releases) - [Changelog](https://github.com/googleapis/google-cloud-go/blob/main/documentai/CHANGES.md) - [Commits](https://github.com/googleapis/google-cloud-go/compare/kms/v1.30.0...dlp/v1.31.0) --- updated-dependencies: - dependency-name: cloud.google.com/go/kms dependency-version: 1.31.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
|
fade4ce77d |
build(deps): bump github.com/rabbitmq/amqp091-go from 1.10.0 to 1.11.0 (#9454)
Bumps [github.com/rabbitmq/amqp091-go](https://github.com/rabbitmq/amqp091-go) from 1.10.0 to 1.11.0. - [Release notes](https://github.com/rabbitmq/amqp091-go/releases) - [Changelog](https://github.com/rabbitmq/amqp091-go/blob/main/CHANGELOG.md) - [Commits](https://github.com/rabbitmq/amqp091-go/compare/v1.10.0...v1.11.0) --- updated-dependencies: - dependency-name: github.com/rabbitmq/amqp091-go dependency-version: 1.11.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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 |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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.
|
||
|
|
91bcc910eb |
build(deps): bump actions/dependency-review-action from 4.9.0 to 5.0.0 (#9450)
Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4.9.0 to 5.0.0. - [Release notes](https://github.com/actions/dependency-review-action/releases) - [Commits](https://github.com/actions/dependency-review-action/compare/2031cfc080254a8a887f58cffee85186f0e49e48...a1d282b36b6f3519aa1f3fc636f609c47dddb294) --- updated-dependencies: - dependency-name: actions/dependency-review-action dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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
|
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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). |
||
|
Chris LuandGitHub
|
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). |
||
|
Chris LuandGitHub
|
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.
|
||
|
Chris LuandGitHub
|
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 |
||
 Chris Lu
|
b456628a7a |
ui(s3_lifecycle): plain-English labels for cadence fields
Dispatch Tick / Cursor Checkpoint Tick / Engine Refresh / Bootstrap Re-walk are internal terms — operators tuning the form had to read the descriptions to guess what each field meant. Renames the visible labels and the section blurb; underlying field names are unchanged so stored configs still load. |
||
|
Chris LuandGitHub
|
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). |
||
|
Chris LuandGitHub
|
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)
|
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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). |
||
|
Chris LuandGitHub
|
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). |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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
|
||
|
Chris LuandGitHub
|
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
|
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |
||
|
Chris LuandGitHub
|
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. |