diff --git a/backend/posix/objlock.go b/backend/posix/objlock.go index 13098e37..8d507d12 100644 --- a/backend/posix/objlock.go +++ b/backend/posix/objlock.go @@ -49,6 +49,11 @@ import ( // file takes its place, which is unsafe to detect reliably on NFS due to // attribute caching. // +// The process-local slot is picked from the bucket hash and the shard, so all +// requests for one lock file take the same slot (fcntl locks don't exclude +// within a process), while the same key in different buckets usually takes +// different slots and doesn't wait on an unrelated lock. +// // The lock is held only for the commit phase (condition re-check, metadata // stores, final link/rename) — request bodies are staged to a temp file // before the lock is taken. The OS releases advisory locks automatically when @@ -131,15 +136,22 @@ func objLockShard(object string) uint8 { return sum[0] } +// objLockSlot returns the process-local slot index for the lock file of the +// shard in the bucket with bucketHash. +func objLockSlot(bucketHash [sha256.Size]byte, shard uint8) uint8 { + return bucketHash[0] ^ shard +} + // lockObjectPublish acquires the publish lock for bucket/object. It returns a // release function that must be called (typically deferred) once the new // object state is visible. All code paths that create or replace an object at // its final key must hold this lock across condition evaluation and // publication. func (p *Posix) lockObjectPublish(ctx context.Context, bucket, object string) (func(), error) { + bucketHash := sha256.Sum256([]byte(bucket)) shard := objLockShard(object) - slot := p.objLockSlots[shard] + slot := p.objLockSlots[objLockSlot(bucketHash, shard)] select { case <-ctx.Done(): return nil, ctx.Err() @@ -154,7 +166,7 @@ func (p *Posix) lockObjectPublish(ctx context.Context, bucket, object string) (f return releaseLocal, nil } - f, err := p.openObjLockFile(bucket, shard) + f, err := p.openObjLockFile(bucketHash, shard) if err != nil { releaseLocal() return nil, err @@ -188,9 +200,8 @@ func newObjLockSlots() [objLockShards]chan struct{} { } // openObjLockFile opens (creating as needed) the lock file for the shard in -// the given bucket. -func (p *Posix) openObjLockFile(bucket string, shard uint8) (*os.File, error) { - bucketHash := sha256.Sum256([]byte(bucket)) +// the bucket with bucketHash. +func (p *Posix) openObjLockFile(bucketHash [sha256.Size]byte, shard uint8) (*os.File, error) { lockDir := filepath.Join(p.rootdir, objLockDir, fmt.Sprintf("%x", bucketHash)) name := filepath.Join(lockDir, fmt.Sprintf("%02x", shard)) diff --git a/backend/posix/posix_conditional_put_test.go b/backend/posix/posix_conditional_put_test.go index 9e392fae..cce428c1 100644 --- a/backend/posix/posix_conditional_put_test.go +++ b/backend/posix/posix_conditional_put_test.go @@ -17,6 +17,7 @@ package posix import ( "bytes" "context" + "crypto/sha256" "errors" "fmt" "io" @@ -85,9 +86,9 @@ func TestObjectPublishLockHonorsContextWhileWaiting(t *testing.T) { bucket := "testbucket" createTestBucket(t, p, bucket) - shard := objLockShard("cancel-wait") - <-p.objLockSlots[shard] - defer func() { p.objLockSlots[shard] <- struct{}{} }() + slot := objLockSlot(sha256.Sum256([]byte(bucket)), objLockShard("cancel-wait")) + <-p.objLockSlots[slot] + defer func() { p.objLockSlots[slot] <- struct{}{} }() if _, err := os.Stat(p.ObjectPath(bucket, objLockDir)); !errors.Is(err, fs.ErrNotExist) { t.Fatalf("bucket contains publish lock directory: %v", err) } @@ -134,6 +135,25 @@ func TestObjectPublishLockHonorsCancellationAfterSlotAcquired(t *testing.T) { } } +func TestObjectPublishLockDoesNotBlockOtherBuckets(t *testing.T) { + p := newTestPosix(t, metaModes(t)["xattr"]) + + unlock, err := p.lockObjectPublish(context.Background(), "bucket-a", "my-obj") + if err != nil { + t.Fatalf("lock object publish: %v", err) + } + defer unlock() + + // the same key in another bucket has its own lock file + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + unlockOther, err := p.lockObjectPublish(ctx, "bucket-b", "my-obj") + if err != nil { + t.Fatalf("lock object publish in another bucket: %v", err) + } + unlockOther() +} + func TestObjectPublishLockModes(t *testing.T) { tests := []struct { name string