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