seaweedfs icon indicating copy to clipboard operation
seaweedfs copied to clipboard

do delete expired entries on s3 list request

Open kmlebedev opened this issue 5 months ago โ€ข 2 comments

What problem are we solving?

https://github.com/seaweedfs/seaweedfs/issues/6837 In S3, objects must be deleted based on their modification time.

How are we solving the problem?

Disable deletion by creation time on the filler side and enable deletion in s3

How is the PR tested?

I'll finish writing

Checks

  • [ ] I have added unit tests if possible.
  • [ ] I will add related wiki document changes and link to this PR after merging.

Summary by CodeRabbit

  • New Features
    • Configurable S3 TTL-based object deletion; writes and lifecycle changes now tag and honor S3 expiry so existing objects can be updated for TTL behavior.
  • Tests
    • CI tests expanded to cover lifecycle expiration and TTL-deletion scenarios; test tooling builds updated to enable S3 test coverage.
  • Chores
    • Upgraded several dependencies (AWS SDK and related modules).

kmlebedev avatar Nov 03 '25 07:11 kmlebedev

[!NOTE]

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds S3-specific TTL/expiry support across S3 API, filer, multipart, and lifecycle handling: new header/constant and extended keys, entry/protobuf expiry helpers, S3-aware expiry checks and deferred deletions, iterative lifecycle TTL propagation, multipart TTL propagation and upload-time TTL suppression, build-tagged lifecycle interval variants, and CI s3tests tag adjustments.

Changes

Cohort / File(s) Summary
CI / Tests
\.github/workflows/s3tests.yml
Add -tags s3tests to go install, use -tags "sqlite s3tests" for SQL-store job, and include lifecycle expiration tests in the run list.
S3 headers & extend keys
weed/s3api/s3_constants/header.go, weed/s3api/s3_constants/extend_key.go
Add SeaweedFSExpiresS3 header constant ("X-Seaweedfs-Expires-S3") and ExtMultipartObjectKey extended-key constant ("key").
Filer entry helpers
weed/filer/entry.go, weed/pb/filer_pb/filer_pb_helper.go
Add Entry helpers: IsExpireS3Enabled(), IsS3Versioning(), GetS3ExpireTime() and protobuf helpers GetExpiryTime() / IsExpired() for TTL/expiry calculations.
Filer expiry & listing cleanup
weed/filer/filer.go
Use S3 expiry time when enabled in FindEntry and listings; collect expired entries during listing and perform post-iteration deletions to avoid DB deadlocks; add DeleteEmptyParentDirectories, IsDirectoryEmpty, and call Store.Shutdown() in Shutdown.
Delete/empty-directory orchestration
weed/s3api/s3api_object_handlers_delete.go, weed/pb/filer_pb/filer_client.go
Replace inline per-object directory cleanup with recursive DoDeleteEmptyParentDirectories; use non-cancellable opCtx for cleanup, track directories with map[string]bool, sort deepest-first for batched cleanup, and remove old traversal helper.
S3 lifecycle TTL propagation
weed/s3api/s3api_bucket_handlers.go, weed/s3api/filer_util.go
PutBucketLifecycleConfiguration computes TTL and calls new updateEntriesTTL() which iteratively traverses directories in batches, updates Attributes.TtlSec and sets Extended[SeaweedFSExpiresS3], and aggregates errors.
Multipart upload TTL handling
weed/s3api/filer_multipart.go
Store multipart object key under ExtMultipartObjectKey; detect TTL among parts and set SeaweedFSExpiresS3="true" on final non-versioned object; exclude multipart-key when copying part metadata.
Put/upload behavior changes
weed/s3api/s3api_object_handlers_put.go, weed/server/filer_server_handlers_write_autochunk.go
Propagate X-Seaweedfs-Expires-S3 on proxied puts; when header present, use TTL-less StorageOption for upload and persist TTL flag into entry Extended in saveMetaData().
Lifecycle interval build variants
weed/util/constants_lifecycle_interval_10sec.go, weed/util/constants_lifecycle_interval_day.go
Add exported LifeCycleInterval constant: 10s variant under s3tests build tag and 24h default variant.
Minor formatting
weed/s3api/s3api_object_handlers.go
Remove stray blank lines (formatting only).
Test module dependencies
test/kafka/go.mod
Bump multiple AWS SDK v2 and related modules; add gorilla/mux indirect; update rclone and go-mega versions.

Sequence Diagram(s)

