mirror of
https://github.com/versity/versitygw.git
synced 2026-09-25 09:24:22 +00:00
fix: stop posix publish lock serializing the same key across buckets
`lockObjectPublish` picked the process-local slot by the key's shard alone, while the shared lock file is per bucket and shard. As a result, writes to the same key name in different buckets waited on each other even though their lock files never conflict. The slot is now chosen from the bucket hash and the shard (`bucketHash[0] ^ shard`). It still depends only on the lock file, never on the full key, so all requests for one lock file keep sharing a slot, which fcntl locks need because they don't exclude within a process.
This commit is contained in:
@@ -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))
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user