diff --git a/weed/s3api/s3api_internal_lifecycle.go b/weed/s3api/s3api_internal_lifecycle.go index 6199bf7b0..85167953d 100644 --- a/weed/s3api/s3api_internal_lifecycle.go +++ b/weed/s3api/s3api_internal_lifecycle.go @@ -56,12 +56,7 @@ func (s3a *S3ApiServer) LifecycleDelete(ctx context.Context, req *s3_lifecycle_p } func (s3a *S3ApiServer) lifecycleDispatch(ctx context.Context, req *s3_lifecycle_pb.LifecycleDeleteRequest, entry *filer_pb.Entry) (*s3_lifecycle_pb.LifecycleDeleteResponse, error) { - // metadataOnly: skip per-chunk DeleteFile RPCs because the volume's TTL - // will reclaim chunks on its own. Per-write TTL stamping (PR 9377) sets - // Attributes.TtlSec on every entry whose lifecycle rule fits within - // volume TTL — observing a non-zero TtlSec on the live entry is the - // authoritative signal. - metadataOnly := entry != nil && entry.Attributes != nil && entry.Attributes.TtlSec > 0 + metadataOnly := entryUsesMetadataOnlyDelete(entry) switch req.ActionKind { case s3_lifecycle_pb.ActionKind_EXPIRATION_DAYS, s3_lifecycle_pb.ActionKind_EXPIRATION_DATE: // Current-version expiration: Enabled -> delete marker; Suspended @@ -347,6 +342,17 @@ func identityMatches(live, want *s3_lifecycle_pb.EntryIdentity) bool { return bytes.Equal(live.ExtendedHash, want.ExtendedHash) } +// entryUsesMetadataOnlyDelete reports whether the lifecycle delete path +// can skip per-chunk DeleteFile RPCs and rely on the volume's TTL to +// reclaim chunks. Per-write TTL stamping (PR 9377) sets Attributes.TtlSec +// on every entry whose lifecycle rule fits within volume TTL — observing +// a non-zero TtlSec on the live entry is the authoritative signal. +// Defensive nil-checks because the caller may be racing a concurrent +// rewrite that nil-ed Attributes briefly during meta-log replay. +func entryUsesMetadataOnlyDelete(entry *filer_pb.Entry) bool { + return entry != nil && entry.Attributes != nil && entry.Attributes.TtlSec > 0 +} + // recordMetadataOnlyIf bumps the metadata-only counter when on=true. // Skipped when off so callers don't need a guard at every call site. // rule_hash is hex-encoded so operators can group by rule when diff --git a/weed/s3api/s3api_internal_lifecycle_test.go b/weed/s3api/s3api_internal_lifecycle_test.go index 739ac65c2..6a8cbeb7c 100644 --- a/weed/s3api/s3api_internal_lifecycle_test.go +++ b/weed/s3api/s3api_internal_lifecycle_test.go @@ -241,6 +241,47 @@ func contains(haystack, needle string) bool { return false } +func TestEntryUsesMetadataOnlyDelete(t *testing.T) { + cases := []struct { + name string + entry *filer_pb.Entry + want bool + }{ + { + name: "nil entry", + entry: nil, + want: false, + }, + { + name: "nil attributes", + entry: &filer_pb.Entry{}, + want: false, + }, + { + name: "TtlSec=0 (no per-write stamp)", + entry: &filer_pb.Entry{Attributes: &filer_pb.FuseAttributes{TtlSec: 0}}, + want: false, + }, + { + name: "TtlSec>0 (PR 9377 stamped a fast-path TTL)", + entry: &filer_pb.Entry{Attributes: &filer_pb.FuseAttributes{TtlSec: 86400}}, + want: true, + }, + { + name: "TtlSec<0 should not happen but must not flip the path on", + entry: &filer_pb.Entry{Attributes: &filer_pb.FuseAttributes{TtlSec: -1}}, + want: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := entryUsesMetadataOnlyDelete(c.entry); got != c.want { + t.Fatalf("want %v, got %v", c.want, got) + } + }) + } +} + func TestRecordMetadataOnlyIf_OnlyFiresWhenOn(t *testing.T) { // Counter must increment exactly once per (bucket, hex(rule_hash)) // when on=true, and not at all when on=false. Other lifecycle paths