Scope the PID lock to mutating commands #179
@@ -171,18 +171,20 @@ vaultik version
|
||||
|
||||
### locking
|
||||
|
||||
Every command that opens the local index — `snapshot create`, `snapshot
|
||||
list`, `snapshot verify`, `snapshot purge`, `snapshot remove`, `snapshot
|
||||
restore`, `prune`, `info`, and `remote info`/`remote nuke` — takes a
|
||||
Commands that write persistent state — `snapshot create`, `snapshot
|
||||
purge`, `snapshot remove`, `prune`, and `remote nuke` — take a
|
||||
process-wide lock at `$XDG_DATA_HOME/vaultik/vaultik.pid`
|
||||
(`~/.local/share/vaultik/vaultik.pid` on Linux) for the whole run. Only
|
||||
one such command runs at a time: a second one exits immediately with an
|
||||
"already running" error rather than waiting. The lock is not scoped to
|
||||
mutating commands, so read-only commands are affected too — `vaultik
|
||||
snapshot list` fails while a backup is in progress; scoping it so
|
||||
read-only commands run during a backup is tracked in
|
||||
[issue #150](https://git.eeqj.de/sneak/vaultik/issues/150). `config`,
|
||||
`database delete`, `completion`, and `version` do not take the lock.
|
||||
one of them runs at a time: a second one exits immediately with an
|
||||
"already running" error rather than waiting, so two writers can never
|
||||
corrupt the local index or the destination store.
|
||||
|
||||
Read-only commands — `info`, `snapshot list`, `snapshot verify`, and
|
||||
`remote info` — do not take the lock and are never blocked, so they run
|
||||
even while a backup is in progress. `snapshot restore` does not take the
|
||||
lock either: it writes only to the target directory you name, not the
|
||||
local index or the destination store. `config`, `database delete`,
|
||||
`completion`, and `version` do not take the lock.
|
||||
|
||||
### stdout and stderr
|
||||
|
||||
|
||||
+63
-24
@@ -33,14 +33,33 @@ import (
|
||||
// may take before we give up.
|
||||
const shutdownTimeout = 30 * time.Second
|
||||
|
||||
// AppOptions contains common options for creating the fx application.
|
||||
// It includes the configuration file path, logging options, and additional
|
||||
// fx modules and invocations that should be included in the application.
|
||||
// lockMode says whether a command mutates persistent state — the local
|
||||
// index database or the remote store — and so must hold the process-wide
|
||||
// PID lock, or only reads that state and may run alongside a mutator.
|
||||
type lockMode int
|
||||
|
||||
const (
|
||||
// mutating commands (snapshot create, snapshot purge, snapshot remove,
|
||||
// prune, remote nuke) write the local index or the remote store. They
|
||||
// hold the PID lock so that at most one runs at a time.
|
||||
mutating lockMode = iota
|
||||
// readOnly commands (info, snapshot list, snapshot verify, remote info,
|
||||
// snapshot restore) do not write the local index or the remote store,
|
||||
// so they run without the lock and are never blocked by a running
|
||||
// mutator. restore writes only to the target directory it is given.
|
||||
readOnly
|
||||
)
|
||||
|
||||
// AppOptions contains common options for creating and running the fx
|
||||
// application: the configuration file path, logging options, additional fx
|
||||
// modules and invocations, and whether the command mutates persistent
|
||||
// state (which decides whether it takes the PID lock).
|
||||
type AppOptions struct {
|
||||
ConfigPath string
|
||||
LogOptions log.Options
|
||||
Modules []fx.Option
|
||||
Invokes []fx.Option
|
||||
Mode lockMode
|
||||
}
|
||||
|
||||
// setupGlobals records the startup time and, when an output-suppression
|
||||
@@ -281,14 +300,15 @@ func RunOperation(
|
||||
}
|
||||
|
||||
// runVaultikApp runs the standard single-operation command lifecycle
|
||||
// shared by the list/purge/verify/remove/remote-info subcommands:
|
||||
// shared by the snapshot list/purge/remove and remote nuke subcommands:
|
||||
// resolve the config, then run op against the Vaultik instance through
|
||||
// RunOperation, reporting a failure prefixed with failMsg (suppressed
|
||||
// while suppressErrors is true, e.g. under --json). jsonOutput marks a
|
||||
// command whose stdout is a JSON document: it quiets the UI but, unlike
|
||||
// Quiet, leaves the stderr log level alone.
|
||||
// while suppressErrors is true, e.g. under --json). mode says whether the
|
||||
// command takes the PID lock. jsonOutput marks a command whose stdout is a
|
||||
// JSON document: it quiets the UI but, unlike Quiet, leaves the stderr log
|
||||
// level alone.
|
||||
func runVaultikApp(
|
||||
cmd *cobra.Command, jsonOutput, suppressErrors bool,
|
||||
cmd *cobra.Command, mode lockMode, jsonOutput, suppressErrors bool,
|
||||
failMsg string, op func(v *vaultik.Vaultik) error,
|
||||
) error {
|
||||
configPath, err := ResolveConfigPath()
|
||||
@@ -306,6 +326,7 @@ func runVaultikApp(
|
||||
Quiet: rootFlags.Quiet,
|
||||
JSON: jsonOutput,
|
||||
},
|
||||
Mode: mode,
|
||||
}, op, func(err error) {
|
||||
if suppressErrors {
|
||||
return
|
||||
@@ -319,28 +340,46 @@ func runVaultikApp(
|
||||
// RunWithApp is a helper that creates and runs an fx app with the given options.
|
||||
// It combines NewApp and RunApp into a single convenient function. This is the
|
||||
// preferred way to run CLI commands that need the full application context.
|
||||
// It acquires a PID lock before starting to prevent concurrent instances.
|
||||
// A mutating command takes the process-wide PID lock before starting so that
|
||||
// only one runs at a time; a read-only command runs without it and is not
|
||||
// blocked while a mutator holds the lock (opts.Mode).
|
||||
func RunWithApp(ctx context.Context, opts AppOptions) error {
|
||||
// Acquire PID lock to prevent concurrent instances
|
||||
lockDir := filepath.Join(xdg.DataHome, "vaultik")
|
||||
|
||||
lock, err := pidlock.Acquire(lockDir)
|
||||
release, err := acquireLockIfMutating(opts.Mode,
|
||||
filepath.Join(xdg.DataHome, "vaultik"))
|
||||
if err != nil {
|
||||
if errors.Is(err, pidlock.ErrAlreadyRunning) {
|
||||
return fmt.Errorf("cannot start: %w", err)
|
||||
return err
|
||||
}
|
||||
|
||||
return fmt.Errorf("failed to acquire lock: %w", err)
|
||||
}
|
||||
|
||||
defer func() {
|
||||
err := lock.Release()
|
||||
if err != nil {
|
||||
log.Warn("Failed to release PID lock", "error", err)
|
||||
}
|
||||
}()
|
||||
defer release()
|
||||
|
||||
app := NewApp(opts)
|
||||
|
||||
return RunApp(ctx, app)
|
||||
}
|
||||
|
||||
// acquireLockIfMutating takes the process-wide PID lock in lockDir for a
|
||||
// mutating command and returns a function that releases it. A read-only
|
||||
// command takes no lock, so it returns a no-op release and is never blocked
|
||||
// while a mutator holds the lock. ErrAlreadyRunning (another mutator holds
|
||||
// the lock) is surfaced as a "cannot start" error.
|
||||
func acquireLockIfMutating(mode lockMode, lockDir string) (func(), error) {
|
||||
if mode != mutating {
|
||||
return func() {}, nil
|
||||
}
|
||||
|
||||
lock, err := pidlock.Acquire(lockDir)
|
||||
if err != nil {
|
||||
if errors.Is(err, pidlock.ErrAlreadyRunning) {
|
||||
return nil, fmt.Errorf("cannot start: %w", err)
|
||||
}
|
||||
|
||||
return nil, fmt.Errorf("failed to acquire lock: %w", err)
|
||||
}
|
||||
|
||||
return func() {
|
||||
err := lock.Release()
|
||||
if err != nil {
|
||||
log.Warn("Failed to release PID lock", "error", err)
|
||||
}
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -2,7 +2,10 @@ package cli //nolint:testpackage // needs access to unexported cleanStartupError
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/vaultik/internal/pidlock"
|
||||
)
|
||||
|
||||
func TestCleanStartupError(t *testing.T) {
|
||||
@@ -53,3 +56,42 @@ func TestCleanStartupError(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestLockScopedToMutatingCommands proves the partition the PID lock now
|
||||
// enforces: a read-only command runs while a mutator holds the lock, and
|
||||
// two mutating commands still mutually exclude.
|
||||
func TestLockScopedToMutatingCommands(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
lockDir := filepath.Join(t.TempDir(), "vaultik")
|
||||
|
||||
// A mutating command takes the process-wide lock.
|
||||
releaseMutator, err := acquireLockIfMutating(mutating, lockDir)
|
||||
if err != nil {
|
||||
t.Fatalf("mutating command could not acquire lock: %v", err)
|
||||
}
|
||||
|
||||
// A read-only command runs to completion even while the lock is held.
|
||||
releaseReader, err := acquireLockIfMutating(readOnly, lockDir)
|
||||
if err != nil {
|
||||
t.Fatalf("read-only command was blocked by held lock: %v", err)
|
||||
}
|
||||
|
||||
releaseReader()
|
||||
|
||||
// A second mutating command is refused while the first holds the lock.
|
||||
_, err = acquireLockIfMutating(mutating, lockDir)
|
||||
if !errors.Is(err, pidlock.ErrAlreadyRunning) {
|
||||
t.Fatalf("second mutating command was not excluded, got: %v", err)
|
||||
}
|
||||
|
||||
// Once the first mutator releases, another mutating command may run.
|
||||
releaseMutator()
|
||||
|
||||
release, err := acquireLockIfMutating(mutating, lockDir)
|
||||
if err != nil {
|
||||
t.Fatalf("mutating command could not acquire released lock: %v", err)
|
||||
}
|
||||
|
||||
release()
|
||||
}
|
||||
|
||||
@@ -35,6 +35,7 @@ func NewInfoCommand() *cobra.Command {
|
||||
Debug: rootFlags.Debug,
|
||||
Quiet: rootFlags.Quiet,
|
||||
},
|
||||
Mode: readOnly,
|
||||
}, func(v *vaultik.Vaultik) error {
|
||||
return v.ShowInfo()
|
||||
}, func(err error) {
|
||||
|
||||
@@ -44,6 +44,7 @@ work (e.g. after a crashed backup or to reclaim storage).`,
|
||||
Quiet: rootFlags.Quiet,
|
||||
JSON: opts.JSON,
|
||||
},
|
||||
Mode: mutating,
|
||||
}, func(v *vaultik.Vaultik) error {
|
||||
return v.Prune(opts)
|
||||
}, func(err error) {
|
||||
|
||||
@@ -45,7 +45,7 @@ This is destructive and irreversible. Requires --force.`,
|
||||
return errNukeNeedsForce
|
||||
}
|
||||
|
||||
return runVaultikApp(cmd, false, false, "Remote nuke failed",
|
||||
return runVaultikApp(cmd, mutating, false, false, "Remote nuke failed",
|
||||
func(v *vaultik.Vaultik) error {
|
||||
return v.NukeRemote(true)
|
||||
})
|
||||
@@ -88,6 +88,7 @@ func newRemoteInfoCommand() *cobra.Command {
|
||||
Quiet: rootFlags.Quiet,
|
||||
JSON: jsonOutput,
|
||||
},
|
||||
Mode: readOnly,
|
||||
}, func(v *vaultik.Vaultik) error {
|
||||
return v.RemoteInfo(jsonOutput)
|
||||
}, func(err error) {
|
||||
|
||||
@@ -92,6 +92,7 @@ specifying a path using --config or by setting VAULTIK_CONFIG to a path.`,
|
||||
Cron: opts.Cron,
|
||||
Quiet: rootFlags.Quiet,
|
||||
},
|
||||
Mode: mutating,
|
||||
}, func(v *vaultik.Vaultik) error {
|
||||
return v.CreateSnapshot(opts)
|
||||
}, func(err error) {
|
||||
@@ -125,7 +126,7 @@ func newSnapshotListCommand() *cobra.Command {
|
||||
Long: "Lists all snapshots with their ID, timestamp, and compressed size",
|
||||
Args: cobra.NoArgs,
|
||||
RunE: func(cmd *cobra.Command, _ []string) error {
|
||||
return runVaultikApp(cmd, false, false,
|
||||
return runVaultikApp(cmd, readOnly, false, false,
|
||||
"Failed to list snapshots",
|
||||
func(v *vaultik.Vaultik) error {
|
||||
return v.ListSnapshots(jsonOutput)
|
||||
@@ -161,7 +162,7 @@ restrict the operation to specific snapshot names.`,
|
||||
return errPurgeCriteriaBoth
|
||||
}
|
||||
|
||||
return runVaultikApp(cmd, false, false,
|
||||
return runVaultikApp(cmd, mutating, false, false,
|
||||
"Failed to purge snapshots",
|
||||
func(v *vaultik.Vaultik) error {
|
||||
return v.PurgeSnapshotsWithOptions(opts)
|
||||
@@ -212,6 +213,7 @@ func newSnapshotVerifyCommand() *cobra.Command {
|
||||
Quiet: rootFlags.Quiet,
|
||||
JSON: opts.JSON,
|
||||
},
|
||||
Mode: readOnly,
|
||||
}, func(v *vaultik.Vaultik) error {
|
||||
return v.VerifySnapshotWithOptions(snapshotID, opts)
|
||||
}, func(err error) {
|
||||
@@ -259,7 +261,7 @@ To wipe the entire destination store and start over, use 'vaultik remote
|
||||
nuke --force' — it is the single supported entry point for that.`,
|
||||
Args: requireSnapshotIDArg,
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
return runVaultikApp(cmd, opts.JSON, opts.JSON,
|
||||
return runVaultikApp(cmd, mutating, opts.JSON, opts.JSON,
|
||||
"Failed to remove snapshot",
|
||||
func(v *vaultik.Vaultik) error {
|
||||
_, err := v.RemoveSnapshot(args[0], opts)
|
||||
|
||||
@@ -88,6 +88,7 @@ func runRestore(cmd *cobra.Command, args []string, opts *RestoreOptions) error {
|
||||
Debug: rootFlags.Debug,
|
||||
Quiet: rootFlags.Quiet,
|
||||
},
|
||||
Mode: readOnly,
|
||||
}, func(v *vaultik.Vaultik) error {
|
||||
return v.Restore(&vaultik.RestoreOptions{
|
||||
SnapshotID: snapshotID,
|
||||
|
||||
Reference in New Issue
Block a user