mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-21 06:36:54 +00:00
e785f59d6fb266d4abbeebb58cd0b26e6f920afd
13793
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris Lu
|
e785f59d6f |
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.
|
||
|
Chris Lu
|
60bee61189 |
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. |
||
|
Chris Lu
|
7948f7a945 |
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. |
||
|
Chris Lu
|
191a088c09 |
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. |
||
|
Chris Lu
|
ab1f448472 |
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. |
||
|
Chris Lu
|
957ac1335c |
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.
|
||
|
Chris Lu
|
794e59ea61 |
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.
|
||
|
Chris Lu
|
1f97d2e5de |
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. |
||
|
Chris Lu
|
1ce184f735 | test(s3/lifecycle): gofmt size-filter test | ||
|
Chris Lu
|
1a226d2688 |
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. |
||
|
Chris Lu
|
155600829a |
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. |
||
|
Chris Lu
|
470339aa21 |
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. |
||
|
Chris Lu
|
4aa1c2fb1b |
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. |
||
|
Chris Lu
|
54954bb3eb |
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. |
||
|
Chris Lu
|
fe70c1d2e5 |
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. |
||
|
Chris Lu
|
66df82b4e8 |
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. |
||
|
Chris Lu
|
bb0d4859ec |
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. |
||
|
Chris Lu
|
ac74c6764a |
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 |
||
|
Chris Lu
|
a4479a927e |
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).
|
||
  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. |
