From fce253fa39d4e313e7d2472006a701edb3c9760f Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Thu, 8 Oct 2026 03:29:07 +0200 Subject: [PATCH] Give a second snapshot create in the same second its own ID (closes #270) The timestamp in a snapshot ID is in whole seconds, so a `snapshot create` that started in the same second as the previous run of that snapshot name got the same ID, and inserting its row failed with `UNIQUE constraint failed: snapshots.id`. CreateSnapshotWithName now looks the ID up in the local index first and, if it is taken, waits a second and takes a new timestamp. The ID format is unchanged. Only the local index is checked. That is where the insert fails, and the process lock serializes runs, so nothing takes the ID between the lookup and the insert. Model: opus-5-5 --- TODO.md | 8 ++++++++ internal/snapshot/snapshot.go | 33 ++++++++++++++++++++++-------- internal/snapshot/snapshot_test.go | 33 ++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/TODO.md b/TODO.md index 52719c6..9d6ca17 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,14 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-08: Made a second `snapshot create` of one name succeed when + it starts in the same second as the first + ([issue #270](https://git.eeqj.de/sneak/vaultik/issues/270)). The + timestamp in a snapshot ID is in whole seconds, so the second run got + the first run's ID and failed with `UNIQUE constraint failed: + snapshots.id`. When the local index already has a snapshot with the + ID, the create now waits a second and takes a new timestamp. + - 2026-10-07: Made a symlink whose target cannot be read stop the backup ([issue #269](https://git.eeqj.de/sneak/vaultik/issues/269)). It was left out of the snapshot with only a debug log line, even without diff --git a/internal/snapshot/snapshot.go b/internal/snapshot/snapshot.go index 4e59092..05e1ac1 100644 --- a/internal/snapshot/snapshot.go +++ b/internal/snapshot/snapshot.go @@ -115,20 +115,37 @@ func ShortHostname(hostname string) string { // CreateSnapshotWithName creates a new snapshot record with an optional // snapshot name. The snapshot ID format is: hostname_name_timestamp or -// hostname_timestamp if name is empty. +// hostname_timestamp if name is empty. The timestamp is in whole seconds. +// If the local index already has a snapshot with that ID, from a run of the +// same name that started in the same second, it waits a second and takes a +// new timestamp. func (sm *SnapshotManager) CreateSnapshotWithName( ctx context.Context, hostname, name, version, gitRevision string, ) (string, error) { shortHostname := ShortHostname(hostname) - // Build snapshot ID with optional name - timestamp := time.Now().UTC().Format("2006-01-02T15:04:05Z") - var snapshotID string - if name != "" { - snapshotID = fmt.Sprintf("%s_%s_%s", shortHostname, name, timestamp) - } else { - snapshotID = fmt.Sprintf("%s_%s", shortHostname, timestamp) + + for { + // Build snapshot ID with optional name + timestamp := time.Now().UTC().Format("2006-01-02T15:04:05Z") + + if name != "" { + snapshotID = fmt.Sprintf("%s_%s_%s", shortHostname, name, timestamp) + } else { + snapshotID = fmt.Sprintf("%s_%s", shortHostname, timestamp) + } + + existing, err := sm.repos.Snapshots.GetByID(ctx, snapshotID) + if err != nil { + return "", fmt.Errorf("looking up snapshot %s: %w", snapshotID, err) + } + + if existing == nil { + break + } + + time.Sleep(time.Second) } snapshot := &database.Snapshot{ diff --git a/internal/snapshot/snapshot_test.go b/internal/snapshot/snapshot_test.go index a0b31bc..38e38d8 100644 --- a/internal/snapshot/snapshot_test.go +++ b/internal/snapshot/snapshot_test.go @@ -386,3 +386,36 @@ func TestCleanSnapshotDBNonExistentSnapshot(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +// Two creates of one snapshot name back to back start within one second, +// the resolution of the timestamp in a snapshot ID. See +// https://git.eeqj.de/sneak/vaultik/issues/270. +func TestCreateSnapshotWithNameTwiceBackToBack(t *testing.T) { + log.Initialize(log.Config{}) + t.Parallel() + + ctx := context.Background() + + db, err := database.New(ctx, filepath.Join(t.TempDir(), "index.sqlite")) + if err != nil { + t.Fatalf("failed to create database: %v", err) + } + + defer func() { _ = db.Close() }() + + sm := &SnapshotManager{repos: database.NewRepositories(db)} + + first, err := sm.CreateSnapshotWithName(ctx, "test-host", "data", "v", "g") + if err != nil { + t.Fatalf("first create failed: %v", err) + } + + second, err := sm.CreateSnapshotWithName(ctx, "test-host", "data", "v", "g") + if err != nil { + t.Fatalf("second create failed: %v", err) + } + + if first == second { + t.Fatalf("both creates returned snapshot ID %s", first) + } +}