Files
seaweedfs/weed/s3api/s3lifecycle/dailyrun/process_matches_test.go
T
Chris Lu 122ca7c020 feat(s3/lifecycle): daily-replay worker behind algorithm flag (Phase 2) (#9446)
* docs(s3lifecycle): design for daily-replay worker

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Addresses PR #9446 review feedback. Eight distinct fixes:

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

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

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

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

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

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

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

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

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

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

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

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

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

Full build clean. Dailyrun + worker test packages green.
2026-05-11 18:07:17 -07:00

153 lines
6.0 KiB
Go

package dailyrun
import (
"context"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/seaweedfs/seaweedfs/weed/pb/s3_lifecycle_pb"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle/reader"
"github.com/seaweedfs/seaweedfs/weed/s3api/s3lifecycle/router"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// recordingClient captures every LifecycleDelete request the
// daily-run path emits. Default outcome is DONE; tests that need
// other outcomes set responses by index.
type recordingClient struct {
mu sync.Mutex
requests []*s3_lifecycle_pb.LifecycleDeleteRequest
responses []s3_lifecycle_pb.LifecycleDeleteOutcome
calls atomic.Int32
}
func (c *recordingClient) LifecycleDelete(_ context.Context, req *s3_lifecycle_pb.LifecycleDeleteRequest) (*s3_lifecycle_pb.LifecycleDeleteResponse, error) {
c.mu.Lock()
c.requests = append(c.requests, req)
idx := int(c.calls.Add(1)) - 1
c.mu.Unlock()
out := s3_lifecycle_pb.LifecycleDeleteOutcome_DONE
if idx < len(c.responses) {
out = c.responses[idx]
}
return &s3_lifecycle_pb.LifecycleDeleteResponse{Outcome: out}, nil
}
func (c *recordingClient) seenObjects() []string {
c.mu.Lock()
defer c.mu.Unlock()
out := make([]string, 0, len(c.requests))
for _, r := range c.requests {
out = append(out, r.ObjectPath)
}
return out
}
// Phase 2's processMatches must NOT use a per-rule done flag.
// routePointerTransitionExpand can return multiple matches for the
// same ActionKey on different objects, each with its own
// SuccessorModTime from a distinct demoting event. A not-yet-due
// sibling must never gate a sibling that's already past its DueTime.
// Pin that here.
func TestProcessMatches_NotYetDueDoesNotSuppressDueOnSameRule(t *testing.T) {
runNow := time.Now()
rh := [8]byte{1, 2, 3, 4, 5, 6, 7, 8}
rule := s3lifecycle.ActionKey{
Bucket: "b",
RuleHash: rh,
ActionKind: s3lifecycle.ActionKindNoncurrentDays,
}
// Three matches for the same ActionKey, different objects, mixed
// due-times. Order intentionally puts the not-yet-due match first
// so a buggy per-rule done flag would suppress the others.
matches := []router.Match{
{Key: rule, Bucket: "b", ObjectKey: "future-sibling", DueTime: runNow.Add(48 * time.Hour)},
{Key: rule, Bucket: "b", ObjectKey: "due-sibling-a", DueTime: runNow.Add(-48 * time.Hour)},
{Key: rule, Bucket: "b", ObjectKey: "due-sibling-b", DueTime: runNow.Add(-24 * time.Hour)},
}
client := &recordingClient{}
cfg := Config{Client: client}
skipped, halted, err := processMatches(context.Background(), cfg, runNow, &reader.Event{TsNs: runNow.UnixNano()}, matches)
require.NoError(t, err)
require.False(t, halted)
require.True(t, skipped, "at least one future-DueTime match must flag skippedAny")
// Both due siblings must have been dispatched.
seen := client.seenObjects()
assert.ElementsMatch(t, []string{"due-sibling-a", "due-sibling-b"}, seen,
"due matches must dispatch regardless of where the future-sibling sits in the slice")
}
func TestProcessMatches_OrderingDoesNotMatter(t *testing.T) {
runNow := time.Now()
rh := [8]byte{0xaa}
rule := s3lifecycle.ActionKey{Bucket: "b", RuleHash: rh, ActionKind: s3lifecycle.ActionKindExpirationDays}
matches := []router.Match{
{Key: rule, Bucket: "b", ObjectKey: "a", DueTime: runNow.Add(-2 * time.Hour)},
{Key: rule, Bucket: "b", ObjectKey: "b", DueTime: runNow.Add(-1 * time.Hour)},
{Key: rule, Bucket: "b", ObjectKey: "future", DueTime: runNow.Add(time.Hour)},
}
client := &recordingClient{}
cfg := Config{Client: client}
skipped, halted, err := processMatches(context.Background(), cfg, runNow, &reader.Event{}, matches)
require.NoError(t, err)
require.False(t, halted)
require.True(t, skipped)
assert.ElementsMatch(t, []string{"a", "b"}, client.seenObjects())
}
func TestProcessMatches_HaltOnServerOutcomeStopsRemaining(t *testing.T) {
runNow := time.Now()
rh := [8]byte{0xbb}
rule := s3lifecycle.ActionKey{Bucket: "b", RuleHash: rh, ActionKind: s3lifecycle.ActionKindExpirationDays}
matches := []router.Match{
{Key: rule, Bucket: "b", ObjectKey: "ok", DueTime: runNow.Add(-time.Hour)},
{Key: rule, Bucket: "b", ObjectKey: "blocked", DueTime: runNow.Add(-time.Hour)},
{Key: rule, Bucket: "b", ObjectKey: "would-be-next", DueTime: runNow.Add(-time.Hour)},
}
client := &recordingClient{responses: []s3_lifecycle_pb.LifecycleDeleteOutcome{
s3_lifecycle_pb.LifecycleDeleteOutcome_DONE,
s3_lifecycle_pb.LifecycleDeleteOutcome_BLOCKED,
s3_lifecycle_pb.LifecycleDeleteOutcome_DONE, // would be a bug if reached
}}
cfg := Config{Client: client}
_, halted, err := processMatches(context.Background(), cfg, runNow, &reader.Event{}, matches)
require.NoError(t, err)
require.True(t, halted, "BLOCKED outcome must halt the event loop")
assert.Equal(t, []string{"ok", "blocked"}, client.seenObjects(),
"would-be-next must NOT be dispatched after the halt")
}
func TestProcessMatches_EmptyMatchesIsNoop(t *testing.T) {
runNow := time.Now()
client := &recordingClient{}
cfg := Config{Client: client}
skipped, halted, err := processMatches(context.Background(), cfg, runNow, &reader.Event{}, nil)
require.NoError(t, err)
require.False(t, halted)
require.False(t, skipped)
assert.Empty(t, client.seenObjects())
}
func TestProcessMatches_AllDueNoSkippedFlag(t *testing.T) {
// All matches past their DueTime — skippedAny must be false so the
// caller can advance the cursor past this event.
runNow := time.Now()
rh := [8]byte{0xcc}
rule := s3lifecycle.ActionKey{Bucket: "b", RuleHash: rh, ActionKind: s3lifecycle.ActionKindExpirationDays}
matches := []router.Match{
{Key: rule, Bucket: "b", ObjectKey: "a", DueTime: runNow.Add(-time.Hour)},
{Key: rule, Bucket: "b", ObjectKey: "b", DueTime: runNow.Add(-30 * time.Minute)},
}
client := &recordingClient{}
cfg := Config{Client: client}
skipped, halted, err := processMatches(context.Background(), cfg, runNow, &reader.Event{}, matches)
require.NoError(t, err)
require.False(t, halted)
require.False(t, skipped, "every match was past DueTime; skippedAny must be false")
}