||
|
Chris LuandGitHub
|
435ef7f94f |
test(s3/lifecycle): pin toProtoActionKind + toProtoIdentity converters (#9404)
test(s3/lifecycle): pin toProtoActionKind + toProtoIdentity The two converters are the worker-side wire to LifecycleDelete; a miss in toProtoActionKind sends ACTION_KIND_UNSPECIFIED that the server rejects FATAL, and a wrong toProtoIdentity flips the CAS witness so every dispatch comes back NOOP_RESOLVED with STALE_IDENTITY even though the entry hasn't changed. 10 tests pin: every listed s3lifecycle.ActionKind maps to its proto counterpart (table-driven, one subtest per kind); ActionKindUnspecified and a future unknown kind both collapse to ACTION_KIND_UNSPECIFIED (forward compat); nil EntryIdentity stays nil (preserves the no-CAS sentinel); a populated identity copies every field; a zero-valued identity still produces a non-nil output so the server treats it as a real CAS witness rather than no-CAS. |
||
|
Chris LuandGitHub
|
1350e681c9 |
test(s3/lifecycle): pin Pipeline.Run dependency + shard validation (Phase 15 slice) (#9402)
* test(s3/lifecycle): pin Pipeline.Run dependency + shard validation Pre-existing TestPipelineRunRequiresDependencies only checked that an empty Pipeline errors; it didn't pin which specific dependency must be present. A refactor that makes one nilable accidentally would slip through. 8 new tests pin every validation branch in Pipeline.Run: missing Engine / Persister / Client / FilerClient each error with "missing required dependency"; missing BucketsPath errors with its own distinct message so operators can spot the missing wiring; ShardID = -1 / ShardCount errors with a range message (covers the half-open [0, ShardCount) boundary so a < to <= refactor can't introduce a one-past-the-end shard); and a multi-shard config with one out-of-range entry refuses the whole run rather than silently disabling the rest. * test(s3/lifecycle): refactor Pipeline.Run validation tests as table-driven Per gemini review on #9402: collapse the eight per-branch tests into TestPipelineRunValidation with a slice of (name, mutate, wantErr) cases. Same coverage, ~30 fewer lines, idiomatic Go pattern that makes adding a new validation case trivial. |
||
|
Chris LuandGitHub
|
cb6e498e0b |
test(s3/lifecycle): pin Descriptor structural invariants (#9401)
* test(s3/lifecycle): pin Descriptor structural invariants Pre-existing handler tests covered Capability and Detect; Descriptor was previously untested. A drift between the form fields it advertises and the defaults config.go reads silently breaks the admin UI in two ways: the form renders blank (admin can't tune) or the worker clamps to a hardcoded fallback ignoring the admin's edits. The new tests catch both directions. Pinned: jobType / DisplayName / Description / DescriptorVersion; AdminConfigForm exposes a workers field whose default matches defaultWorkers; WorkerConfigForm has a default and a field for every cadence knob ParseConfig reads (dispatch_tick / checkpoint_tick / refresh_interval / bootstrap_interval / max_runtime); AdminRuntime- Defaults hits a daily cadence with bounded detection timeout and single job per detection. * test(s3/lifecycle): tighten Descriptor invariant assertions Per gemini review on #9401: pin DetectionTimeoutSeconds to its exact value (60) instead of ">0" so an accidental tweak is caught, and assert WorkerConfigForm fields are INT64 (matching ParseConfig's readInt64) so a STRING-type drift can't silently make the worker ignore admin edits. |
||
|
Chris LuandGitHub
|
6f9668c20b |
test(s3/lifecycle): pin lifecycleDispatch validation early-returns (#9400)
Three pure-validation paths in lifecycleDispatch return BLOCKED before any filer call; without coverage a refactor could let them fall through to a real delete. ABORT_MPU at dispatch time is a defensive catch (the route bypass should never happen, but if it does the fallthrough must not become a default-case rm). Unknown ActionKind gets the same treatment for forward-compatibility with new proto values. Empty version_id on noncurrent / EXPIRED_DELETE_MARKER kinds must be rejected before deleteSpecificObjectVersion is called, so a malformed event can't silently delete the latest pointer. |
||
|
Chris LuandGitHub
|
af2a359e45 |
feat(s3/lifecycle): metadata_only_total Prometheus counter (#9399)
Operator-visible signal for the metadata-only delete path landed in
PR 9390. Increment seaweedfs_s3_lifecycle_metadata_only_total{bucket,
rule_hash} after each successful unversioned or noncurrent / expired-
marker delete that took the skip-chunk path. Suspended-versioning
null delete is intentionally not counted: that path's nil err can
mean "deleted" or "NotFound", so a count there would over-report.
rule_hash is hex-encoded for label safety; nil bytes collapse to
"". DeleteBucketMetrics tears the new series down alongside the
existing lifecycle counters when a bucket is removed.
|
||
|
Chris LuandGitHub
|
c0cf1417f1 |
test(s3/lifecycle): cover worker handler Execute validation paths (#9398)
7 tests pin the Execute early-return surface that runs without a filer or S3 dial: nil request / nil Job / nil sender all error; foreign JobType errors with the offending name in the message; no S3 endpoints in cluster context errors (Execute is stricter than Detect — the admin shouldn't have routed the job there); missing filer_grpc_address parameter errors (proposal must have been tampered with or dropped); empty JobType is accepted as broadcast routing and flows through to the next validation step. The dial path itself is intentionally not covered here — those tests would need an in-process gRPC server and belong with the integration suite. |
||
|
Chris LuandGitHub
|
284d37c3b6 |
test(s3/lifecycle): cover InMemoryPersister deep-copy contract (#9397)
* test(s3/lifecycle): cover InMemoryPersister deep-copy contract 8 tests pin the persister contract other lifecycle tests rely on for cursor checkpointing: Load on an unknown shard returns an empty map (not an error); Save then Load roundtrips; Save copies the input so caller-side mutation doesn't bleed into stored state; Load returns a copy so caller-side mutation of the snapshot doesn't bleed back; Save replaces (not merges) prior state so stale resume points don't survive restart; different shards stay isolated; saving an empty map clears state; concurrent Save+Load is race-free under -race. A regression on any of these silently corrupts downstream tests. * test(s3/lifecycle): assert.NotContains for InMemoryPersister key absence assert.Empty on a map[K]V index returns true when the value is the zero value, which would mask a key that leaked through with int64(0). Use assert.NotContains so the assertion fails on key presence regardless of the stored value. |
||
|
Chris LuandGitHub
|
62e04623ce |
test(s3/lifecycle): cover worker handler Detect + helpers (#9396)
* test(s3/lifecycle): cover worker handler Detect + helpers 13 tests pin the worker-handler surface that runs without a live filer or S3 server. Pure helpers: clusterS3Endpoints (nil context, empty list, filter empty entries while preserving order, all-valid passthrough); readString (missing key, nil ConfigValue, wrong kind falls back, string returned). Capability advertises jobType with single-job concurrency caps. Detect: nil request / nil sender / wrong JobType all error; no S3 endpoints emits a 'skipped' activity and completes with success; no filer addresses behaves the same; the happy path proposes one job parameterized with the first filer address; empty JobType is accepted (broadcast detect); a SendProposals failure propagates without firing complete. * test(s3/lifecycle): cover SendComplete error propagation in worker Detect The recordingSender already supported forcing an err on SendComplete via errOn, but no case exercised it. A SendComplete failure must propagate so the admin learns the completion signal never landed; proposals went out before the failure so they remain recorded. |
||
|
Chris LuandGitHub
|
551e700e64 |
test(s3/lifecycle): cover scheduler configload surface (#9395)
* test(s3/lifecycle): cover scheduler configload surface LoadCompileInputs is the bridge between the filer's bucket directory and the engine snapshot the scheduler compiles every refresh; a missed or misclassified bucket silently disables lifecycle for that prefix until the next refresh. Tests pin: empty bucket dir, files at the bucket level skipped, buckets without the lifecycle XML extended key skipped, empty-bytes XML skipped, valid XML becomes a CompileInput, versioning attr propagates to CompileInput.Versioned, malformed XML surfaces as a ParseError without aborting the walk, and pagination across the 1024 page boundary preserves bucket order. Also covers the IsBucketVersioned (case + whitespace tolerance, rejection of garbage values) and AllActivePriorStates (one entry per (bucket, ruleHash, actionKind), bucket-keyed isolation) helpers. * test(s3/lifecycle): tighten configload pagination boundary check Switch the bucket-count check to require.Len so a regression that returns the wrong number of buckets fails fast before the boundary asserts panic on out-of-range index. Add explicit assertions on the last entry of page 1 (b01023) and the first entry of page 2 (b01024) so a pagination-loop bug that drops or duplicates the seam is caught directly rather than only via the count check. |
||
|
Chris LuandGitHub
|
6021a88606 |
test(s3/lifecycle): cover CompareVersionIds tiebreak surface (#9394)
* test(s3/lifecycle): cover CompareVersionIds tiebreak surface 13 tests pin every documented branch of the version-id comparator and its helpers (isNewFormatVersionId, getVersionTimestamp): equality and short-circuit paths, null sorting last, both-new-format with smaller- =-newer ordering, both-old-format with larger-=-newer ordering, mixed- format compared by parsed timestamp, mixed-format with synthesized equal timestamps, length / null / non-hex rejection, the strictly- greater-than threshold boundary at 0x4000000000000000, and the inverted-value invariant the comparator relies on. Getting any axis wrong silently inverts retention rankings, which would resurrect deleted versions or evict live ones. * test(s3/lifecycle): use plain assert.Equal in mixed-format compare test The previous local require := assert.New(t) shadowed testify's require package while actually returning an assert.Assertions (continue-on-fail semantics, not fail-fast). Use plain assert.Equal(t, ...) calls so the behavior matches the variable's name and the rest of the file. |
||
|
Chris LuandGitHub
|
7781eef429 |
test(s3/lifecycle): cover dispatcher filerSiblingLister surface (Phase 14 slice) (#9392)
* test(s3/lifecycle): cover dispatcher's filerSiblingLister surface Tests pin the four routing-critical filer interactions on the filerSiblingLister: Survivors (count cap, LoneEntry semantics, null-version detection across regular files and directory-key markers, error propagation in both list and lookup paths), ListVersions (NotFound collapse, dir/missing-id filtering, 1024-page boundary, error propagation), LookupNullVersion (regular file, explicit-null flag, directory-key marker accept, plain-dir reject, NotFound collapse, error propagation), and LookupVersion (empty version-id no-op, v_ prefix, NotFound collapse, error propagation). The fake SeaweedFilerClient mirrors the real filer's NotFound shape — gRPC succeeds at stream creation and the first Recv() surfaces filer_pb.ErrNotFound — which is what the lister's errors.Is check depends on. NewFullPath strips a trailing slash before splitting so directory-key markers are stored under their slashless Name. * test(s3/lifecycle): gofmt sibling_lister_test.go Trailing comment alignment. |
||
|
Chris LuandGitHub
|
8cf42a5abb |
test(s3/lifecycle): assert per-goroutine errors in fake-server concurrent test (#9393)
test(s3/lifecycle): assert per-goroutine errors in concurrent fake test The previous TestFake_ConcurrentCallsSerializeWithoutDeadlock dropped the err return from each LifecycleDelete call, so a regression in the concurrent path could pass the length-only assertion. Capture each err on a buffered channel and require.NoError after wg.Wait(). |
||
|
Chris LuandGitHub
|
ddfb219ec3 |
test(s3/lifecycle): fake LifecycleDelete server (Phase 12 slice) (#9391)
* test(s3/lifecycle): fake LifecycleDelete server for component tests A reusable double for SeaweedS3LifecycleInternalServer with per-key FIFO outcome queues, a fallback Default, and recorded request capture. Tests of the worker pipeline that need to hit the proto boundary can queue up DONE/NOOP/RETRY/FATAL/SKIPPED_OBJECT_LOCK responses per (bucket, objectPath, versionId) and assert dispatch order against Recorded(). SetError flips the server into transport-failure mode without polluting the request log. * test(s3/lifecycle): use struct map key for FakeLifecycleServer queues Bucket / object path / version-id are user-supplied strings that can contain "/" or "@", which would collide if the queue map were keyed by "<bucket>/<object>@<version>". Switch to a struct key so the components stay separate. * test(s3/lifecycle): deep-copy recorded LifecycleDelete requests Tests that mutate a Recorded() entry — or a request pointer they already passed in — were able to corrupt the fake's bookkeeping because the slice carried shared pointers. Clone with proto.Clone at both record and read time so the fake holds an independent snapshot of every arriving request and hands callers an independent snapshot back. Tightened TestFake_VersionIDPartOfKey error checks while there. |
||
|
Chris LuandGitHub
|
bb0c7c779f |
feat(s3/lifecycle): metadata-only delete when entry TtlSec > 0 (Phase 2b) (#9390)
* refactor(s3): thread metadataOnly into delete helpers Add a metadataOnly bool to deleteUnversionedObjectWithClient and deleteSpecificObjectVersion. When true the helper sends IsDeleteData= false to the filer's DeleteEntry RPC so per-chunk DeleteFile RPCs are skipped — the volume server reclaims chunks on its own at TTL drop. Non-lifecycle callers (DELETE handlers, batch delete) pass false to preserve today's eager-chunk-delete behavior; only the lifecycle handler in the next commit will pass true. * feat(s3/lifecycle): metadata-only delete when entry TtlSec > 0 Per-write TTL stamping (PR 9377) sets Attributes.TtlSec on every lifecycle-fitting entry. When the live entry the LifecycleDelete handler fetched carries TtlSec > 0 the volume server is guaranteed to reclaim chunks at TTL drop, so the filer can skip per-chunk DeleteFile RPCs and just remove the entry record. lifecycleDispatch now computes metadataOnly from the live entry and threads it through the unversioned, suspended-null, and noncurrent/expired-marker delete paths. createDeleteMarker is unaffected — it creates a marker, never deletes chunks. |
||
|
Chris LuandGitHub
|
255e9cd0f7 |
test(s3/lifecycle): cover reader cursor + Run validation contracts (#9389)
* test(s3/lifecycle): cover reader cursor + Run validation contracts Layer 2 tests pinning four reader-package contracts the dispatcher pipeline depends on: MinTsNs anchors at frozen positions, Snapshot returns a deep copy in both directions, Restore replaces (not merges), and Run validates ShardID/Events/BucketsPath before subscribing. * test(s3/lifecycle): tighten cursor composition assertions Snapshot deep-copy: also assert cursor doesn't see keys added to the returned map. Restore replace: freeze before second Restore and assert IsFrozen returns false after, pinning the contract that Restore wipes frozen state alongside the value map. Run validation: bound the call with a 5s context timeout so a regression that lets Run reach the nil client surfaces as a failure instead of a hang. |
||
|
Chris LuandGitHub
|
aa280443e7 |
test(s3/lifecycle): Layer 2 multi-shard composition for the dispatcher (#9387)
* test(s3/lifecycle): Layer 2 multi-shard composition for the dispatcher The existing dispatcher unit tests cover individual outcomes (DONE / RETRY_LATER / BLOCKED / etc.) on a single shard, and pipeline_test.go has only one end-to-end happy-path assertion. Multi-shard composition — the contract Pipeline.Run wires up at runtime — was untested at the component level. Add four Layer 2 tests in dispatcher/multi_shard_test.go: Two events for two shards land in different schedules, dispatch independently, and each cursor advances only for its own event (no cross-contamination on the action-key map). A poison event on shard 0 returns BLOCKED and freezes shard 0's cursor; shard 1's normal event continues to dispatch and its cursor advances. Per-shard isolation contract. Save/Load round-trips a per-shard cursor snapshot through the Persister: a fresh dispatcher restores the same TsNs map. Pins the contract Pipeline.Run drives on the checkpoint ticker. RETRY_LATER respects RetryBackoff against the wall clock — a Tick within the backoff window doesn't re-dispatch; a Tick past the new DueTime does. Guards against premature retries from refresh ticks landing inside the backoff. Pipeline.Run itself can't run here (it builds a real reader.Reader), so the tests share the same fakeClient pattern dispatcher_test.go uses and drive Tick directly. * test(s3/lifecycle): drop unused snapshot helper and addAndTick parameter |
||
|
Chris LuandGitHub
|
1854101125 |
feat(s3/lifecycle): bootstrap re-walk cadence + operator hooks (Phase 8) (#9386)
* feat(s3/lifecycle): bootstrap re-walk cadence + operator hooks (Phase 8) scan_only actions only fire from the bootstrap walk: the engine classifies a rule as scan_only when its retention horizon exceeds the meta-log retention, so event-driven routing can't be trusted. Today each bucket walks once per process, so a long-running worker never revisits — scan_only retention only catches up when the worker restarts. Replace BucketBootstrapper.known (set) with BucketBootstrapper.lastWalk (name -> completion time). KickOffNew now re-walks a bucket whose last walk completed more than BootstrapInterval ago. Zero interval preserves the legacy walk-once-per-process behavior so existing deployments don't change cadence by default. walkBucket re-stamps on success and clears the stamp on failure (via MarkDirty), so the next KickOffNew picks failed walks back up. Add MarkDirty / MarkAllDirty operator hooks for forced re-walks, and a Now func() for testable time travel. weed shell run-shard grows --bootstrap-interval (cadence knob) and --force-bootstrap (drop in-memory state at startup so every bucket walks again immediately, useful when a config change should take effect without a restart). Tests: cadence respected (skip inside interval, re-walk past it); zero interval keeps once-per-process; MarkDirty forces re-walk under a 24h interval; MarkAllDirty resets every record. The fakeClock helper guards the test clock with a mutex so race-detector runs are clean. * fix(s3/lifecycle): split walk state, thread BootstrapInterval through worker, drop dead flag Three issues with the Phase 8 cadence work as it landed: 1. lastWalk did double duty as both completed-walk timestamp and in-flight debounce. A walk that took longer than BootstrapInterval would have a fresh KickOffNew start a duplicate goroutine on the next refresh tick because the stamp from KickOffNew looked stale against the interval. Split into lastCompleted (set on success) and inFlight (set on dispatch, cleared after the walk goroutine returns success or failure). KickOffNew skips inFlight buckets regardless of cadence. 2. The cadence knob existed on `weed shell` but not on the production path: scheduler.Scheduler constructed BucketBootstrapper without BootstrapInterval, and weed/worker/tasks/s3_lifecycle/Config had no field for it. Add Scheduler.BootstrapInterval, parse `bootstrap_interval_minutes` in ParseConfig (zero = legacy walk- once-per-process; negative clamps to zero), and forward it from the handler. Tests cover default, override, clamp, and explicit-zero. 3. --force-bootstrap was a no-op: BucketBootstrapper is freshly allocated at command start, so MarkAllDirty on empty state does nothing, and the flag couldn't influence an already-running process anyway. Remove it; a real runtime trigger (SIGHUP, control RPC) is a separate change. In-flight regression: a blockingInjector pins the first walk in progress while the test advances the clock past the interval. The second KickOffNew is a no-op (inFlight check). After release, the post-completion KickOffNew within the interval is also a no-op. * test(s3/lifecycle): wait for lastCompleted stamp before advancing fake clock The cadence test polled listedN to know "the walk happened" — but that fires once both list passes are issued, while the success-stamp lands later, after walkBucketDir returns. A clock.Advance(30m) between those two events would record the stamp at clock+30m instead of T0; the next assertion would then see now.Sub(last) < 1h and skip the expected re-walk. Tight in practice but exposed under -race / load. Add a waitForCompleted helper that polls b.lastCompleted directly, and use it before each clock advance in both the cadence and zero- interval tests. * fix(s3/lifecycle): expose bootstrap interval in worker UI; honor MarkDirty during walks Two follow-ups on Phase 8. The worker config descriptor had no bootstrap_interval_minutes field, so the production operator UI couldn't enable the cadence — only the internal ParseConfig + Scheduler wiring knew about it. Add the field to the cadence section (MinValue=0 since 0 is the legacy default) and include the default in DefaultValues so existing deployments see the knob with the right preset. MarkDirty / MarkAllDirty silently lost their effect when a walk was in flight: the methods cleared lastCompleted, but the walk's success path then wrote a fresh timestamp, hiding the operator's invalidation. Track a pendingDirty set; the walk goroutine consumes the flag on exit and skips the success stamp, so the next KickOffNew picks the bucket up immediately. Regression: pin a walk in progress with a blockingInjector, MarkDirty the bucket, release the walk, and assert lastCompleted stayed empty plus the next KickOffNew triggers a new walk inside the BootstrapInterval window. * refactor(s3/lifecycle): drop unused MarkDirty / MarkAllDirty + pendingDirty These methods were the operator-hook half of Phase 8, but the only caller (--force-bootstrap on the shell command) was removed when it turned out to be a no-op against a freshly-allocated bootstrapper. Nothing in production calls them anymore. Strip the dead surface: MarkDirty, MarkAllDirty, the pendingDirty set, the dirty-suppression branch in walkBucket, and the three tests that only exercised those methods. BootstrapInterval-driven re-bootstrap is the live mechanism. A real runtime trigger (SIGHUP, control RPC) is a separate change with a real call site. |
||
|
Chris LuandGitHub
|
edfa1ce210 |
feat(s3/lifecycle): pointer-transition routing for live PUTs (Phase 5b/4) (#9385)
* feat(s3/lifecycle): pointer-transition routing for live PUTs (Phase 5b/4) Bootstrap covers existing versions, but a live PUT that creates a new .versions/<v-new> file and updates the parent's ExtLatestVersionIdKey didn't fire NoncurrentDays / NewerNoncurrent on the displaced prior version until the next bootstrap. Close that runtime gap. The meta-log already emits an Update event for the .versions/ directory itself when the latest pointer changes; the router was dropping it because buildObjectInfo returns nil for directories. New branch in Route detects that shape (versioned bucket, NewEntry + OldEntry both directories with the .versions/ suffix, ExtLatestVersionIdKey changed, ID different from the new ID) and emits a Match against the LOGICAL key with VersionID=oldID. Match.Identity comes from a single LookupVersion RPC for the displaced version file; SuccessorModTime is the directory update's mtime, which is the moment the displaced version became noncurrent. SiblingLister grows LookupVersion(bucket, key, versionID) for that single-RPC fetch. filerSiblingLister implements it; routing path treats NotFound as "displaced version was hard-deleted in the meantime, suppress" rather than an error. The router gates the lookup on at least one active event-driven NoncurrentDays / NewerNoncurrent rule for the bucket, so most buckets pay nothing per directory update. Tests: pointer-flip fires NoncurrentDays with displaced version_id; unchanged pointer skips; empty old pointer skips (first-PUT scenario); displaced-version NotFound suppresses; no-rule skips lookup; NewerNoncurrentVersions retains rank-0; unversioned bucket skips. * fix(s3/lifecycle): SuccessorModTime cache + NewerNoncurrent expansion Two correctness gaps in pointer-transition routing. The .versions/ directory's own Attributes.Mtime is preserved across pointer updates by updateLatestVersionInDirectory: it's a stale clock relative to the freshly-written latest version. Using it as the displaced version's SuccessorModTime made NoncurrentDays compute due = staleMtime + days, which fires immediately on a fresh PUT into an old .versions/ container. Read ExtLatestVersionMtimeKey written by setCachedListMetadata; suppress (return no matches) when the cache is missing rather than fall back to dir mtime. Single-oldID lookup is only enough for pure NoncurrentVersionExpirationDays. Any rule with NewerNoncurrentVersions > 0 cares about the noncurrent ranks, and a pointer flip shifts every prior noncurrent's index by one — the version that just crossed the keep-count threshold needs to be evaluated too. When any matching rule needs ranks, list the full .versions/ container, sort newest- first with mtime + version-id tiebreak, and route every noncurrent with its real index. Identity-CAS dedups against earlier schedules. SiblingLister grows ListVersions(bucket, key); filerSiblingLister's implementation paginates the container fully. Two regression tests: stale dir mtime + correct cached mtime schedules ~30 days out (not immediate); NewerNoncurrentVersions=2 with 4 versions fires on the rank-2 entry that just crossed the threshold while rank-0/1 are retained. * fix(s3/lifecycle): bound pointer-transition expansion to threshold crossings routePointerTransitionExpand emitted a Match for every eligible noncurrent on every PUT. Schedule.Add doesn't dedup, so identity-CAS at dispatch only saved the wasted RPC, not the heap slot. A hot key with many already-eligible versions and a count rule would push O(versions) entries per flip, repeatedly, until dispatch caught up. Bound the emission to versions that newly entered eligibility on this specific flip: rank 0 (the displaced version, for the NoncurrentDays clock) plus rank == rule.NewerNoncurrentVersions for each active count-gated rule (the version that just crossed from kept to expired). Bootstrap still owns full backfill for versions that were already over-threshold. Adds a regression with 6 versions and NewerNoncurrentVersions=2: asserts only the rank-2 entry that just crossed fires, not the already-over-threshold rank-3/rank-4 entries. * fix(s3/lifecycle): suppress pointer-transition expansion when newID missing routePointerTransitionExpand defaulted latestPos to 0 if newID wasn't found in the listing. That made the actual newest sibling latest against the pointer's intent, then misranked every other version. A race between the pointer write and the version write could land us there. Default latestPos to -1, set it only on a real match, and suppress the expansion when the search misses. Bootstrap repairs state on the next walk. The NewerNoncurrentVersions retention test was setting only lookupEntry, so Route never reached the expansion path it claimed to exercise. Repoint to listVersions and assert ListVersions was consulted while LookupVersion was not. Adds a regression covering the missing-newID suppression directly. * fix(s3/lifecycle): include bare null version in pointer-transition routing Bootstrap models the bare-key object as a "null" sibling alongside .versions/ children, but the live pointer-transition path didn't. Two cases lost: 1. oldID == "" was treated as "nothing displaced". A pre-versioning bare object becomes noncurrent when the first versioned PUT lands and the pointer flips to a real id, but live routing skipped it and waited for the next bootstrap. 2. The expansion path's ListVersions returned only .versions/ children. With a bare null in the picture, the noncurrent ranks were wrong, so NewerNoncurrentVersions could keep the wrong versions and delete the right ones (or vice versa). SiblingLister grows LookupNullVersion(bucket, key) returning the bare entry plus an explicit-null flag (matches the bootstrap shape). filerSiblingLister implements it via util.NewFullPath + filer_pb.LookupEntry. routePointerTransitionDisplaced: oldID == "" now consults LookupNullVersion. When the bare entry exists, route it as VersionID="null" against the LOGICAL key. routePointerTransitionExpand: collect .versions/ children plus the null entry into one sibling slice before sorting and ranking. The threshold-crossing logic now sees the same N-version set that bootstrap would compute. Three new tests: oldID == "" with no null is a no-op (one null lookup, no version lookup); oldID == "" with a bare null schedules NoncurrentDays as VersionID="null"; expansion with a bare null between .versions/ siblings places null at its mtime-correct rank and only that rank-N entry fires. * fix(s3/lifecycle): atomic listPageSize so test cleanup doesn't race KickOffNew dispatches walks via `go b.walkBucket(...)`. A test that finishes before its goroutines drain leaves them running into the next test's t.Cleanup, which mutates listPageSize. -race spots the read/write collision intermittently. Convert listPageSize to atomic.Uint32; tests use Load/Store. No production semantics change. * fix(s3/lifecycle): null becomes latest when suspended PUT clears pointer The router treated newID == "" as if the cached ExtLatestVersionMtimeKey were still authoritative — but that cache holds the displaced version's mtime, written by setCachedListMetadata when the prior version became latest. Using it as SuccessorModTime made NoncurrentDays=30 immediately fire on a 100-day-old displaced version even though it became noncurrent today. When newID == "" the bare null is the new latest. Look it up, substitute its mtime as the successor clock, and substitute "null" as the latestPos target for the expansion path's id match. Both displaced and expand paths now derive the right clock. updateIsLatestFlagsForSuspendedVersioning was the upstream cause of the staleness — it cleared ExtLatestVersionIdKey and FileNameKey but left the cached size/mtime/etag/owner/delete-marker behind. Call clearCachedVersionMetadata so the .versions/ container is consistent with "null is latest". The router-side guard is still needed for older deployments that ran the buggy code, but new writes won't exercise the workaround. Two regressions: 100-day-old displaced under NoncurrentDays=30 with a today-null PUT schedules ~30d out (not immediate); same shape with NewerNoncurrentVersions=2 ranks the null at latest and only the rank-2 entry fires. |
||
|
Chris LuandGitHub
|
2f7ac1d664 |
feat(s3/lifecycle): NoncurrentVersionExpiration via bootstrap (Phase 5b/3) (#9383)
* feat(s3/lifecycle): NoncurrentVersionExpiration via bootstrap (Phase 5b/3)
Bootstrap now expands every <key>.versions/ directory into one event
per version with sibling state pre-computed. The router fires
NoncurrentDays / NewerNoncurrent off these events using
SuccessorModTime as the noncurrent clock; previously these rules
never ran on a versioned bucket because buildObjectInfo couldn't
classify version-folder events without the latest pointer.
Mechanics
walkBucketDir treats a directory ending in .versions and carrying
ExtLatestVersionIdKey as a SeaweedFS .versions container — emit it
once and skip the recursion. Coincidentally-named directories without
the latest pointer recurse normally.
BucketBootstrapper.expandVersionsDir lists the children, sorts
newest-first by mtime, resolves the latest position from the pointer,
and injects a synthesized reader.Event per version with
BootstrapVersion populated. NoncurrentIndex is 0-based among
noncurrents in newest-first order; SuccessorModTime is the immediate
newer sibling's mtime (zero for the latest). Pointer naming a missing
or absent version falls back to the newest-by-mtime sibling so a
race window can't flag every entry as noncurrent.
routeBootstrapVersion uses BootstrapVersion to build ObjectInfo
directly (bypassing the version-folder skip in buildObjectInfo) and
runs the standard match loop. ABORT_MPU is excluded by kind-shape
gate. The schedule clock uses SuccessorModTime for noncurrents and
ModTime for the latest, so the dispatcher fires when the rule's days
threshold is met. Match.ObjectKey is the LOGICAL key,
Match.VersionID is the marker's stored version_id — the dispatcher
reaches deleteSpecificObjectVersion or createDeleteMarker correctly.
Layer 2 tests cover both sides. Router: latest fires ExpirationDays;
noncurrent fires NoncurrentDays; NewerNoncurrentVersions retains the
N newest noncurrents; ABORT_MPU never matches. Bootstrap: .versions
dir emitted once and not recursed; missing latest pointer falls back
to newest; backdated PUT (latest pointer is older by mtime) keeps
the right noncurrent index; delete-marker flag propagates.
* fix(s3/lifecycle): no VersionID for latest expirations, child-based dir disambig
Two correctness gaps in Phase 5b/3.
Bootstrap was pinning the version_id on every Match. For
EXPIRATION_DAYS / EXPIRATION_DATE on the latest version this is
unsafe: between schedule and dispatch a fresh PUT can land, the
dispatcher would still identity-match against the original version's
bytes (it still exists at that path) and the resulting delete marker
would hide the new latest. Drop VersionID for those kinds; an empty
VersionID makes the dispatcher fetch the current latest, where
identity-CAS resolves to STALE_IDENTITY and bootstrap re-schedules
with the new latest's identity. NoncurrentDays / NewerNoncurrent /
EXPIRED_DELETE_MARKER still pin the version_id since those are
version-targeted.
isVersionsDir gating on ExtLatestVersionIdKey lost a race window:
createDeleteMarker writes the version file before updating the
parent's Extended pointer, so a walk between those two steps would
see a .versions/ dir without the pointer, recurse into it, and emit
raw version files that the router drops. Match the suffix only and
let expandVersionsDir disambiguate by child inspection: if any child
carries ExtVersionIdKey it's a real .versions container and we expand;
otherwise it's a coincidentally-named user folder and we recurse via
the bucket-walk's own callback so nested entries still flow through.
Tests: latest-expiration assertion flipped to expect empty VersionID;
new tests cover the coincidentally-named-folder recursion and the
race-window expansion (children present, pointer absent).
* fix(s3/lifecycle): filter directory + missing-version-id children at listing
expandVersionsDir's listing callback collected every child with
attributes; subdirectories or entries without ExtVersionIdKey would
make it past the empty-id skip in the inner loop but still inflate
NumVersions and skew NoncurrentIndex (the rank derives from the
filtered slice's position, which was wrong when the unfiltered slice
was sorted). Drop directories at listing time and partition the
file children into a versions slice that's the actual rank source.
Test cleanups: out-of-order-mtime test now sets v1 older than v2 so
latestPos > 0 actually exercises the rank-skip branch in
expandVersionsDir; bootstrapVersionEntry preserves nanosecond
precision via MtimeNs to match markerLoneEntry's pattern; drop a
leftover unused idx variable.
* fix(s3/lifecycle): null version + canonical version-id tiebreak
Two correctness gaps in Phase 5b/3 bootstrap.
Null versions live at the bare logical path, not under .versions/.
Bootstrap previously expanded only .versions/<key>/ children, so:
- pre-versioning objects with newer .versions/ history never had
their null version expired by NoncurrentDays
- suspended-bucket writes (which clear the .versions/ latest pointer
so null becomes current) had every .versions/ child wrongly
classified as latest by the buildObjectInfo fallback
expandVersionsDir now looks up the bare key via NewFullPath +
LookupEntry, accepts a regular file or an explicit S3 directory-key
marker (Mime set), and folds it into the sibling set with
VersionID="null". Latest resolution: pointer present + names a real
id wins; pointer absent + null exists makes null latest; otherwise
falls back to newest sibling. The walker's regular emission for the
bare entry would otherwise duplicate, so walkBucketDir now does a
two-pass walk per directory level — .versions/ first, then everything
else with a per-walk skipBare set keyed by bucket-relative path that
expandVersionsDir populates when it claims a bare null sibling.
Sort tiebreak: PUTs only set second-level Mtime, so two versions
written in the same second tied. The unstable secondary order let
old-format version filenames sort oldest-first and corrupt
NoncurrentIndex under NewerNoncurrentVersions retention. Add
CompareVersionIds to s3lifecycle/version_time.go (mirrors the
canonical comparator in s3api/s3api_version_id.go to avoid the
import cycle) and use it as a secondary key after mtime equality.
Tests: pre-versioning null-as-noncurrent, suspended null-as-current,
directory-key marker as null version, end-to-end claim through
walkBucketDir's two-pass ordering, and same-second tiebreak via
canonical version-id ordering. fakeFilerClient grows a
LookupDirectoryEntry implementation backed by the same in-memory tree.
* fix(s3/lifecycle): only treat explicit-null bare entries as current
The pointer-missing branch in expandVersionsDir made null latest as
soon as a bare object was found. That's correct for suspended-bucket
writes (s3api_object_handlers_put.go writes the bare entry with
ExtVersionIdKey="null") but wrong for the pre-versioning race window:
a brand-new version under .versions/<file> exists before the parent's
ExtLatestVersionIdKey update lands, and a pre-versioning bare object
has no version-id marker. Marking that older bare object latest hides
the real new version and skips noncurrent expiration of the null
until the next process restart/bootstrap.
Distinguish the two: lookupNullVersion now returns whether the bare
entry's Extended map carries ExtVersionIdKey="null" (the suspended
write marker). expandVersionsDir's pointer-missing branch only
promotes null to latest when explicit; otherwise it falls back to
newest-sibling, which is safe for the race window since the new
version's mtime is fresher than the bare object's.
The existing suspended-null test now uses a new helper that adds the
explicit marker. New regression test covers the race window: bare
entry without the marker + a fresh .versions/<v1> file + missing
parent pointer must keep v1 as latest and the null as noncurrent.
* fix(s3/lifecycle): only the newest item can be the explicit-null latest
The pointer-missing branch in expandVersionsDir scanned every item for
an explicit null and promoted it to latest. After a suspended->enabled
transition that's the wrong call: createVersion writes the version
file before updating ExtLatestVersionIdKey, so a bootstrap that lands
in the race window sees an older bare null with ExtVersionIdKey="null"
plus a newer .versions/<v-new> child and no parent pointer. Promoting
the null misclassifies v-new as noncurrent and skips both the new
version's current-version expiration and the null's noncurrent
scheduling until the next bootstrap.
Constrain the explicit-null branch to items[0]: if the suspended-null
write is genuinely current it'll be the newest by mtime AND tagged.
Anything else falls through to the newest-sibling default.
Adds a regression test for the suspended->re-enabled race.
* fix(s3/lifecycle): paginate bootstrap directory listings
SeaweedList(..., limit=0) is a single-page request: the filer caps
limit=0 at DirListingLimit (1000 by default) and returns whatever fits
in one round trip. expandVersionsDir and walkBucketDir both relied on
that, so any directory bigger than the cap silently truncated. For
noncurrent retention this is correctness, not just scale — a hot key
with more versions than the cap had its rank/sort math computed off
the first page only, NumVersions, NoncurrentIndex, SuccessorModTime,
and the latest-fallback all wrong, with the older versions never
scheduled until a future bootstrap.
Add a listAll helper that drives pagination via StartFromFileName +
inclusive=false, looping until a page returns fewer entries than the
configured page size. Use it in both call sites. Page size is a var
(listPageSize, default 1024) so tests can shrink it without
generating thousands of entries.
The fake filer client now mirrors the real semantics: sort children
by name, honor StartFromFileName/InclusiveStartFrom, cap at Limit.
New regression tests force a small page size and assert the full
result set is processed and the call count matches what pagination
should drive.
* perf(s3/lifecycle): stream bucket walk in two passes instead of buffering
walkBucketDir was paginating into a children slice and then iterating
twice (pass 1: .versions/, pass 2: everything else). For flat buckets
with millions of entries the buffer is a real memory spike. Drop the
materialization: each pass now drives its own listAll over the same
directory and acts on entries as they stream in. The skipBare ordering
contract is preserved — pass 2 still runs after pass 1 finishes — and
the per-pass paging keeps memory bounded by listPageSize.
Tradeoff: each directory level is listed twice. For workloads where
that matters more than the memory headroom, we can revisit; the
correctness/scale dial here is what the noncurrent rules need.
Updated three tests for the new call count: each walk now records 2
listings per directory (pass 1 + pass 2). The KickOffNew dedup tests
expect 2 calls per bucket; the pagination test expects 6 instead of 3.
|
||
|
Chris LuandGitHub
|
1c917ffacb |
fix(volume): sticky EIO quarantine; track streamed reads (#9384)
Two follow-ups on PR #9382: 1. Quarantine wasn't sticky. Once CollectHeartbeat crossed the streak threshold and hid the replica, a subsequent successful read called checkReadWriteError(nil), wiping the streak; the next heartbeat then re-announced the suspect replica as read-only and master could send reads back to a disk that already failed IoErrorTolerance. Added an ioErrorQuarantined sticky flag set on the first heartbeat that observes the threshold and cleared only by MarkVolumeWritable (resetIoErrorState). clearIoError continues to reset just the streak so successful ops don't accumulate phantom errors. 2. Streamed reads bypassed the EIO counter. readNeedleDataInto and ReadNeedleBlob — the hot paths for large/range GETs — returned ReadNeedleData / needle.ReadNeedleBlob errors without threading them through checkReadWriteError, so a disk failing only on those paths would never trip IoErrorTolerance. Both now route the backend error through the tracker, and a fully clean readNeedleDataInto call clears the streak. Tests cover the sticky flag (TestQuarantineIsSticky) and the streamed read path (TestReadNeedleBlobTracksEIO via a fake EIO backend). |
||
|
Chris LuandGitHub
|
7c60407897 |
fix(volume): don't nuke local data on transient IO error (#9378) (#9382)
* fix(volume): don't nuke local data on transient IO error (#9378) A single syscall.EIO from any read/write/delete set v.lastIoError, and the next CollectHeartbeat then called Volume.Destroy on the replica — removing the .dat/.idx/.vif/.sdx/.ldb/.rdb files. A brief NFS / fabric / controller blip hitting several replicas at once could cascade into removal of the last healthy copy, with no recovery for non-tiered volumes. Now require IoErrorTolerance (3) consecutive EIOs before acting, and on that threshold mark the volume read-only and stop announcing it to the master so re-replication kicks in from healthy peers — never delete the data files. The on-disk copy stays for operator inspection / recovery. * review: fix race, accounting, recovery, non-EIO streak break Addressing PR #9382 review: - Data race on lastIoError: guard lastIoError + lastIoErrorCount with a RWMutex and expose them through note/clear/get helpers so the heartbeat reader sees a consistent snapshot. Verified with -race. - Collection-size accounting: when a volume is quarantined for sustained EIO, skip the entire per-volume bookkeeping (`continue`) instead of flipping shouldDeleteVolume — the old branch subtracted a size that was never added, dragging the collection gauge to zero / negative. - Recoverability: MarkVolumeWritable now also calls clearIoError so an operator can rejoin a quarantined replica. The next failed op re-arms the streak if the disk is still bad. - Non-EIO streak break: a non-EIO error (e.g. ENOSPC) now resets the consecutive-EIO counter, so a sequence EIO,EIO,ENOSPC,EIO is treated as a streak of one — the counter only tracks consecutive EIOs. Reads already call checkReadWriteError (volume_read.go), so successful reads also clear the streak — no change needed there. |