* Refactor JWT generation for volume server authentication to use centralized function from filer package, improving code clarity and reducing redundancy.
* Update s3api_object_handlers.go
* add foundationdb
* Update foundationdb_store.go
* fix
* apply the patch
* avoid panic on error
* address comments
* remove extra data
* address comments
* adds more debug messages
* fix range listing
* delete with prefix range; list with right start key
* fix docker files
* use the more idiomatic FoundationDB KeySelectors
* address comments
* proper errors
* fix API versions
* more efficient
* recursive deletion
* clean up
* clean up
* pagination, one transaction for deletion
* error checking
* Use fdb.Strinc() to compute the lexicographically next string and create a proper range
* fix docker
* Update README.md
* delete in batches
* delete in batches
* fix build
* add foundationdb build
* Updated FoundationDB Version
* Fixed glibc/musl Incompatibility (Alpine → Debian)
* Update container_foundationdb_version.yml
* build SeaweedFS
* build tag
* address comments
* separate transaction
* address comments
* fix build
* empty vs no data
* fixes
* add go test
* Install FoundationDB client libraries
* nil compare
* Lazy Versioning Check, Conditional SSE Entry Fetch, HEAD Request Optimization
* revert
Reverted the conditional versioning check to always check versioning status
Reverted the conditional SSE entry fetch to always fetch entry metadata
Reverted the conditional versioning check to always check versioning status
Reverted the conditional SSE entry fetch to always fetch entry metadata
* Lazy Entry Fetch for SSE, Skip Conditional Header Check
* SSE-KMS headers are present, this is not an SSE-C request (mutually exclusive)
* SSE-C is mutually exclusive with SSE-S3 and SSE-KMS
* refactor
* Removed Premature Mutual Exclusivity Check
* check for the presence of the X-Amz-Server-Side-Encryption header
* not used
* fmt
* directly read write volume servers
* HTTP Range Request Support
* set header
* md5
* copy object
* fix sse
* fmt
* implement sse
* sse continue
* fixed the suffix range bug (bytes=-N for "last N bytes")
* debug logs
* Missing PartsCount Header
* profiling
* url encoding
* test_multipart_get_part
* headers
* debug
* adjust log level
* handle part number
* Update s3api_object_handlers.go
* nil safety
* set ModifiedTsNs
* remove
* nil check
* fix sse header
* same logic as filer
* decode values
* decode ivBase64
* s3: Fix SSE decryption JWT authentication and streaming errors
Critical fix for SSE (Server-Side Encryption) test failures:
1. **JWT Authentication Bug** (Root Cause):
- Changed from GenJwtForFilerServer to GenJwtForVolumeServer
- S3 API now uses correct JWT when directly reading from volume servers
- Matches filer's authentication pattern for direct volume access
- Fixes 'unexpected EOF' and 500 errors in SSE tests
2. **Streaming Error Handling**:
- Added error propagation in getEncryptedStreamFromVolumes goroutine
- Use CloseWithError() to properly communicate stream failures
- Added debug logging for streaming errors
3. **Response Header Timing**:
- Removed premature WriteHeader(http.StatusOK) call
- Let Go's http package write status automatically on first write
- Prevents header lock when errors occur during streaming
4. **Enhanced SSE Decryption Debugging**:
- Added IV/Key validation and logging for SSE-C, SSE-KMS, SSE-S3
- Better error messages for missing or invalid encryption metadata
- Added glog.V(2) debugging for decryption setup
This fixes SSE integration test failures where encrypted objects
could not be retrieved due to volume server authentication failures.
The JWT bug was causing volume servers to reject requests, resulting
in truncated/empty streams (EOF) or internal errors.
* s3: Fix SSE multipart upload metadata preservation
Critical fix for SSE multipart upload test failures (SSE-C and SSE-KMS):
**Root Cause - Incomplete SSE Metadata Copying**:
The old code only tried to copy 'SeaweedFSSSEKMSKey' from the first
part to the completed object. This had TWO bugs:
1. **Wrong Constant Name** (Key Mismatch Bug):
- Storage uses: SeaweedFSSSEKMSKeyHeader = 'X-SeaweedFS-SSE-KMS-Key'
- Old code read: SeaweedFSSSEKMSKey = 'x-seaweedfs-sse-kms-key'
- Result: SSE-KMS metadata was NEVER copied → 500 errors
2. **Missing SSE-C and SSE-S3 Headers**:
- SSE-C requires: IV, Algorithm, KeyMD5
- SSE-S3 requires: encrypted key data + standard headers
- Old code: copied nothing for SSE-C/SSE-S3 → decryption failures
**Fix - Complete SSE Header Preservation**:
Now copies ALL SSE headers from first part to completed object:
- SSE-C: SeaweedFSSSEIV, CustomerAlgorithm, CustomerKeyMD5
- SSE-KMS: SeaweedFSSSEKMSKeyHeader, AwsKmsKeyId, ServerSideEncryption
- SSE-S3: SeaweedFSSSES3Key, ServerSideEncryption
Applied consistently to all 3 code paths:
1. Versioned buckets (creates version file)
2. Suspended versioning (creates main object with null versionId)
3. Non-versioned buckets (creates main object)
**Why This Is Correct**:
The headers copied EXACTLY match what putToFiler stores during part
upload (lines 496-521 in s3api_object_handlers_put.go). This ensures
detectPrimarySSEType() can correctly identify encrypted multipart
objects and trigger inline decryption with proper metadata.
Fixes: TestSSEMultipartUploadIntegration (SSE-C and SSE-KMS subtests)
* s3: Add debug logging for versioning state diagnosis
Temporary debug logging to diagnose test_versioning_obj_plain_null_version_overwrite_suspended failure.
Added glog.V(0) logging to show:
1. setBucketVersioningStatus: when versioning status is changed
2. PutObjectHandler: what versioning state is detected (Enabled/Suspended/none)
3. PutObjectHandler: which code path is taken (putVersionedObject vs putSuspendedVersioningObject)
This will help identify if:
- The versioning status is being set correctly in bucket config
- The cache is returning stale/incorrect versioning state
- The switch statement is correctly routing to suspended vs enabled handlers
* s3: Enhanced versioning state tracing for suspended versioning diagnosis
Added comprehensive logging across the entire versioning state flow:
PutBucketVersioningHandler:
- Log requested status (Enabled/Suspended)
- Log when calling setBucketVersioningStatus
- Log success/failure of status change
setBucketVersioningStatus:
- Log bucket and status being set
- Log when config is updated
- Log completion with error code
updateBucketConfig:
- Log versioning state being written to cache
- Immediate cache verification after Set
- Log if cache verification fails
getVersioningState:
- Log bucket name and state being returned
- Log if object lock forces VersioningEnabled
- Log errors
This will reveal:
1. If PutBucketVersioning(Suspended) is reaching the handler
2. If the cache update succeeds
3. What state getVersioningState returns during PUT
4. Any cache consistency issues
Expected to show why bucket still reports 'Enabled' after 'Suspended' call.
* s3: Add SSE chunk detection debugging for multipart uploads
Added comprehensive logging to diagnose why TestSSEMultipartUploadIntegration fails:
detectPrimarySSEType now logs:
1. Total chunk count and extended header count
2. All extended headers with 'sse'/'SSE'/'encryption' in the name
3. For each chunk: index, SseType, and whether it has metadata
4. Final SSE type counts (SSE-C, SSE-KMS, SSE-S3)
This will reveal if:
- Chunks are missing SSE metadata after multipart completion
- Extended headers are copied correctly from first part
- The SSE detection logic is working correctly
Expected to show if chunks have SseType=0 (none) or proper SSE types set.
* s3: Trace SSE chunk metadata through multipart completion and retrieval
Added end-to-end logging to track SSE chunk metadata lifecycle:
**During Multipart Completion (filer_multipart.go)**:
1. Log finalParts chunks BEFORE mkFile - shows SseType and metadata
2. Log versionEntry.Chunks INSIDE mkFile callback - shows if mkFile preserves SSE info
3. Log success after mkFile completes
**During GET Retrieval (s3api_object_handlers.go)**:
1. Log retrieved entry chunks - shows SseType and metadata after retrieval
2. Log detected SSE type result
This will reveal at which point SSE chunk metadata is lost:
- If finalParts have SSE metadata but versionEntry.Chunks don't → mkFile bug
- If versionEntry.Chunks have SSE metadata but retrieved chunks don't → storage/retrieval bug
- If chunks never have SSE metadata → multipart completion SSE processing bug
Expected to show chunks with SseType=NONE during retrieval even though
they were created with proper SseType during multipart completion.
* s3: Fix SSE-C multipart IV base64 decoding bug
**Critical Bug Found**: SSE-C multipart uploads were failing because:
Root Cause:
- entry.Extended[SeaweedFSSSEIV] stores base64-encoded IV (24 bytes for 16-byte IV)
- SerializeSSECMetadata expects raw IV bytes (16 bytes)
- During multipart completion, we were passing base64 IV directly → serialization error
Error Message:
"Failed to serialize SSE-C metadata for chunk in part X: invalid IV length: expected 16 bytes, got 24"
Fix:
- Base64-decode IV before passing to SerializeSSECMetadata
- Added error handling for decode failures
Impact:
- SSE-C multipart uploads will now correctly serialize chunk metadata
- Chunks will have proper SSE metadata for decryption during GET
This fixes the SSE-C subtest of TestSSEMultipartUploadIntegration.
SSE-KMS still has a separate issue (error code 23) being investigated.
* fixes
* kms sse
* handle retry if not found in .versions folder and should read the normal object
* quick check (no retries) to see if the .versions/ directory exists
* skip retry if object is not found
* explicit update to avoid sync delay
* fix map update lock
* Remove fmt.Printf debug statements
* Fix SSE-KMS multipart base IV fallback to fail instead of regenerating
* fmt
* Fix ACL grants storage logic
* header handling
* nil handling
* range read for sse content
* test range requests for sse objects
* fmt
* unused code
* upload in chunks
* header case
* fix url
* bucket policy error vs bucket not found
* jwt handling
* fmt
* jwt in request header
* Optimize Case-Insensitive Prefix Check
* dead code
* Eliminated Unnecessary Stream Prefetch for Multipart SSE
* range sse
* sse
* refactor
* context
* fmt
* fix type
* fix SSE-C IV Mismatch
* Fix Headers Being Set After WriteHeader
* fix url parsing
* propergate sse headers
* multipart sse-s3
* aws sig v4 authen
* sse kms
* set content range
* better errors
* Update s3api_object_handlers_copy.go
* Update s3api_object_handlers.go
* Update s3api_object_handlers.go
* avoid magic number
* clean up
* Update s3api_bucket_policy_handlers.go
* fix url parsing
* context
* data and metadata both use background context
* adjust the offset
* SSE Range Request IV Calculation
* adjust logs
* IV relative to offset in each part, not the whole file
* collect logs
* offset
* fix offset
* fix url
* logs
* variable
* jwt
* Multipart ETag semantics: conditionally set object-level Md5 for single-chunk uploads only.
* sse
* adjust IV and offset
* multipart boundaries
* ensures PUT and GET operations return consistent ETags
* Metadata Header Case
* CommonPrefixes Sorting with URL Encoding
* always sort
* remove the extra PathUnescape call
* fix the multipart get part ETag
* the FileChunk is created without setting ModifiedTsNs
* Sort CommonPrefixes lexicographically to match AWS S3 behavior
* set md5 for multipart uploads
* prevents any potential data loss or corruption in the small-file inline storage path
* compiles correctly
* decryptedReader will now be properly closed after use
* Fixed URL encoding and sort order for CommonPrefixes
* Update s3api_object_handlers_list.go
* SSE-x Chunk View Decryption
* Different IV offset calculations for single-part vs multipart objects
* still too verbose in logs
* less logs
* ensure correct conversion
* fix listing
* nil check
* minor fixes
* nil check
* single character delimiter
* optimize
* range on empty object or zero-length
* correct IV based on its position within that part, not its position in the entire object
* adjust offset
* offset
Fetch FULL encrypted chunk (not just the range)
Adjust IV by PartOffset/ChunkOffset only
Decrypt full chunk
Skip in the DECRYPTED stream to reach OffsetInChunk
* look breaking
* refactor
* error on no content
* handle intra-block byte skipping
* Incomplete HTTP Response Error Handling
* multipart SSE
* Update s3api_object_handlers.go
* address comments
* less logs
* handling directory
* Optimized rejectDirectoryObjectWithoutSlash() to avoid unnecessary lookups
* Revert "handling directory"
This reverts commit 3a335f0ac3.
* constant
* Consolidate nil entry checks in GetObjectHandler
* add range tests
* Consolidate redundant nil entry checks in HeadObjectHandler
* adjust logs
* SSE type
* large files
* large files
Reverted the plain-object range test
* ErrNoEncryptionConfig
* Fixed SSERangeReader Infinite Loop Vulnerability
* Fixed SSE-KMS Multipart ChunkReader HTTP Body Leak
* handle empty directory in S3, added PyArrow tests
* purge unused code
* Update s3_parquet_test.py
* Update requirements.txt
* According to S3 specifications, when both partNumber and Range are present, the Range should apply within the selected part's boundaries, not to the full object.
* handle errors
* errors after writing header
* https
* fix: Wait for volume assignment readiness before running Parquet tests
The test-implicit-dir-with-server test was failing with an Internal Error
because volume assignment was not ready when tests started. This fix adds
a check that attempts a volume assignment and waits for it to succeed
before proceeding with tests.
This ensures that:
1. Volume servers are registered with the master
2. Volume growth is triggered if needed
3. The system can successfully assign volumes for writes
Fixes the timeout issue where boto3 would retry 4 times and fail with
'We encountered an internal error, please try again.'
* sse tests
* store derived IV
* fix: Clean up gRPC ports between tests to prevent port conflicts
The second test (test-implicit-dir-with-server) was failing because the
volume server's gRPC port (18080 = VOLUME_PORT + 10000) was still in use
from the first test. The cleanup code only killed HTTP port processes,
not gRPC port processes.
Added cleanup for gRPC ports in all stop targets:
- Master gRPC: MASTER_PORT + 10000 (19333)
- Volume gRPC: VOLUME_PORT + 10000 (18080)
- Filer gRPC: FILER_PORT + 10000 (18888)
This ensures clean state between test runs in CI.
* add import
* address comments
* docs: Add placeholder documentation files for Parquet test suite
Added three missing documentation files referenced in test/s3/parquet/README.md:
1. TEST_COVERAGE.md - Documents 43 total test cases (17 Go unit tests,
6 Python integration tests, 20 Python end-to-end tests)
2. FINAL_ROOT_CAUSE_ANALYSIS.md - Explains the s3fs compatibility issue
with PyArrow, the implicit directory problem, and how the fix works
3. MINIO_DIRECTORY_HANDLING.md - Compares MinIO's directory handling
approach with SeaweedFS's implementation
Each file contains:
- Title and overview
- Key technical details relevant to the topic
- TODO sections for future expansion
These placeholder files resolve the broken README links and provide
structure for future detailed documentation.
* clean up if metadata operation failed
* Update s3_parquet_test.py
* clean up
* Update Makefile
* Update s3_parquet_test.py
* Update Makefile
* Handle ivSkip for non-block-aligned offsets
* Update README.md
* stop volume server faster
* stop volume server in 1 second
* different IV for each chunk in SSE-S3 and SSE-KMS
* clean up if fails
* testing upload
* error propagation
* fmt
* simplify
* fix copying
* less logs
* endian
* Added marshaling error handling
* handling invalid ranges
* error handling for adding to log buffer
* fix logging
* avoid returning too quickly and ensure proper cleaning up
* Activity Tracking for Disk Reads
* Cleanup Unused Parameters
* Activity Tracking for Kafka Publishers
* Proper Test Error Reporting
* refactoring
* less logs
* less logs
* go fmt
* guard it with if entry.Attributes.TtlSec > 0 to match the pattern used elsewhere.
* Handle bucket-default encryption config errors explicitly for multipart
* consistent activity tracking
* obsolete code for s3 on filer read/write handlers
* Update weed/s3api/s3api_object_handlers_list.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* avoid aggressive volume assignment
* also test ec shards
* separate DiskLocation instances for each subtest
* edge cases
* No volumes plus low disk space
* Multiple EC volumes
* simplify
* Lazy Versioning Check, Conditional SSE Entry Fetch, HEAD Request Optimization
* revert
Reverted the conditional versioning check to always check versioning status
Reverted the conditional SSE entry fetch to always fetch entry metadata
Reverted the conditional versioning check to always check versioning status
Reverted the conditional SSE entry fetch to always fetch entry metadata
* Lazy Entry Fetch for SSE, Skip Conditional Header Check
* SSE-KMS headers are present, this is not an SSE-C request (mutually exclusive)
* SSE-C is mutually exclusive with SSE-S3 and SSE-KMS
* refactor
* Removed Premature Mutual Exclusivity Check
* check for the presence of the X-Amz-Server-Side-Encryption header
* not used
* fmt
* refactor to avoids circular dependency
* converts a policy.PolicyDocument to policy_engine.PolicyDocument
* convert numeric types to strings
* Update weed/s3api/policy_conversion.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* refactoring
* not skipping numeric and boolean values in arrays
* avoid nil
* edge cases
* handling conversion failure
The handling of unsupported types in convertToString could lead to silent policy alterations.
The conversion of map-based principals in convertPrincipal is too generic and could misinterpret policies.
* concise
* fix doc
* adjust warning
* recursion
* return errors
* reject empty principals
* better error message
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* evaluate policies during authorization
* cache bucket policy
* refactor
* matching with regex special characters
* Case Sensitivity, pattern cache, Dead Code Removal
* Fixed Typo, Restored []string Case, Added Cache Size Limit
* hook up with policy engine
* remove old implementation
* action mapping
* validate
* if not specified, fall through to IAM checks
* fmt
* Fail-close on policy evaluation errors
* Explicit `Allow` bypasses IAM checks
* fix error message
* arn:seaweed => arn:aws
* remove legacy support
* fix tests
* Clean up bucket policy after this test
* fix for tests
* address comments
* security fixes
* fix tests
* temp comment out
* handle volume not found when backing up
* error handling on reading volume ttl and replication
* fix Inconsistent error handling: should continue to next location.
* adjust messages
* close volume
* refactor
* refactor
* proper v.Close()
* Rework parameters passing for functions within `ec.rebuild`
This simplifies the overall codebase and allows to cleanly handle parallelization via waitgroups.
* fix copy source
* add tests
* remove tests not useful
* fmt
* nil check
---------
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: chrislu <chris.lu@gmail.com>
* Rework parameters passing for functions within `volume.check.disk`.
We'll need to rework this logic to account for read-only volumes, and there're already way too many parameters shuffled around.
Grouping these into a single struct simplifies the overall codebase.
* similar fix
* Improved Error Handling in Tests
* propagate the errors
* edge cases
* edge case on modified time
* clean up
---------
Co-authored-by: chrislu <chris.lu@gmail.com>
* Unify the parameter to disable dry-run on weed shell commands to --apply (instead of --force).
* lint
* refactor
* Execution Order Corrected
* handle deprecated force flag
* fix help messages
* Refactoring]: Using flag.FlagSet.Visit()
* consistent with other commands
* Checks for both flags
* fix toml files
---------
Co-authored-by: chrislu <chris.lu@gmail.com>
* do delete expired entries on s3 list request
https://github.com/seaweedfs/seaweedfs/issues/6837
* disable delete expires s3 entry in filer
* pass opt allowDeleteObjectsByTTL to all servers
* delete on get and head
* add lifecycle expiration s3 tests
* fix opt allowDeleteObjectsByTTL for server
* fix test lifecycle expiration
* fix IsExpired
* fix locationPrefix for updateEntriesTTL
* fix s3tests
* resolv coderabbitai
* GetS3ExpireTime on filer
* go mod
* clear TtlSeconds for volume
* move s3 delete expired entry to filer
* filer delete meta and data
* del unusing func removeExpiredObject
* test s3 put
* test s3 put multipart
* allowDeleteObjectsByTTL by default
* fix pipline tests
* rm dublicate SeaweedFSExpiresS3
* revert expiration tests
* fix updateTTL
* rm log
* resolv comment
* fix delete version object
* fix S3Versioning
* fix delete on FindEntry
* fix delete chunks
* fix sqlite not support concurrent writes/reads
* move deletion out of listing transaction; delete entries and empty folders
* Revert "fix sqlite not support concurrent writes/reads"
This reverts commit 5d5da14e0e.
* clearer handling on recursive empty directory deletion
* handle listing errors
* strut copying
* reuse code to delete empty folders
* use iterative approach with a queue to avoid recursive WithFilerClient calls
* stop a gRPC stream from the client-side callback is to return a specific error, e.g., io.EOF
* still issue UpdateEntry when the flag must be added
* errors join
* join path
* cleaner
* add context, sort directories by depth (deepest first) to avoid redundant checks
* batched operation, refactoring
* prevent deleting bucket
* constant
* reuse code
* more logging
* refactoring
* s3 TTL time
* Safety check
---------
Co-authored-by: chrislu <chris.lu@gmail.com>
- Update matchesCollection to recognize 'default' as filter for empty collection
- Remove incorrect conversion of 'default' to empty string in handlers
- Fixes issue where ?collection=default would show all collections instead of just default collection
* Added continue statements after all state transitions in the state machine to ensure immediate state processing
* simplify
* remove redundant continue clause
* ensure wrong signature
* weed master -peers=none
* single master mode only when peers is none
* refactoring
* revert duplicated code
* revert
* Update weed/command/master.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* preventing "none" passed to other components if master is not started
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* adjust "weed benchmark" CLI to use readOnly/writeOnly
* consistently use "-master" CLI option
* If both -readOnly and -writeOnly are specified, the current logic silently allows it with -writeOnly taking precedence. This is confusing and could lead to unexpected behavior.
* fallback to check master
* clean up
* parsing
* refactor
* handle parse error
* return error
* avoid dup lookup
* use batch key
* dedup lookup logic
* address comments
* errors.Join(lookupErrors...)
* add a comment
* Fix: Critical data race in MasterClient vidMap
Fixes a critical data race where resetVidMap() was writing to the vidMap
pointer while other methods were reading it concurrently without synchronization.
Changes:
- Removed embedded *vidMap from MasterClient
- Added vidMapLock (sync.RWMutex) to protect vidMap pointer access
- Created safe accessor methods (GetLocations, GetDataCenter, etc.)
- Updated all direct vidMap accesses to use thread-safe methods
- Updated resetVidMap() to acquire write lock during pointer swap
The vidMap already has internal locking for its operations, but this fix
protects the vidMap pointer itself from concurrent read/write races.
Verified with: go test -race ./weed/wdclient/...
Impact:
- Prevents potential panics from concurrent pointer access
- No performance impact - uses RWMutex for read-heavy workloads
- Maintains backward compatibility through wrapper methods
* fmt
* Fix: Critical data race in MasterClient vidMap
Fixes a critical data race where resetVidMap() was writing to the vidMap
pointer while other methods were reading it concurrently without synchronization.
Changes:
- Removed embedded *vidMap from MasterClient struct
- Added vidMapLock (sync.RWMutex) to protect vidMap pointer access
- Created minimal public accessor methods for external packages:
* GetLocations, GetLocationsClone, GetVidLocations
* LookupFileId, LookupVolumeServerUrl
* GetDataCenter
- Internal code directly locks and accesses vidMap (no extra indirection)
- Updated resetVidMap() to acquire write lock during pointer swap
- Updated shell/commands.go to use GetDataCenter() method
Design Philosophy:
- vidMap already has internal locking for its map operations
- This fix specifically protects the vidMap *pointer* from concurrent access
- Public methods for external callers, direct locking for internal use
- Minimizes wrapper overhead while maintaining thread safety
Verified with: go test -race ./weed/wdclient/... (passes)
Impact:
- Prevents potential panics/crashes from data races
- Minimal performance impact (RWMutex for read-heavy workload)
- Maintains full backward compatibility
* fix more concurrent access
* reduce lock scope
* Optimize vidMap locking for better concurrency
Improved locking strategy based on the understanding that:
- vidMapLock protects the vidMap pointer from concurrent swaps
- vidMap has internal locks that protect its data structures
Changes:
1. Read operations: Grab pointer with RLock, release immediately, then operate
- Reduces lock hold time
- Allows resetVidMap to proceed sooner
- Methods: GetLocations, GetLocationsClone, GetVidLocations,
LookupVolumeServerUrl, GetDataCenter
2. Write operations: Changed from Lock() to RLock()
- RLock prevents pointer swap during operation
- Allows concurrent readers and other writers (serialized by vidMap's lock)
- Methods: addLocation, deleteLocation, addEcLocation, deleteEcLocation
Benefits:
- Significantly reduced lock contention
- Better concurrent performance under load
- Still prevents all race conditions
Verified with: go test -race ./weed/wdclient/... (passes)
* Further reduce lock contention in LookupVolumeIdsWithFallback
Optimized two loops that were holding RLock for extended periods:
Before:
- Held RLock during entire loop iteration
- Included string parsing and cache lookups
- Could block resetVidMap for significant time with large batches
After:
- Grab vidMap pointer with brief RLock
- Release lock immediately
- Perform all loop operations on local pointer
Impact:
- First loop: Cache check on initial volumeIds
- Second loop: Double-check after singleflight wait
Benefits:
- Minimal lock hold time (just pointer copy)
- resetVidMap no longer blocked by long loops
- Better concurrent performance with large volume ID lists
- Still thread-safe (vidMap methods have internal locks)
Verified with: go test -race ./weed/wdclient/... (passes)
* Add clarifying comments to vidMap helper functions
Added inline documentation to each helper function (addLocation, deleteLocation,
addEcLocation, deleteEcLocation) explaining the two-level locking strategy:
- RLock on vidMapLock prevents resetVidMap from swapping the pointer
- vidMap has internal locks that protect the actual map mutations
- This design provides optimal concurrency
The comments make it clear why RLock (not Lock) is correct and intentional,
preventing future confusion about the locking strategy.
* Improve encapsulation: Add shallowClone() method to vidMap
Added a shallowClone() method to vidMap to improve encapsulation and prevent
MasterClient from directly accessing vidMap's internal fields.
Changes:
1. Added vidMap.shallowClone() in vid_map.go
- Encapsulates the shallow copy logic within vidMap
- Makes vidMap responsible for its own state representation
- Documented that caller is responsible for thread safety
2. Simplified resetVidMap() in masterclient.go
- Uses tail := mc.vidMap.shallowClone() instead of manual field access
- Cleaner, more maintainable code
- Better adherence to encapsulation principles
Benefits:
- Improved code organization and maintainability
- vidMap internals are now properly encapsulated
- Easier to modify vidMap structure in the future
- More self-documenting code
Verified with: go test -race ./weed/wdclient/... (passes)
* Optimize locking: Reduce lock acquisitions and use helper methods
Two optimizations to further reduce lock contention and improve code consistency:
1. LookupFileIdWithFallback: Eliminated redundant lock acquisition
- Before: Two separate locks to get vidMap and dataCenter
- After: Single lock gets both values together
- Benefit: 50% reduction in lock/unlock overhead for this hot path
2. KeepConnected: Use GetDataCenter() helper for consistency
- Before: Manual lock/unlock to access DataCenter field
- After: Use existing GetDataCenter() helper method
- Benefit: Better encapsulation and code consistency
Impact:
- Reduced lock contention in high-traffic lookup path
- More consistent use of accessor methods throughout codebase
- Cleaner, more maintainable code
Verified with: go test -race ./weed/wdclient/... (passes)
* Refactor: Extract common locking patterns into helper methods
Eliminated code duplication by introducing two helper methods that encapsulate
the common locking patterns used throughout MasterClient:
1. getStableVidMap() - For read operations
- Acquires lock, gets pointer, releases immediately
- Returns stable snapshot for thread-safe reads
- Used by: GetLocations, GetLocationsClone, GetVidLocations,
LookupFileId, LookupVolumeServerUrl, GetDataCenter
2. withCurrentVidMap(f func(vm *vidMap)) - For write operations
- Holds RLock during callback execution
- Prevents pointer swap while allowing concurrent operations
- Used by: addLocation, deleteLocation, addEcLocation, deleteEcLocation
Benefits:
- Reduced code duplication (eliminated 48 lines of repetitive locking code)
- Centralized locking logic makes it easier to understand and maintain
- Self-documenting pattern through named helper methods
- Easier to modify locking strategy in the future (single point of change)
- Improved readability - accessor methods are now one-liners
Code size reduction: ~40% fewer lines for accessor/helper methods
Verified with: go test -race ./weed/wdclient/... (passes)
* consistent
* Fix cache pointer race condition with atomic.Pointer
Use atomic.Pointer for vidMap cache field to prevent data races
during cache trimming in resetVidMap. This addresses the race condition
where concurrent GetLocations calls could read the cache pointer while
resetVidMap is modifying it during cache chain trimming.
Changes:
- Changed cache field from *vidMap to atomic.Pointer[vidMap]
- Updated all cache access to use Load() and Store() atomic operations
- Updated shallowClone, GetLocations, deleteLocation, deleteEcLocation
- Updated resetVidMap to use atomic operations for cache trimming
* Merge: Resolve conflict in deleteEcLocation - keep atomic.Pointer and fix bug
Resolved merge conflict by combining:
1. Atomic pointer access pattern (from HEAD): cache.Load()
2. Correct method call (from fix): deleteEcLocation (not deleteLocation)
Resolution:
- Before (HEAD): cachedMap.deleteLocation() - WRONG, reintroduced bug
- Before (fix): vc.cache.deleteEcLocation() - RIGHT method, old pattern
- After (merged): cachedMap.deleteEcLocation() - RIGHT method, new pattern
This preserves both improvements:
✓ Thread-safe atomic.Pointer access pattern
✓ Correct recursive call to deleteEcLocation
Verified with: go test -race ./weed/wdclient/... (passes)
* Update vid_map.go
* remove shallow clone
* simplify
* Add nginx reverse proxy documentation for S3 API
Fixes#7407
Add comprehensive documentation and example configuration for using
nginx as a reverse proxy with SeaweedFS S3 API while maintaining AWS
Signature V4 authentication compatibility.
Changes:
- Add docker/nginx/README.md with detailed setup guide
- Add docker/nginx/s3-example.conf with working configuration
- Update docker/nginx/proxy.conf with important S3 notes
The documentation covers:
- Critical requirements for AWS Signature V4 authentication
- Common mistakes and why they break S3 authentication
- Complete working nginx configurations
- Debugging tips and troubleshooting
- Performance tuning recommendations
* Fix IPv6 host header formatting to match AWS SDK behavior
Follow-up to PR #7403
When a default port (80 for HTTP, 443 for HTTPS) is stripped from an
IPv6 address, the square brackets should also be removed to match AWS
SDK behavior for S3 signature calculation.
Reference: https://github.com/aws/aws-sdk-go-v2/blob/main/aws/signer/internal/v4/host.go
The AWS SDK's stripPort function explicitly removes brackets when
returning an IPv6 address without a port.
Changes:
- Update extractHostHeader to strip brackets from IPv6 addresses when
no port or default port is used
- Update test expectations to match AWS SDK behavior
- Add detailed comments explaining the AWS SDK compatibility requirement
This ensures S3 signature validation works correctly with IPv6 addresses
behind reverse proxies, matching AWS S3 canonical request format.
Fixes the issue raised in PR #7403 comment:
https://github.com/seaweedfs/seaweedfs/pull/7403#issuecomment-3471105438
* Update docker/nginx/README.md
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Add nginx reverse proxy documentation for S3 API
Fixes#7407
Add comprehensive documentation and example configuration for using
nginx as a reverse proxy with SeaweedFS S3 API while maintaining AWS
Signature V4 authentication compatibility.
Changes:
- Add docker/nginx/README.md with detailed setup guide
- Add docker/nginx/s3-example.conf with working configuration
- Update docker/nginx/proxy.conf with important S3 notes
The documentation covers:
- Critical requirements for AWS Signature V4 authentication
- Common mistakes and why they break S3 authentication
- Complete working nginx configurations
- Debugging tips and troubleshooting
- Performance tuning recommendations
Fix IPv6 host header formatting to match AWS SDK behavior
Follow-up to PR #7403
When a default port (80 for HTTP, 443 for HTTPS) is stripped from an
IPv6 address, the square brackets should also be removed to match AWS
SDK behavior for S3 signature calculation.
Reference: https://github.com/aws/aws-sdk-go-v2/blob/main/aws/signer/internal/v4/host.go
The AWS SDK's stripPort function explicitly removes brackets when
returning an IPv6 address without a port.
Changes:
- Update extractHostHeader to strip brackets from IPv6 addresses when
no port or default port is used
- Update test expectations to match AWS SDK behavior
- Add detailed comments explaining the AWS SDK compatibility requirement
This ensures S3 signature validation works correctly with IPv6 addresses
behind reverse proxies, matching AWS S3 canonical request format.
Fixes the issue raised in PR #7403 comment:
https://github.com/seaweedfs/seaweedfs/pull/7403#issuecomment-3471105438
* Revert "Merge branch 'fix-ipv6-brackets-default-port' of https://github.com/seaweedfs/seaweedfs into fix-ipv6-brackets-default-port"
This reverts commit cca3f3985f, reversing
changes made to 2b8f9de78e.
---------
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* server can start when no network for local dev
* fixed superfluous response.WriteHeader call" warning
* adaptive based on last write time
* more doc
* refactoring
* Fix S3 bucket policy ARN validation to accept AWS ARNs and simplified formats
Fixes#7252
The bucket policy validation was rejecting valid AWS-style ARNs and
simplified resource formats, causing validation failures with the
error 'resource X does not match bucket X' even when they were
identical strings.
Changes:
- Updated validateResourceForBucket() to accept three formats:
1. AWS-style ARNs: arn:aws:s3:::bucket-name[/*|/path]
2. SeaweedFS ARNs: arn:seaweed:s3:::bucket-name[/*|/path]
3. Simplified formats: bucket-name[/*|/path]
- Added comprehensive test coverage for all three formats
- Added specific test cases from issue #7252 to prevent regression
This ensures compatibility with standard AWS S3 bucket policies
while maintaining support for SeaweedFS-specific ARN format.
* Refactor validateResourceForBucket to reduce code duplication
Simplified the validation logic by stripping ARN prefixes first,
then performing validation on the remaining resource path.
This reduces code duplication and improves maintainability while
maintaining identical functionality.
Addresses review feedback from Gemini Code Assist.
* Use strings.CutPrefix for cleaner ARN prefix stripping
Replace strings.HasPrefix checks with strings.CutPrefix for more
idiomatic Go code. This function is available in Go 1.20+ and
provides cleaner conditional logic with the combined check and
extraction.
Addresses review feedback from Gemini Code Assist.
* Filer: Add retry mechanism for failed file deletions
Implement a retry queue with exponential backoff for handling transient
deletion failures, particularly when volumes are temporarily read-only.
Key features:
- Automatic retry for retryable errors (read-only volumes, network issues)
- Exponential backoff: 5min → 10min → 20min → ... (max 6 hours)
- Maximum 10 retry attempts per file before giving up
- Separate goroutine processing retry queue every minute
- Enhanced logging with retry/permanent error classification
This addresses the issue where file deletions fail when volumes are
temporarily read-only (tiered volumes, maintenance, etc.) and these
deletions were previously lost.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Update weed/filer/filer_deletion.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Filer: Add retry mechanism for failed file deletions
Implement a retry queue with exponential backoff for handling transient
deletion failures, particularly when volumes are temporarily read-only.
Key features:
- Automatic retry for retryable errors (read-only volumes, network issues)
- Exponential backoff: 5min → 10min → 20min → ... (max 6 hours)
- Maximum 10 retry attempts per file before giving up
- Separate goroutine processing retry queue every minute
- Map-based retry queue for O(1) lookups and deletions
- Enhanced logging with retry/permanent error classification
- Consistent error detail limiting (max 10 total errors logged)
- Graceful shutdown support with quit channel for both processors
This addresses the issue where file deletions fail when volumes are
temporarily read-only (tiered volumes, maintenance, etc.) and these
deletions were previously lost.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* Filer: Replace magic numbers with named constants in retry processor
Replace hardcoded values with package-level constants for better
maintainability:
- DeletionRetryPollInterval (1 minute): interval for checking retry queue
- DeletionRetryBatchSize (1000): max items to process per iteration
This improves code readability and makes configuration changes easier.
* Filer: Optimize retry queue with min-heap data structure
Replace map-based retry queue with a min-heap for better scalability
and deterministic ordering.
Performance improvements:
- GetReadyItems: O(N) → O(K log N) where K is items retrieved
- AddOrUpdate: O(1) → O(log N) (acceptable trade-off)
- Early exit when checking ready items (heap top is earliest)
- No full iteration over all items while holding lock
Benefits:
- Deterministic processing order (earliest NextRetryAt first)
- Better scalability for large retry queues (thousands of items)
- Reduced lock contention duration
- Memory efficient (no separate slice reconstruction)
Implementation:
- Min-heap ordered by NextRetryAt using container/heap
- Dual index: heap for ordering + map for O(1) FileId lookups
- heap.Fix() used when updating existing items
- Comprehensive complexity documentation in comments
This addresses the performance bottleneck identified in GetReadyItems
where iterating over the entire map with a write lock could block
other goroutines in high-failure scenarios.
* Filer: Modernize heap interface and improve error handling docs
1. Replace interface{} with any in heap methods
- Addresses modern Go style (Go 1.18+)
- Improves code readability
2. Enhance isRetryableError documentation
- Acknowledge string matching brittleness
- Add comprehensive TODO for future improvements:
* Use HTTP status codes (503, 429, etc.)
* Implement structured error types with errors.Is/As
* Extract gRPC status codes
* Add error wrapping for better context
- Document each error pattern with context
- Add defensive check for empty error strings
Current implementation remains pragmatic for initial release while
documenting a clear path for future robustness improvements. String
matching is acceptable for now but should be replaced with structured
error checking when refactoring the deletion pipeline.
* Filer: Refactor deletion processors for better readability
Extract large callback functions into dedicated private methods to
improve code organization and maintainability.
Changes:
1. Extract processDeletionBatch method
- Handles deletion of a batch of file IDs
- Classifies errors (success, not found, retryable, permanent)
- Manages retry queue additions
- Consolidates logging logic
2. Extract processRetryBatch method
- Handles retry attempts for previously failed deletions
- Processes retry results and updates queue
- Symmetric to processDeletionBatch for consistency
Benefits:
- Main loop functions (loopProcessingDeletion, loopProcessingDeletionRetry)
are now concise and focused on orchestration
- Business logic is separated into testable methods
- Reduced nesting depth improves readability
- Easier to understand control flow at a glance
- Better separation of concerns
The refactored methods follow the single responsibility principle,
making the codebase more maintainable and easier to extend.
* Update weed/filer/filer_deletion.go
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
* Filer: Fix critical retry count bug and add comprehensive error patterns
Critical bug fixes from PR review:
1. Fix RetryCount reset bug (CRITICAL)
- Problem: When items are re-queued via AddOrUpdate, RetryCount
resets to 1, breaking exponential backoff
- Solution: Add RequeueForRetry() method that preserves retry state
- Impact: Ensures proper exponential backoff progression
2. Add overflow protection in backoff calculation
- Check shift amount > 63 to prevent bit-shift overflow
- Additional safety: check if delay <= 0 or > MaxRetryDelay
- Protects against arithmetic overflow in extreme cases
3. Expand retryable error patterns
- Added: timeout, deadline exceeded, context canceled
- Added: lookup error/failed (volume discovery issues)
- Added: connection refused, broken pipe (network errors)
- Added: too many requests, service unavailable (backpressure)
- Added: temporarily unavailable, try again (transient errors)
- Added: i/o timeout (network timeouts)
Benefits:
- Retry mechanism now works correctly across restarts
- More robust against edge cases and overflow
- Better coverage of transient failure scenarios
- Improved resilience in high-failure environments
Addresses feedback from CodeRabbit and Gemini Code Assist in PR #7402.
* Filer: Add persistence docs and comprehensive unit tests
Documentation improvements:
1. Document in-memory queue limitation
- Acknowledge that retry queue is volatile (lost on restart)
- Document trade-offs and future persistence options
- Provide clear path for production hardening
- Note eventual consistency through main deletion queue
Unit test coverage:
1. TestDeletionRetryQueue_AddAndRetrieve
- Basic add/retrieve operations
- Verify items not ready before delay elapsed
2. TestDeletionRetryQueue_ExponentialBackoff
- Verify exponential backoff progression (5m→10m→20m→40m→80m)
- Validate delay calculations with timing tolerance
3. TestDeletionRetryQueue_OverflowProtection
- Test high retry counts (60+) that could cause overflow
- Verify capping at MaxRetryDelay
4. TestDeletionRetryQueue_MaxAttemptsReached
- Verify items discarded after MaxRetryAttempts
- Confirm proper queue cleanup
5. TestIsRetryableError
- Comprehensive error pattern coverage
- Test all retryable error types (timeout, connection, lookup, etc.)
- Verify non-retryable errors correctly identified
6. TestDeletionRetryQueue_HeapOrdering
- Verify min-heap property maintained
- Test items processed in NextRetryAt order
- Validate heap.Init() integration
All tests passing. Addresses PR feedback on testing requirements.
* Filer: Add code quality improvements for deletion retry
Address PR feedback with minor optimizations:
- Add MaxLoggedErrorDetails constant (replaces magic number 10)
- Pre-allocate slices and maps in processRetryBatch for efficiency
- Improve log message formatting to use constant
These changes improve code maintainability and runtime performance
without altering functionality.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
* refactoring retrying
* use constant
* assert
* address comment
* refactor
* address comments
* dedup
* process retried deletions
* address comment
* check in-flight items also; dedup code
* refactoring
* refactoring
* simplify
* reset heap
* more efficient
* add DeletionBatchSize as a constant;Permanent > Retryable > Success > Not Found
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: chrislu <chris.lu@gmail.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
chrislu
Chris Lu
Leonardo Lara
Feng Shao
Konstantin Lebedev
Lisandro Pin
Dmitriy Pavlov