%%{init: {"themeVariables":{"actorBackground":"#f4f7f4","signalColor":"#a7d7ff","noteBackground":"#fff7e6"}}}%%
sequenceDiagram
    participant Client
    participant S3API
    participant Filer
    participant Store

    Client->>S3API: PutObject (+ optional X-Seaweedfs-Expires-S3)
    S3API->>S3API: forward header on proxied put
    S3API->>Filer: putToFiler (proxy request)
    Filer->>Filer: inspect Entry.IsExpireS3Enabled()/IsS3Versioning()
    alt S3 expiry header present
        Filer->>Store: upload with TTL-less StorageOption
        Filer->>Filer: set Extended[SeaweedFSExpiresS3] = "true"
    else default TTL path
        Filer->>Store: upload with Attributes.TtlSec
    end

    Client->>S3API: List / Get
    S3API->>Filer: FindEntry() / list directory
    Filer->>Filer: evaluate expiry using GetS3ExpireTime() or TTL fallback
    alt expired
        Filer->>Filer: collect expired entries
        Filer->>Store: delete expired entries (post-iteration)
        Filer-->>S3API: 404 Not Found
    else valid
        Filer-->>S3API: return entry
    end
    S3API-->>Client: Object or 404

Estimated code review effort

๐ŸŽฏ 4 (Complex) | โฑ๏ธ ~45 minutes

  • Inspect TTL precedence and time-source choices in GetS3ExpireTime / GetExpiryTime / IsExpired.
  • Verify deferred deletion batching in doListDirectoryEntries for deadlock avoidance and correctness.
  • Review updateEntriesTTL traversal, pagination, error aggregation, and nil-safety for Attributes/Extended.
  • Validate multipart completion changes: ExtMultipartObjectKey usage, metadata copying/exclusion, and final TTL propagation.
  • Check DoDeleteEmptyParentDirectories recursion, ancestry checks, and non-cancellable opCtx usage in delete handlers.
  • Ensure build-tagged LifeCycleInterval variants and CI s3tests tag changes compile in both modes.

Possibly related PRs

  • seaweedfs/seaweedfs#7402 โ€” modifies filer deletion logic and related filer internals; likely overlaps with empty-directory deletion/refactor work.

Poem

๐Ÿ‡ I hopped through branches, flags held high,
I tucked TTLs where S3 sighs.
Parts stitched tight, lifecycles hum along,
Empty folders pruned with a tidy song.
Meadow tidy, carrots saved โ€” the repo hops along.

Pre-merge checks and finishing touches

โŒ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check โš ๏ธ Warning The description is incomplete. The 'How is the PR tested?' section states 'I'll finish writing' without providing actual test details, and both checklist items remain unchecked despite being required. Complete the 'How is the PR tested?' section with concrete test details, add unit tests, and check off the checklist items before merging.
โœ… Passed checks (1 passed)
Check name Status Explanation
Title check โœ… Passed The title clearly summarizes the main change: enabling deletion of expired entries during S3 list operations, which aligns with the changeset's focus on S3-based TTL expiration.
โœจ Finishing touches
  • [ ] ๐Ÿ“ Generate docstrings
๐Ÿงช Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 03 '25 07:11 coderabbitai[bot]

@chrislusf Look at the PR

kmlebedev avatar Nov 04 '25 10:11 kmlebedev

============================= test session starts ==============================
platform linux -- Python 3.9.24, pytest-8.4.2, pluggy-1.6.0
cachedir: .tox/py/.pytest_cache
rootdir: /home/runner/work/seaweedfs/seaweedfs/s3-tests
configfile: pytest.ini
collected 180 items
s3tests/functional/test_s3.py .......................................... [ 23%]
........................................................................ [ 63%]

The test hangs. Ceph S3 tests / Basic S3 tests (SQL store) (pull_request)

chrislusf avatar Nov 05 '25 01:11 chrislusf

The test hangs @chrislusf hangs in this place f.Store.DeleteOneEntry(ctx, entry) gets stuck at this point because of the new test s3tests/functional/test_s3.py::test_lifecycle_expiration It looks like it wasn't working before.

https://github.com/seaweedfs/seaweedfs/blob/cc444b186849cc4e476d539dd2643058a8160534/weed/filer/filer.go#L371

goroutine 651 gp=0x14011e35c00 m=nil [select, 7 minutes]:
runtime.gopark(0x1401216ef48?, 0x2?, 0xf8?, 0xec?, 0x1401216ee58?)
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/runtime/proc.go:460 +0xc0 fp=0x1401216ecc0 sp=0x1401216eca0 pc=0x102fc0ba0
runtime.selectgo(0x1401216ef48, 0x1401216ee54, 0x1401216ee98?, 0x0, 0x1?, 0x1)
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/runtime/select.go:351 +0x670 fp=0x1401216ee00 sp=0x1401216ecc0 pc=0x102f9ed30
database/sql.(*DB).conn(0x14011bec680, {0x1075f88e0, 0x14022f82df0}, 0x1)
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/database/sql/sql.go:1369 +0x46c fp=0x1401216efb0 sp=0x1401216ee00 pc=0x1037aafbc
database/sql.(*DB).exec(0x14011bec680, {0x1075f88e0, 0x14022f82df0}, {0x14000f22af0, 0x64}, {0x14011cf6b10, 0x3, 0x3}, 0xc8?)
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/database/sql/sql.go:1689 +0x40 fp=0x1401216f020 sp=0x1401216efb0 pc=0x1037acb20
database/sql.(*DB).ExecContext.func1(0xb8?)
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/database/sql/sql.go:1672 +0x40 fp=0x1401216f090 sp=0x1401216f020 pc=0x1037ac9f0
database/sql.(*DB).retry(0x20016f110?, 0x1401216f120)
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/database/sql/sql.go:1576 +0x4c fp=0x1401216f0e0 sp=0x1401216f090 pc=0x1037ac2cc
database/sql.(*DB).ExecContext(0x14000fc85e8?, {0x1075f88e0?, 0x14022f82df0?}, {0x14000f22af0?, 0x34?}, {0x14011cf6b10?, 0x102fca560?, 0x1401216f1f8?})
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/database/sql/sql.go:1671 +0x80 fp=0x1401216f180 sp=0x1401216f0e0 pc=0x1037ac950
github.com/seaweedfs/seaweedfs/weed/filer/abstract_sql.(*AbstractSqlStore).DeleteEntry(0x14011b93890, {0x1075f88e0, 0x14022f82df0}, {0x14000fc85c0, 0x34})
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/abstract_sql/abstract_sql_store.go:250 +0x1e4 fp=0x1401216f260 sp=0x1401216f180 pc=0x103e92954
github.com/seaweedfs/seaweedfs/weed/filer.(*FilerStoreWrapper).DeleteOneEntry(0x14011b93a10, {0x1075f8d38?, 0x14011cf6840?}, 0x14011fa0a00)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filerstore_wrapper.go:231 +0x1e4 fp=0x1401216f360 sp=0x1401216f260 pc=0x103a1ded4
github.com/seaweedfs/seaweedfs/weed/filer.(*Filer).doListDirectoryEntries.func1(0x14011fa0a00)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filer.go:388 +0x280 fp=0x1401216f450 sp=0x1401216f360 pc=0x103a10400
github.com/seaweedfs/seaweedfs/weed/filer.(*FilerStoreWrapper).ListDirectoryPrefixedEntries.func2(0x14011fa0a00)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filerstore_wrapper.go:279 +0x54 fp=0x1401216f490 sp=0x1401216f450 pc=0x103a1ea24
github.com/seaweedfs/seaweedfs/weed/filer/abstract_sql.(*AbstractSqlStore).ListDirectoryPrefixedEntries(0x14011b93890, {0x1075f88e0, 0x14022f82cf0}, {0x140121c2d50, 0x30}, {0x0, 0x0}, 0x0, 0x3ea, {0x0, ...}, ...)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/abstract_sql/abstract_sql_store.go:329 +0x444 fp=0x1401216f5c0 sp=0x1401216f490 pc=0x103e933a4
github.com/seaweedfs/seaweedfs/weed/filer.(*FilerStoreWrapper).ListDirectoryPrefixedEntries(0x14011b93a10, {0x1075f8d38?, 0x14011cf6840?}, {0x140121c2d50, 0x30}, {0x0, 0x0}, 0x0, 0x3ea, {0x0, ...}, ...)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filerstore_wrapper.go:281 +0x1e4 fp=0x1401216f710 sp=0x1401216f5c0 pc=0x103a1e8b4
github.com/seaweedfs/seaweedfs/weed/filer.(*Filer).doListDirectoryEntries(0x14000cae6c0, {0x1075f8d38, 0x14011cf6840}, {0x140121c2d50, 0x30}, {0x0, 0x0}, 0x0, 0x3ea, {0x0, ...}, ...)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filer.go:372 +0xf4 fp=0x1401216f790 sp=0x1401216f710 pc=0x103a100e4
github.com/seaweedfs/seaweedfs/weed/filer.(*Filer).doListValidEntries(0x14000cae6c0, {0x1075f8d38, 0x14011cf6840}, {0x140121c2d50, 0x30}, {0x0?, 0x14011bb91d0?}, 0x1?, 0x140010e7878?, {0x0, ...}, ...)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filer_search.go:98 +0x40 fp=0x1401216f800 sp=0x1401216f790 pc=0x103a1a970
github.com/seaweedfs/seaweedfs/weed/filer.(*Filer).doListPatternMatchedEntries(0x14000cae6c0, {0x1075f8d38, 0x14011cf6840}, {0x140121c2d50, 0x30}, {0x0, 0x0}, 0x0, 0x3ea, {0x0, ...}, ...)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filer_search.go:67 +0xac fp=0x1401216f880 sp=0x1401216f800 pc=0x103a1a64c
github.com/seaweedfs/seaweedfs/weed/filer.(*Filer).StreamListDirectoryEntries(0x14000cae6c0, {0x1075f8d38, 0x14011cf6840}, {0x140121c2d50, 0x30?}, {0x0, 0x0}, 0x0, 0x3ea, {0x0, ...}, ...)
        /Users/whitefox/GolandProjects/seaweedfs/weed/filer/filer_search.go:55 +0x148 fp=0x1401216f940 sp=0x1401216f880 pc=0x103a1a468
