Skip to content

Conversation

SmoothDenis
Copy link
Contributor

@SmoothDenis SmoothDenis commented May 11, 2025

What problem are we solving?

  1. Bucket deletion: deleting a single S3 bucket removed all buckets (top-level entries) due to incorrect path extraction, problem with empty ls s3:// response ater restart filer
  2. Cleanup: empty directories remained in per-bucket YDB schema after bucket deletion
  3. Pagination hang: listing directories with more than 1000 entries hung because YDB limited pages to 1000 rows (seaweed filer operating with 1024 entries requests)
  4. Session recovery: PUT failed on YDB BAD_SESSION in cases with transport errors because of losted sessions
  5. useBucketPrefix=false: unwanted per-bucket tables and directories were created even when bucket prefixing was disabled
  6. Incorrect creation path to filemeta tables due to MakeRecursive, so I implemented ensureTables

How are we solving the problem?

  1. Path fixes in DeleteFolderChildren and OnBucketDeletion, so only the target bucket is removed from main table with buckets list
  2. Directory removal in OnBucketDeletion call RemoveDirectory when SupportBucketTable=true to clean up empty schema folders related to that bucket
  3. Chunked pagination fix in ListDirectoryPrefixedEntries, added chunkLimit (max 1000) and pass that to YDB, avoiding 1000+ requests in one call
  4. Idempotent retries in doTxOrDB to wrap non-transactional calls in Table().Do with table.WithIdempotent(). It should be safe, but I might missing some details
  5. Added early return in OnBucketCreation and OnBucketDeletion when SupportBucketTable is false, so no per-bucket artifacts are created in that mode
  6. Look at ensureTables

How is the PR tested?

I tested all the scenarios described above manually, as there are currently no tests that would check:

  • listing over 1000 files
  • deleting a bucket and checking ls s3://
  • checking YDB structures in different modes
  • simulation of network problems, restarts, and other problems

I also don't quite understand the purpose of the "useBucketPrefix=false" mode. However, I tried to preserve its functionality, restore it if possible and not disrupt it with new changes.

Checks

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

@chrislusf
Copy link
Collaborator

Thanks for the fixes! There is a test build error to address.

@SmoothDenis
Copy link
Contributor Author

I will need to check a couple more things, then I will return the request from the draft

@SmoothDenis SmoothDenis marked this pull request as draft May 11, 2025 19:02
@SmoothDenis SmoothDenis marked this pull request as ready for review May 12, 2025 07:56
@SmoothDenis
Copy link
Contributor Author

@chrislusf @kmlebedev look at the changes, please

@chrislusf chrislusf merged commit 45964c2 into seaweedfs:master May 12, 2025
7 checks passed
@SmoothDenis SmoothDenis deleted the fix/ydb-list branch May 13, 2025 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants