diff options
| author | chrislu <chris.lu@gmail.com> | 2025-12-05 20:58:58 -0800 |
|---|---|---|
| committer | chrislu <chris.lu@gmail.com> | 2025-12-05 20:58:58 -0800 |
| commit | acd7f1a4d551108f296455a480356db17cec3a5b (patch) | |
| tree | d943a7e9ccfbe5191252077fd8808f557fc3f1ac | |
| parent | eb6a56b6c61c8c2adf0d356f3761646847f26b01 (diff) | |
| download | seaweedfs-acd7f1a4d551108f296455a480356db17cec3a5b.tar.xz seaweedfs-acd7f1a4d551108f296455a480356db17cec3a5b.zip | |
s3api: reuse prefetched entry in versioned DELETE operations
Update deleteSpecificObjectVersion() to accept an optional prefetchedEntry
parameter. When the entry was already fetched by enforceObjectLockProtections(),
it is now passed through to avoid a duplicate filer lookup.
This optimization applies to:
- DELETE specific version (versionId specified)
- DELETE null version (suspended versioning)
- DeleteMultipleObjects for versioned buckets
Performance impact:
- Versioned DELETE: reduces filer lookups from 2 to 1 (50% reduction)
- Non-versioned DELETE: no change (requires gRPC protocol changes)
| -rw-r--r-- | weed/s3api/s3api_object_handlers_delete.go | 28 | ||||
| -rw-r--r-- | weed/s3api/s3api_object_versioning.go | 20 |
2 files changed, 31 insertions, 17 deletions
diff --git a/weed/s3api/s3api_object_handlers_delete.go b/weed/s3api/s3api_object_handlers_delete.go index 3cd5aa3d4..1cc11da70 100644 --- a/weed/s3api/s3api_object_handlers_delete.go +++ b/weed/s3api/s3api_object_handlers_delete.go @@ -54,15 +54,17 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque if versionId != "" { // Delete specific version (same for both enabled and suspended) // Check object lock permissions before deleting specific version + // The returned entry is reused to avoid duplicate filer lookups governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object) - if _, lockErr := s3a.enforceObjectLockProtections(r, bucket, object, versionId, governanceBypassAllowed); lockErr != nil { + prefetchedEntry, lockErr := s3a.enforceObjectLockProtections(r, bucket, object, versionId, governanceBypassAllowed) + if lockErr != nil { glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, lockErr) s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) return } - // Delete specific version - err := s3a.deleteSpecificObjectVersion(bucket, object, versionId) + // Delete specific version, passing prefetched entry to avoid duplicate lookup + err := s3a.deleteSpecificObjectVersion(bucket, object, versionId, prefetchedEntry) if err != nil { glog.Errorf("Failed to delete specific version %s: %v", versionId, err) s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) @@ -92,15 +94,17 @@ func (s3a *S3ApiServer) DeleteObjectHandler(w http.ResponseWriter, r *http.Reque glog.V(2).Infof("DeleteObjectHandler: deleting null version for suspended versioning %s/%s", bucket, object) // Check object lock permissions before deleting "null" version + // The returned entry is reused to avoid duplicate filer lookups governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object) - if _, lockErr := s3a.enforceObjectLockProtections(r, bucket, object, "null", governanceBypassAllowed); lockErr != nil { + prefetchedEntry, lockErr := s3a.enforceObjectLockProtections(r, bucket, object, "null", governanceBypassAllowed) + if lockErr != nil { glog.V(2).Infof("DeleteObjectHandler: object lock check failed for %s/%s: %v", bucket, object, lockErr) s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied) return } - // Delete the "null" version (the regular file) - err := s3a.deleteSpecificObjectVersion(bucket, object, "null") + // Delete the "null" version (the regular file), passing prefetched entry + err := s3a.deleteSpecificObjectVersion(bucket, object, "null", prefetchedEntry) if err != nil { glog.Errorf("Failed to delete null version: %v", err) s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) @@ -235,10 +239,14 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h } // Check object lock permissions before deletion (only for versioned buckets) + // The returned entry is reused to avoid duplicate filer lookups + var prefetchedEntry *filer_pb.Entry if versioningConfigured { // Validate governance bypass for this specific object governanceBypassAllowed := s3a.evaluateGovernanceBypassRequest(r, bucket, object.Key) - if _, lockErr := s3a.enforceObjectLockProtections(r, bucket, object.Key, object.VersionId, governanceBypassAllowed); lockErr != nil { + var lockErr error + prefetchedEntry, lockErr = s3a.enforceObjectLockProtections(r, bucket, object.Key, object.VersionId, governanceBypassAllowed) + if lockErr != nil { glog.V(2).Infof("DeleteMultipleObjectsHandler: object lock check failed for %s/%s (version: %s): %v", bucket, object.Key, object.VersionId, lockErr) deleteErrors = append(deleteErrors, DeleteError{ Code: s3err.GetAPIError(s3err.ErrAccessDenied).Code, @@ -257,7 +265,8 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h // Handle versioned delete based on specific versioning state if object.VersionId != "" { // Delete specific version (same for both enabled and suspended) - err := s3a.deleteSpecificObjectVersion(bucket, object.Key, object.VersionId) + // Pass prefetched entry to avoid duplicate lookup + err := s3a.deleteSpecificObjectVersion(bucket, object.Key, object.VersionId, prefetchedEntry) if err != nil { deleteErrors = append(deleteErrors, DeleteError{ Code: "", @@ -288,7 +297,8 @@ func (s3a *S3ApiServer) DeleteMultipleObjectsHandler(w http.ResponseWriter, r *h // Suspended versioning: Actually delete the "null" version object glog.V(2).Infof("DeleteMultipleObjectsHandler: deleting null version for suspended versioning %s/%s", bucket, object.Key) - err := s3a.deleteSpecificObjectVersion(bucket, object.Key, "null") + // Pass prefetched entry to avoid duplicate lookup + err := s3a.deleteSpecificObjectVersion(bucket, object.Key, "null", prefetchedEntry) if err != nil { deleteErrors = append(deleteErrors, DeleteError{ Code: "", diff --git a/weed/s3api/s3api_object_versioning.go b/weed/s3api/s3api_object_versioning.go index bbc43f205..fcc385c65 100644 --- a/weed/s3api/s3api_object_versioning.go +++ b/weed/s3api/s3api_object_versioning.go @@ -637,8 +637,10 @@ func (s3a *S3ApiServer) getSpecificObjectVersion(bucket, object, versionId strin return entry, nil } -// deleteSpecificObjectVersion deletes a specific version of an object -func (s3a *S3ApiServer) deleteSpecificObjectVersion(bucket, object, versionId string) error { +// deleteSpecificObjectVersion deletes a specific version of an object. +// The optional prefetchedEntry parameter allows callers to pass an already-fetched entry +// to avoid duplicate filer lookups (optimization for DELETE operations). +func (s3a *S3ApiServer) deleteSpecificObjectVersion(bucket, object, versionId string, prefetchedEntry *filer_pb.Entry) error { // Normalize object path to ensure consistency with toFilerPath behavior normalizedObject := removeDuplicateSlashes(object) @@ -651,12 +653,14 @@ func (s3a *S3ApiServer) deleteSpecificObjectVersion(bucket, object, versionId st bucketDir := s3a.option.BucketsPath + "/" + bucket cleanObject := strings.TrimPrefix(normalizedObject, "/") - // Check if the object exists - _, err := s3a.getEntry(bucketDir, cleanObject) - if err != nil { - // Object doesn't exist - this is OK for delete operations (idempotent) - glog.V(2).Infof("deleteSpecificObjectVersion: null version object %s already deleted or doesn't exist", cleanObject) - return nil + // Check if the object exists (skip if we have a prefetched entry) + if prefetchedEntry == nil { + _, err := s3a.getEntry(bucketDir, cleanObject) + if err != nil { + // Object doesn't exist - this is OK for delete operations (idempotent) + glog.V(2).Infof("deleteSpecificObjectVersion: null version object %s already deleted or doesn't exist", cleanObject) + return nil + } } // Delete the regular file |