github.com/seaweedfs/seaweedfs/weed/server.(*FilerServer).ListEntries(0x14000a3d400, 0x14000ae6fc0, {0x1076156c0, 0x14022f82cd0})
        /Users/whitefox/GolandProjects/seaweedfs/weed/server/filer_grpc_server.go:59 +0x1e0 fp=0x1401216fa30 sp=0x1401216f940 pc=0x10494b260
github.com/seaweedfs/seaweedfs/weed/pb/filer_pb._SeaweedFiler_ListEntries_Handler({0x107550120, 0x14000a3d400}, {0x10760e348, 0x14011bb91d0})
        /Users/whitefox/GolandProjects/seaweedfs/weed/pb/filer_pb/filer_grpc.pb.go:543 +0x11c fp=0x1401216fa70 sp=0x1401216fa30 pc=0x10384e09c
google.golang.org/grpc.(*Server).processStreamingRPC(0x14011cb4000, {0x1075f8d38, 0x14011cf6600}, 0x14000ae6f00, 0x14011ed7020, 0x109c078c0, 0x0)
        /Users/whitefox/go/pkg/mod/google.golang.org/[email protected]/server.go:1722 +0xde8 fp=0x1401216fd80 sp=0x1401216fa70 pc=0x10373bf08
google.golang.org/grpc.(*Server).handleStream(0x14011cb4000, {0x1075fac18, 0x140009791e0}, 0x14000ae6f00)
        /Users/whitefox/go/pkg/mod/google.golang.org/[email protected]/server.go:1846 +0x82c fp=0x1401216ff60 sp=0x1401216fd80 pc=0x10373d2fc
google.golang.org/grpc.(*Server).serveStreams.func2.1()
        /Users/whitefox/go/pkg/mod/google.golang.org/[email protected]/server.go:1061 +0x74 fp=0x1401216ffd0 sp=0x1401216ff60 pc=0x103737564
runtime.goexit({})
        /Users/whitefox/go/pkg/mod/golang.org/[email protected]/src/runtime/asm_arm64.s:1268 +0x4 fp=0x1401216ffd0 sp=0x1401216ffd0 pc=0x102fc9624
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 477
        /Users/whitefox/go/pkg/mod/google.golang.org/[email protected]/server.go:1072 +0x120

kmlebedev avatar Nov 05 '25 11:11 kmlebedev

@chrislusf Hi, I haven't quite figured out how the queue works and where fileIdDeletionQueue is used, but I'd like to switch the deletion above to background deletion. Can I use the queue above for this?

kmlebedev avatar Nov 05 '25 17:11 kmlebedev

/gemini review

chrislusf avatar Nov 05 '25 20:11 chrislusf

/gemini review

chrislusf avatar Nov 05 '25 21:11 chrislusf

/gemini review

chrislusf avatar Nov 05 '25 21:11 chrislusf

/gemini review

chrislusf avatar Nov 05 '25 22:11 chrislusf

/gemini review

chrislusf avatar Nov 05 '25 22:11 chrislusf

/gemini review

chrislusf avatar Nov 05 '25 23:11 chrislusf

/gemini review

chrislusf avatar Nov 06 '25 00:11 chrislusf