-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(snapshot): name migration snapshots for the retention commands #9434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
prekshivyas
merged 5 commits into
NVIDIA:main
from
udsy19:fix/snapshot-dir-name-pattern
Aug 19, 2026
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
4790f14
fix(snapshot): name migration snapshots for the retention commands
udsy19 73a8e97
fix(snapshot): reserve each migration snapshot directory atomically
udsy19 f20286e
refactor(snapshot): one snapshot-directory contract for both writers
udsy19 a5cab83
test(snapshot): use a portable temporary root
prekshivyas 658d2f0
test(snapshot): cover migration retention handoff
prekshivyas File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; | ||
| import { tmpdir } from "node:os"; | ||
| import { basename, join } from "node:path"; | ||
|
|
||
| import { afterEach, describe, expect, it } from "vitest"; | ||
|
|
||
| import { | ||
| compactUtcTimestamp, | ||
| reserveSnapshotDir, | ||
| SNAPSHOT_DIR_NAME_RE, | ||
| } from "./snapshot-directory.js"; | ||
|
|
||
| // macOS resolves the default TMPDIR through a /var symlink, which the snapshot delete helper | ||
| // rejects. Other platforms can use their native temporary directory. | ||
| const temporaryRoot = process.platform === "darwin" ? "/private/tmp" : tmpdir(); | ||
| const roots: string[] = []; | ||
|
|
||
| function makeSnapshotsDir(): string { | ||
| const root = mkdtempSync(join(temporaryRoot, "nemoclaw-snapshot-dir-")); | ||
| roots.push(root); | ||
| return join(root, "snapshots"); | ||
| } | ||
|
|
||
| afterEach(() => { | ||
| roots.splice(0).forEach((root) => rmSync(root, { force: true, recursive: true })); | ||
| }); | ||
|
|
||
| describe("blueprint snapshot directory reservation", () => { | ||
| it("names a reserved directory in the grammar the retention reader accepts", () => { | ||
| const reserved = reserveSnapshotDir(makeSnapshotsDir(), Date.parse("2026-08-18T06:43:16.500Z")); | ||
|
|
||
| expect(basename(reserved)).toBe("20260818T064316Z"); | ||
| expect(basename(reserved)).toMatch(SNAPSHOT_DIR_NAME_RE); | ||
| expect(compactUtcTimestamp(Date.parse("2026-08-18T06:43:16.500Z"))).toBe(basename(reserved)); | ||
| }); | ||
|
|
||
| it("gives a same-second reservation the next unused second (#9433)", () => { | ||
| const snapshotsDir = makeSnapshotsDir(); | ||
| const startedAt = Date.parse("2026-08-18T06:43:16.500Z"); | ||
|
|
||
| const first = reserveSnapshotDir(snapshotsDir, startedAt); | ||
| writeFileSync(join(first, "reservation-marker"), "first"); | ||
| const second = reserveSnapshotDir(snapshotsDir, startedAt); | ||
|
|
||
| expect(basename(first)).toBe("20260818T064316Z"); | ||
| expect(basename(second)).toBe("20260818T064317Z"); | ||
| expect(basename(second)).toMatch(SNAPSHOT_DIR_NAME_RE); | ||
| // The second reservation owns an empty directory, so it can neither read nor clean up the first. | ||
| expect(second).not.toBe(first); | ||
| }); | ||
|
|
||
| it("advances past a planted symlink instead of following it", () => { | ||
| const snapshotsDir = makeSnapshotsDir(); | ||
| const startedAt = Date.parse("2026-08-18T06:43:16.500Z"); | ||
| reserveSnapshotDir(snapshotsDir, startedAt); | ||
| rmSync(join(snapshotsDir, "20260818T064316Z"), { recursive: true }); | ||
| symlinkSync("/etc", join(snapshotsDir, "20260818T064316Z")); | ||
|
|
||
| const reserved = reserveSnapshotDir(snapshotsDir, startedAt); | ||
|
|
||
| expect(basename(reserved)).toBe("20260818T064317Z"); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| import { mkdirSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
|
|
||
| /** The snapshot directory grammar that the retention commands accept. */ | ||
| export const SNAPSHOT_DIR_NAME_RE = /^\d{8}T\d{6}Z$/; | ||
|
|
||
| /** A UTC instant in the snapshot directory grammar: 20260818T064316Z. */ | ||
| export function compactUtcTimestamp(at: number = Date.now()): string { | ||
| return new Date(at).toISOString().replace(/[-:]|\.\d+(?=Z)/g, ""); | ||
| } | ||
|
|
||
| /** | ||
| * Reserve one snapshot directory for the calling operation alone. | ||
| * | ||
| * The leaf mkdir is non-recursive, so the reservation is a single atomic syscall: EEXIST means | ||
| * some other snapshot already owns that second, and the caller never writes into, or cleans up, a | ||
| * directory it did not create. The grammar above is second-resolution, so a taken second advances | ||
| * to the next second rather than taking a suffix the retention reader would reject. Each attempt | ||
| * names a later second than the last, so the loop ends at the first unused one. | ||
| * | ||
| * A non-directory entry planted at a candidate name, including a symlink, also fails with EEXIST, | ||
| * so reservation advances past it instead of following it. | ||
| */ | ||
| export function reserveSnapshotDir(snapshotsDir: string, startedAt: number = Date.now()): string { | ||
| mkdirSync(snapshotsDir, { recursive: true }); | ||
| for (let at = startedAt; ; at += 1000) { | ||
| const candidate = join(snapshotsDir, compactUtcTimestamp(at)); | ||
| try { | ||
| mkdirSync(candidate); | ||
| return candidate; | ||
| } catch (error: unknown) { | ||
| if ((error as NodeJS.ErrnoException).code !== "EEXIST") throw error; | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.