Merge pull request #2359 from versity/ben/lock-options

fix: make conditional publish lock modes explicit
This commit is contained in:
Ben McClelland
2026-09-11 07:57:08 -07:00
committed by GitHub
11 changed files with 258 additions and 68 deletions
+68 -24
View File
@@ -23,7 +23,6 @@ import (
"time"
"github.com/versity/versitygw/backend"
"github.com/versity/versitygw/debuglogger"
)
// Object publish locking
@@ -33,8 +32,8 @@ import (
// replacement happen as one atomic step per bucket/key. The gateway is
// stateless and multiple gateway processes may share the same backend
// filesystem, so an in-process mutex alone is not sufficient: exclusion is
// provided by an advisory lock (flock on unix, LockFileEx on windows) on a
// shared lock file, combined with a process-local striped mutex so that
// provided by a configured advisory lock (flock or fcntl on unix, LockFileEx
// on windows) on a shared lock file, combined with a process-local striped mutex so that
// contention within one process is resolved cheaply and each process presents
// at most one waiter to the filesystem lock.
//
@@ -56,13 +55,11 @@ import (
// the file handle is closed or the process exits, so failures, cancellation,
// or crashes cannot leave a permanently stale lock.
//
// NFS notes: flock on Linux NFS clients is mapped to NFSv4 byte-range locks
// (or NLM on NFSv3), giving cross-client exclusion. Mounting with
// "-o nolock" or "-o local_lock=flock"/"local_lock=all" disables server-side
// locking and reduces exclusion to a single client; conditional-write
// atomicity across gateways requires server-backed locking. If the filesystem
// does not support advisory locking at all, the gateway falls back to
// process-local exclusion and logs a warning once.
// Filesystem lock scope varies by filesystem and mount options. Operators using
// multiple gateway processes must select a mode that their filesystem provides
// with cluster-wide exclusion. The local mode provides only process-local
// exclusion and is unsafe for conditional writes across gateway processes. The
// none mode rejects conditional writes.
const (
// objLockDir is the root directory holding object publish lock files.
@@ -71,6 +68,63 @@ const (
objLockShards = 256
)
// ObjectLockMode selects the advisory locking primitive used to serialize
// conditional object publishes.
type ObjectLockMode string
const (
ObjectLockModeFlock ObjectLockMode = "flock"
ObjectLockModeFcntl ObjectLockMode = "fcntl"
ObjectLockModeLocal ObjectLockMode = "local"
ObjectLockModeNone ObjectLockMode = "none"
)
func resolveObjectLockMode(mode ObjectLockMode, forceNoObjLockFile bool) (ObjectLockMode, error) {
if mode == "" {
if forceNoObjLockFile {
return ObjectLockModeLocal, nil
}
return ObjectLockModeFlock, nil
}
if forceNoObjLockFile && mode != ObjectLockModeLocal {
return "", fmt.Errorf("disable object lock file conflicts with object lock mode %q", mode)
}
switch mode {
case ObjectLockModeFlock, ObjectLockModeFcntl, ObjectLockModeLocal, ObjectLockModeNone:
return mode, nil
default:
return "", fmt.Errorf("invalid object lock mode %q (want flock, fcntl, local, or none)", mode)
}
}
// verifyObjectLockMode verifies that the configured shared lock primitive is
// available on the filesystem containing the gateway's lock directory. It
// cannot verify cross-node lock coherence.
func (p *Posix) verifyObjectLockMode() error {
if p.objectLockMode == ObjectLockModeLocal || p.objectLockMode == ObjectLockModeNone {
return nil
}
if err := validateObjectLockMode(p.objectLockMode); err != nil {
return err
}
lockDir := filepath.Join(p.rootdir, objLockDir)
if err := backend.MkdirAll(lockDir, 0, 0, false, p.newDirPerm); err != nil {
return fmt.Errorf("make object lock directory: %w", err)
}
f, err := os.CreateTemp(lockDir, ".startup-lock-*")
if err != nil {
return fmt.Errorf("create object lock probe: %w", err)
}
defer os.Remove(f.Name())
defer f.Close()
if err := lockFileExclusive(context.Background(), f, p.objectLockMode); err != nil {
return fmt.Errorf("verify object lock mode %q: %w", p.objectLockMode, err)
}
return nil
}
// objLockShard returns the shard index for an object key.
func objLockShard(object string) uint8 {
sum := sha256.Sum256([]byte(object))
@@ -96,7 +150,7 @@ func (p *Posix) lockObjectPublish(ctx context.Context, bucket, object string) (f
releaseLocal()
return nil, err
}
if p.forceNoObjLockFile {
if p.objectLockMode == ObjectLockModeLocal || p.objectLockMode == ObjectLockModeNone {
return releaseLocal, nil
}
@@ -106,25 +160,15 @@ func (p *Posix) lockObjectPublish(ctx context.Context, bucket, object string) (f
return nil, err
}
err = lockFileExclusive(ctx, f)
err = lockFileExclusive(ctx, f, p.objectLockMode)
if err != nil {
f.Close()
if ctx.Err() != nil {
releaseLocal()
return nil, ctx.Err()
}
if !isAdvisoryLockUnsupported(err) {
releaseLocal()
return nil, fmt.Errorf("lock object publish: %w", err)
}
// The filesystem does not support advisory locking (e.g. NFS mounted
// with -o nolock). Fall back to process-local exclusion and warn once:
// conditional writes are then only atomic within this gateway process.
p.objLockWarn.Do(func() {
debuglogger.Logf("object lock file locking unavailable (%v): "+
"conditional write atomicity limited to this process", err)
})
return releaseLocal, nil
releaseLocal()
return nil, fmt.Errorf("lock object publish: %w", err)
}
return func() {
+19 -10
View File
@@ -19,25 +19,35 @@ package posix
import (
"context"
"errors"
"io"
"os"
"time"
"golang.org/x/sys/unix"
)
// lockFileExclusive takes an exclusive advisory flock on f, polling with
// backoff so that context cancellation is honored while waiting. The lock is
// released by closing f or on process exit, so no cleanup beyond Close is
// required on any failure path.
func lockFileExclusive(ctx context.Context, f *os.File) error {
// lockFileExclusive takes the configured exclusive advisory lock on f,
// polling with backoff so that context cancellation is honored while waiting.
func lockFileExclusive(ctx context.Context, f *os.File, mode ObjectLockMode) error {
backoff := objLockInitialBackoff
for {
err := unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB)
var err error
switch mode {
case ObjectLockModeFlock:
err = unix.Flock(int(f.Fd()), unix.LOCK_EX|unix.LOCK_NB)
case ObjectLockModeFcntl:
err = unix.FcntlFlock(f.Fd(), unix.F_SETLK, &unix.Flock_t{
Type: unix.F_WRLCK,
Whence: int16(io.SeekStart),
})
default:
return errors.New("unsupported object lock mode")
}
if err == nil {
return nil
}
if !errors.Is(err, unix.EWOULDBLOCK) && !errors.Is(err, unix.EAGAIN) &&
!errors.Is(err, unix.EINTR) {
!errors.Is(err, unix.EACCES) && !errors.Is(err, unix.EINTR) {
return err
}
@@ -50,7 +60,6 @@ func lockFileExclusive(ctx context.Context, f *os.File) error {
}
}
func isAdvisoryLockUnsupported(err error) bool {
return errors.Is(err, unix.ENOTSUP) || errors.Is(err, unix.EOPNOTSUPP) ||
errors.Is(err, unix.ENOSYS)
func validateObjectLockMode(ObjectLockMode) error {
return nil
}
+10 -4
View File
@@ -19,6 +19,7 @@ package posix
import (
"context"
"errors"
"fmt"
"os"
"time"
@@ -28,7 +29,10 @@ import (
// lockFileExclusive takes an exclusive lock on f via LockFileEx, polling with
// backoff so that context cancellation is honored while waiting. The lock is
// released when f is closed or the process exits.
func lockFileExclusive(ctx context.Context, f *os.File) error {
func lockFileExclusive(ctx context.Context, f *os.File, mode ObjectLockMode) error {
if mode != ObjectLockModeFlock {
return errors.New("object lock mode fcntl is not supported on Windows")
}
backoff := objLockInitialBackoff
for {
ol := new(windows.Overlapped)
@@ -51,7 +55,9 @@ func lockFileExclusive(ctx context.Context, f *os.File) error {
}
}
func isAdvisoryLockUnsupported(err error) bool {
return errors.Is(err, windows.ERROR_INVALID_FUNCTION) ||
errors.Is(err, windows.ERROR_NOT_SUPPORTED)
func validateObjectLockMode(mode ObjectLockMode) error {
if mode == ObjectLockModeFcntl {
return fmt.Errorf("object lock mode %q is not supported on Windows; use flock, local, or none", mode)
}
return nil
}
+26 -18
View File
@@ -96,9 +96,10 @@ type Posix struct {
// support copy_file_range is mounted over NFSv4.2.
forceNoCopyFileRange bool
// forceNoObjLockFile disables the shared advisory lock file used for
// conditional object publishes. Process-local serialization remains active.
forceNoObjLockFile bool
// objectLockMode selects the advisory lock used for conditional object
// publishes. The local mode retains process-local serialization only, and
// the none mode rejects conditional writes.
objectLockMode ObjectLockMode
// enableODirect is a flag to open object data files with O_DIRECT.
// This is best-effort and falls back to buffered I/O when unsupported.
@@ -144,9 +145,6 @@ type Posix struct {
// per-bucket lock files by lockObjectPublish. Channels make local lock
// acquisition cancelable while avoiding filesystem lock thrash.
objLockSlots [objLockShards]chan struct{}
// objLockWarn ensures the advisory-locking-unavailable warning is
// logged at most once
objLockWarn sync.Once
}
func (o *PosixOpts) SetNewDirPerm(perm fs.FileMode) {
@@ -250,10 +248,13 @@ type PosixOpts struct {
ForceNoTmpFile bool
// ForceNoCopyFileRange disables the use of io.Copy for multipart uploads parts
ForceNoCopyFileRange bool
// ForceNoObjLockFile disables the shared advisory lock file used for
// conditional object publishes. Conditional writes are only atomic within a
// single gateway process when enabled.
// ForceNoObjLockFile is a deprecated compatibility alias for
// ObjectLockModeLocal.
ForceNoObjLockFile bool
// ObjectLockMode selects flock, fcntl, local, or none for conditional object
// publishes. An empty value defaults to flock. Local locking is only atomic
// within a single gateway process; none rejects conditional writes.
ObjectLockMode ObjectLockMode
// EnableODirect enables best-effort O_DIRECT for object data reads/writes.
// Disabled by default.
EnableODirect bool
@@ -291,13 +292,12 @@ type PosixOpts struct {
DataIntegrityEtag bool
}
// New returns a backend serving buckets from the directories under rootdir.
//
// By default New changes the process working directory to rootdir and
// addresses every bucket and object by a path relative to it. With
// opts.AbsolutePaths the working directory is left alone and paths are built
// from the absolute root directory instead (see BucketPath and ObjectPath).
// New returns a Posix backend serving buckets from the directories under rootdir.
func New(rootdir string, ms meta.MetadataStorer, opts PosixOpts) (*Posix, error) {
objectLockMode, err := resolveObjectLockMode(opts.ObjectLockMode, opts.ForceNoObjLockFile)
if err != nil {
return nil, err
}
ioBufferSize := ioBufferSizeOrDefault(opts.IOBufferSize)
rootdirAbs, err := filepath.Abs(rootdir)
if err != nil {
@@ -396,7 +396,7 @@ func New(rootdir string, ms meta.MetadataStorer, opts PosixOpts) (*Posix, error)
newFilePerm = opts.NewFilePerm.Perm()
}
return &Posix{
p := &Posix{
meta: ms,
rootfd: f,
rootdir: rootdirAbs,
@@ -411,7 +411,7 @@ func New(rootdir string, ms meta.MetadataStorer, opts PosixOpts) (*Posix, error)
newFilePerm: newFilePerm,
forceNoTmpFile: opts.ForceNoTmpFile,
forceNoCopyFileRange: opts.ForceNoCopyFileRange,
forceNoObjLockFile: opts.ForceNoObjLockFile,
objectLockMode: objectLockMode,
enableODirect: opts.EnableODirect,
validateBucketName: opts.ValidateBucketNames,
actionLimiter: semaphore.NewWeighted(int64(concurrencyOrDefault(opts.Concurrency))),
@@ -424,7 +424,12 @@ func New(rootdir string, ms meta.MetadataStorer, opts PosixOpts) (*Posix, error)
}},
dataIntegrityEtag: opts.DataIntegrityEtag,
objLockSlots: newObjLockSlots(),
}, nil
}
if err := p.verifyObjectLockMode(); err != nil {
p.Shutdown()
return nil, err
}
return p, nil
}
// BucketPath returns the filesystem path of the bucket directory: the bucket
@@ -4114,6 +4119,9 @@ func (p *Posix) checkPutPreconditions(bucket, object string, ifMatch, ifNoneMatc
if ifMatch == nil && ifNoneMatch == nil {
return nil
}
if p.objectLockMode == ObjectLockModeNone {
return s3err.GetAPIError(s3err.ErrNotImplemented)
}
etagBytes, err := p.meta.RetrieveAttribute(nil, bucket, object, etagkey)
if err == nil || errors.Is(err, fs.ErrNotExist) || errors.Is(err, meta.ErrNoSuchKey) {
+86 -1
View File
@@ -134,7 +134,44 @@ func TestObjectPublishLockHonorsCancellationAfterSlotAcquired(t *testing.T) {
}
}
func TestObjectPublishLockCanDisableSharedLockFile(t *testing.T) {
func TestObjectPublishLockModes(t *testing.T) {
tests := []struct {
name string
mode ObjectLockMode
local bool
}{
{name: "flock", mode: ObjectLockModeFlock},
{name: "fcntl", mode: ObjectLockModeFcntl},
{name: "local", mode: ObjectLockModeLocal, local: true},
{name: "none", mode: ObjectLockModeNone, local: true},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if err := validateObjectLockMode(test.mode); err != nil {
t.Skip(err)
}
root := t.TempDir()
p, err := New(root, meta.XattrMeta{}, PosixOpts{
NewDirPerm: 0755,
ObjectLockMode: test.mode,
})
if err != nil {
t.Fatalf("new posix: %v", err)
}
defer p.Shutdown()
_, err = os.Stat(filepath.Join(root, objLockDir))
if test.local && !errors.Is(err, fs.ErrNotExist) {
t.Fatalf("shared lock directory exists or stat failed: %v", err)
}
if !test.local && err != nil {
t.Fatalf("startup did not create shared lock directory: %v", err)
}
})
}
}
func TestObjectPublishLockDisableFlagUsesLocalMode(t *testing.T) {
root := t.TempDir()
p, err := New(root, meta.XattrMeta{}, PosixOpts{
NewDirPerm: 0755,
@@ -156,6 +193,54 @@ func TestObjectPublishLockCanDisableSharedLockFile(t *testing.T) {
}
}
func TestObjectLockModeValidation(t *testing.T) {
_, err := New(t.TempDir(), meta.XattrMeta{}, PosixOpts{ObjectLockMode: "invalid"})
if err == nil {
t.Fatal("new posix with invalid object lock mode succeeded")
}
_, err = New(t.TempDir(), meta.XattrMeta{}, PosixOpts{
ForceNoObjLockFile: true,
ObjectLockMode: ObjectLockModeFcntl,
})
if err == nil {
t.Fatal("new posix with conflicting object lock options succeeded")
}
}
func TestObjectLockModeNoneRejectsConditionalWrites(t *testing.T) {
p, err := New(t.TempDir(), meta.XattrMeta{}, PosixOpts{
NewDirPerm: 0755,
ObjectLockMode: ObjectLockModeNone,
})
if err != nil {
t.Fatalf("new posix: %v", err)
}
defer p.Shutdown()
bucket := "testbucket"
createTestBucket(t, p, bucket)
for _, condition := range []struct {
name string
ifMatch *string
ifNoneMatch *string
}{
{name: "if-match", ifMatch: aws.String("etag")},
{name: "if-none-match", ifNoneMatch: aws.String("*")},
} {
t.Run(condition.name, func(t *testing.T) {
_, err := testPut(p, bucket, "object", []byte("body"), condition.ifMatch, condition.ifNoneMatch)
if !errors.Is(err, s3err.GetAPIError(s3err.ErrNotImplemented)) {
t.Fatalf("conditional put error = %v, want NotImplemented", err)
}
})
}
if _, err := testPut(p, bucket, "unconditional", []byte("body"), nil, nil); err != nil {
t.Fatalf("unconditional put: %v", err)
}
}
func TestObjectPublishLockUsesRootForRelativeBackendPath(t *testing.T) {
parent := t.TempDir()
rootName := "gateway-root"
+3
View File
@@ -18,6 +18,7 @@ import (
"io/fs"
"github.com/versity/versitygw/backend"
"github.com/versity/versitygw/backend/posix"
)
// ScoutfsOpts are the options for the ScoutFS backend
@@ -67,6 +68,8 @@ type ScoutfsOpts struct {
// (e.g. "CRC64NVME-<base64>"). For multipart uploads, part ETags become
// CRC64NVME-based values and the completed object ETag is checksum-derived.
DataIntegrityEtag bool
// ObjectLockMode selects local or none for conditional object publishes.
ObjectLockMode posix.ObjectLockMode
}
func (o *ScoutfsOpts) SetNewDirPerm(perm fs.FileMode) {
+5 -1
View File
@@ -69,6 +69,10 @@ type ScoutFS struct {
func New(rootdir string, opts ScoutfsOpts) (*ScoutFS, error) {
metastore := meta.XattrMeta{}
objectLockMode := opts.ObjectLockMode
if objectLockMode == "" {
objectLockMode = posix.ObjectLockModeNone
}
posixOpts := posix.PosixOpts{
ChownUID: opts.ChownUID,
@@ -80,7 +84,7 @@ func New(rootdir string, opts ScoutfsOpts) (*ScoutFS, error) {
CopyObjectThreshold: opts.CopyObjectThreshold,
DefaultEtag: opts.DefaultEtag,
DataIntegrityEtag: opts.DataIntegrityEtag,
ForceNoObjLockFile: true, // scoutfs flock not cluster consistent
ObjectLockMode: objectLockMode,
}
if opts.newDirPermSet {
posixOpts.SetNewDirPerm(opts.NewDirPerm)
+10 -1
View File
@@ -40,6 +40,7 @@ var (
forceNoTmpFile bool
forceNoCopyFileRange bool
forceNoObjLockFile bool
objectLockMode string
enableODirect bool
actionsConcurrency int
ioBufferSize int
@@ -145,10 +146,17 @@ will be translated into the file /mnt/fs/gwroot/mybucket/a/b/c/myobject`,
},
&cli.BoolFlag{
Name: "disable-object-lock-file",
Usage: "disable shared advisory lock files for conditional object publishes (unsafe with multiple gateway processes)",
Usage: "deprecated alias for --object-lock-mode=local",
EnvVars: []string{"VGW_DISABLE_OBJECT_LOCK_FILE"},
Destination: &forceNoObjLockFile,
},
&cli.StringFlag{
Name: "object-lock-mode",
Usage: "lock mode for conditional object publishes: flock, fcntl, local, or none",
EnvVars: []string{"VGW_OBJECT_LOCK_MODE"},
DefaultText: "flock",
Destination: &objectLockMode,
},
&cli.BoolFlag{
Name: "enable-odirect",
Usage: "enable best-effort O_DIRECT for object data reads/writes",
@@ -202,6 +210,7 @@ func runPosix(ctx *cli.Context) error {
ForceNoTmpFile: forceNoTmpFile,
ForceNoCopyFileRange: forceNoCopyFileRange,
ForceNoObjLockFile: forceNoObjLockFile,
ObjectLockMode: posix.ObjectLockMode(objectLockMode),
EnableODirect: enableODirect,
ValidateBucketNames: DisableStrictBucketNames,
Concurrency: actionsConcurrency,
+15
View File
@@ -20,6 +20,7 @@ import (
"math"
"github.com/urfave/cli/v2"
"github.com/versity/versitygw/backend/posix"
"github.com/versity/versitygw/backend/scoutfs"
)
@@ -27,6 +28,7 @@ var (
glacier bool
disableNoArchive bool
setProjectID bool
scoutfsLockMode string
)
// ScoutfsCommand returns the "scoutfs" subcommand, common to all versitygw
@@ -109,6 +111,14 @@ move interfaces as well as support for tiered filesystems.`,
EnvVars: []string{"VGW_DISABLE_NOARCHIVE"},
Destination: &disableNoArchive,
},
&cli.StringFlag{
Name: "object-lock-mode",
Usage: "lock mode for conditional object publishes: local or none",
EnvVars: []string{"VGW_OBJECT_LOCK_MODE"},
Value: "none",
DefaultText: "none",
Destination: &scoutfsLockMode,
},
&cli.IntFlag{
Name: "concurrency",
Usage: "maximum concurrent actions allowed",
@@ -149,6 +159,10 @@ func runScoutfs(ctx *cli.Context) error {
return fmt.Errorf("concurrency must be positive, got %d", actionsConcurrency)
}
if scoutfsLockMode != string(posix.ObjectLockModeLocal) && scoutfsLockMode != string(posix.ObjectLockModeNone) {
return fmt.Errorf("invalid scoutfs object lock mode %q (want local or none)", scoutfsLockMode)
}
var opts scoutfs.ScoutfsOpts
opts.GlacierMode = glacier
opts.ChownUID = chownuid
@@ -160,6 +174,7 @@ func runScoutfs(ctx *cli.Context) error {
opts.SetProjectID = setProjectID
opts.Concurrency = actionsConcurrency
opts.CopyObjectThreshold = CopyObjectThreshold
opts.ObjectLockMode = posix.ObjectLockMode(scoutfsLockMode)
opts.DefaultEtag = defaultEtag
opts.DataIntegrityEtag = dataIntegrityEtag
opts.SetNewDirPerm(fs.FileMode(dirPerms))
+4 -1
View File
@@ -125,9 +125,12 @@ func main() {
app := initApp()
posixCommand := gwcli.PosixCommand()
posixCommand.Before = func(ctx *cli.Context) error {
if ctx.Bool("disable-object-lock-file") {
if ctx.Bool("disable-object-lock-file") || ctx.String("object-lock-mode") == "local" {
fmt.Println("Warning: shared object publish locking disabled; conditional write atomicity is limited to this gateway process")
}
if ctx.String("object-lock-mode") == "none" {
fmt.Println("Warning: conditional writes are disabled because object-lock-mode is none")
}
return nil
}
+12 -8
View File
@@ -704,14 +704,18 @@ ROOT_SECRET_ACCESS_KEY=
# NFS servers that may hang on this call.
#VGW_DISABLE_COPY_FILE_RANGE=false
# The VGW_DISABLE_OBJECT_LOCK_FILE option disables shared advisory lock files
# used for conditional object publishes. This can be necessary on filesystems
# that do not support advisory file locking. When enabled, conditional writes
# are atomic only within a single versitygw process and are unsafe with
# multiple gateway processes sharing the same backend.
# This can be set to true to disable the .vgwlocks directory if running in
# single versitygw instance mode or conditional writes are not important
# to this deployment.
# The VGW_OBJECT_LOCK_MODE option selects the advisory lock used for conditional
# object publishes. Use flock (default) or fcntl only after verifying that the
# selected primitive is cluster-coherent for the backend filesystem and mount
# options. The local mode disables the .vgwlocks directory and is atomic only
# within one versitygw process, so it is unsafe with multiple gateways. The
# none mode rejects conditional writes with a NotImplemented response.
# At startup, flock and fcntl modes verify that their primitive works on the
# filesystem hosting .vgwlocks; this cannot verify cross-node lock coherence.
#VGW_OBJECT_LOCK_MODE=flock
# VGW_DISABLE_OBJECT_LOCK_FILE is a deprecated alias for
# VGW_OBJECT_LOCK_MODE=local.
#VGW_DISABLE_OBJECT_LOCK_FILE=false
# The VGW_ENABLE_O_DIRECT option enables best-effort O_DIRECT for object data