From af810258e67debd9871483f2aad41ce8440c3e78 Mon Sep 17 00:00:00 2001 From: twotalesanimation <80506065+twotalesanimation@users.noreply.github.com> Date: Fri, 14 Aug 2026 09:29:04 +0200 Subject: [PATCH] perf(delivery): stop re-reading hard links to verify them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 25-shot episode took about an hour to package when the linking itself is instant. All of it was verification: every delivered file was read back and hashed, and at ~100 MB/s over SMB one 2.8 GB shot costs 30 s. Re-hashing a hard link cannot disagree with validation. The entry is a second name for the same data, so it re-reads the exact bytes that were already hashed — half an hour per episode to prove something that cannot be false. Verification is now proportional to what was actually done: copies are read back in full, links carry over the validation-time digest. Linking has one gap hashing covered incidentally: a source changed after it passed QC, which a link would carry faithfully. Source timestamps are now checked against the export's validation time, which costs a stat per file rather than a full read. "verifyMode": "always-hash" restores unconditional read-back for a production that requires it. Worker 1.3.0. Co-Authored-By: Claude Opus 5 --- DELIVERY_WORKFLOW.md | 25 ++++-- .../DeliveryStageTests.cs | 78 +++++++++++++++++-- RenderWorker/VFXReviewWorker/DeliveryStage.cs | 55 ++++++++++--- RenderWorker/VFXReviewWorker/Dtos.cs | 4 + .../VFXReviewWorker/VFXReviewWorker.csproj | 2 +- lib/render-pipeline/deliveries.ts | 25 +++++- 6 files changed, 165 insertions(+), 24 deletions(-) diff --git a/DELIVERY_WORKFLOW.md b/DELIVERY_WORKFLOW.md index 451f09c..d304490 100644 --- a/DELIVERY_WORKFLOW.md +++ b/DELIVERY_WORKFLOW.md @@ -114,16 +114,28 @@ never claims to have linked when it copied. ## Verification -Every file is read back after packaging and hashed, and the sequence digest is -compared against the one recorded at validation time. `fsutil hardlink list` does -not work over SMB, so link identity is never queried — reading the bytes covers -links and copies through one path. +Verification is proportional to what the builder actually did, which is what +keeps a package fast: + +| Mode | How it verifies | Cost for a 25-shot episode | +|---|---|---| +| **Hard-linked** (default) | The entry *is* the source data, so the validation-time digest already describes it. Checks each file exists, and that no source was modified after it passed QC. | seconds | +| **Copied** | Read back in full and hashed; the sequence digest must match the one recorded at validation. | ~30 s per 2.8 GB shot | + +Re-reading a hard link cannot disagree with validation — it is the same bytes +under a second name — and at ~100 MB/s over SMB it costs roughly half an hour +per episode to prove something that cannot be false. A copy is genuinely +different: it can be truncated or corrupted, so it is always read back. + +Set `"verifyMode": "always-hash"` in the delivery config to read everything back +regardless, for a production that requires it contractually. An item fails, and the whole package fails with it, when: | Failure | Meaning | |---|---| -| Checksum mismatch | The delivered bytes differ from what passed QC | +| Checksum mismatch (copied items) | The delivered bytes differ from what passed QC | +| A source file modified after validation | The render changed after it passed QC — a link would carry the new bytes faithfully | | Frame count differs from the export record | The render folder changed after QC — someone re-rendered in place | | Source folder missing | The render is not where the export says it is | | No matching files | The folder exists but holds nothing matching the output pattern | @@ -186,7 +198,8 @@ key by key, so a project can override just `deliveryRoot`. "includeMovs": true, "checksumAlgo": "xxh64", "reportFormats": ["csv", "json"], - "copyMode": "auto" + "copyMode": "auto", + "verifyMode": "auto" } ``` diff --git a/RenderWorker/VFXReviewWorker.Tests/DeliveryStageTests.cs b/RenderWorker/VFXReviewWorker.Tests/DeliveryStageTests.cs index 13ad93b..52a227d 100644 --- a/RenderWorker/VFXReviewWorker.Tests/DeliveryStageTests.cs +++ b/RenderWorker/VFXReviewWorker.Tests/DeliveryStageTests.cs @@ -123,17 +123,80 @@ public class DeliveryStageTests : IDisposable } [Fact] - public async Task ChecksumMismatchFailsTheItem() + public async Task ChecksumMismatchFailsACopiedItem() { WriteFrames(3); - var result = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, - Manifest(checksum: "deadbeefdeadbeef"), "machine1", CancellationToken.None, CancellationToken.None); + var manifest = Manifest(checksum: "deadbeefdeadbeef", copyMode: "always-copy"); + var result = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, manifest, "machine1", + CancellationToken.None, CancellationToken.None); Assert.False(result.Ok); Assert.Equal("FAILED", result.Items[0].Status); Assert.Contains("Checksum mismatch", result.Items[0].ErrorMessage!); } + [Fact] + public async Task HardLinkedItemsAreNotReadBack() + { + // A hard link is the same data as the source, so re-hashing it re-reads + // the exact bytes validation already hashed — half an hour per episode + // over SMB to prove something that cannot be false. A deliberately wrong + // recorded checksum is therefore not consulted on the linked path, and + // the validation-time digest carries over unchanged. + WriteFrames(3); + var result = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, + Manifest(checksum: "deadbeefdeadbeef"), "machine1", CancellationToken.None, CancellationToken.None); + + Assert.True(result.Ok); + Assert.Equal("VERIFIED", result.Items[0].Status); + Assert.Equal("hardlink", result.Items[0].LinkMode); + Assert.Equal("deadbeefdeadbeef", result.Items[0].Checksum); + } + + [Fact] + public async Task AlwaysHashForcesReadBackEvenWhenLinked() + { + WriteFrames(3); + var manifest = Manifest(checksum: "deadbeefdeadbeef"); + manifest.VerifyMode = "always-hash"; + var result = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, manifest, "machine1", + CancellationToken.None, CancellationToken.None); + + Assert.False(result.Ok); + Assert.Contains("Checksum mismatch", result.Items[0].ErrorMessage!); + } + + [Fact] + public async Task SourceTouchedAfterValidationFailsTheLinkedItem() + { + // The one thing linking cannot rule out: the source changing after it + // passed QC. The link would faithfully carry the new bytes. + WriteFrames(3); + var manifest = Manifest(); + manifest.Items[0].ValidatedAt = DateTimeOffset.UtcNow.AddHours(-2); + File.SetLastWriteTimeUtc( + Path.Combine(_sourceDir, "SHOT_001_cmp_TT_v001.01002.exr"), DateTime.UtcNow); + + var result = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, manifest, "machine1", + CancellationToken.None, CancellationToken.None); + + Assert.False(result.Ok); + Assert.Contains("modified after validation", result.Items[0].ErrorMessage!); + } + + [Fact] + public async Task UntouchedSourcesPassTheValidationTimestampCheck() + { + WriteFrames(3); + var manifest = Manifest(); + manifest.Items[0].ValidatedAt = DateTimeOffset.UtcNow.AddHours(1); + + var result = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, manifest, "machine1", + CancellationToken.None, CancellationToken.None); + + Assert.True(result.Ok); + } + [Fact] public async Task FrameCountDriftSinceQcFailsTheItem() { @@ -163,14 +226,15 @@ public class DeliveryStageTests : IDisposable public async Task ChecksumMatchesTheValidationTimeDigestForIdenticalContent() { WriteFrames(3); - var first = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, Manifest(), "machine1", - CancellationToken.None, CancellationToken.None); + var first = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, + Manifest(copyMode: "always-copy"), "machine1", CancellationToken.None, CancellationToken.None); // Feeding the recorded digest back in must verify, which is exactly what - // the delivery builder does against Export.checksum. + // the delivery builder does against Export.checksum on the copy path. Directory.Delete(_packageDir, recursive: true); var second = await NewStage().RunAsync(new ClaimedJob { Id = "job1" }, - Manifest(checksum: first.Items[0].Checksum), "machine1", CancellationToken.None, CancellationToken.None); + Manifest(checksum: first.Items[0].Checksum, copyMode: "always-copy"), "machine1", + CancellationToken.None, CancellationToken.None); Assert.True(second.Ok); } diff --git a/RenderWorker/VFXReviewWorker/DeliveryStage.cs b/RenderWorker/VFXReviewWorker/DeliveryStage.cs index f8e3ff8..f95cd88 100644 --- a/RenderWorker/VFXReviewWorker/DeliveryStage.cs +++ b/RenderWorker/VFXReviewWorker/DeliveryStage.cs @@ -37,6 +37,7 @@ public sealed class DeliveryStage var results = new List(); var manifestFiles = new List(); var alwaysCopy = string.Equals(manifest.CopyMode, "always-copy", StringComparison.OrdinalIgnoreCase); + var alwaysHash = string.Equals(manifest.VerifyMode, "always-hash", StringComparison.OrdinalIgnoreCase); _log.LogInformation("Building delivery package {Name} ({Count} shots) at {Dir}", manifest.PackageName, manifest.Items.Count, packageDir); @@ -50,7 +51,7 @@ public sealed class DeliveryStage await _api.ProgressAsync(job.Id, machineId, (double)i / manifest.Items.Count, i, manifest.Items.Count, null, $"Packaging {item.ShotCode} {item.VersionString} ({i + 1}/{manifest.Items.Count})", shutdown); - var result = BuildItem(item, mapper, alwaysCopy, manifestFiles); + var result = BuildItem(item, mapper, alwaysCopy, alwaysHash, manifestFiles); results.Add(result); if (result.Status == "FAILED") { @@ -77,7 +78,7 @@ public sealed class DeliveryStage } private DeliveryItemResult BuildItem( - DeliveryManifestItem item, PathMapper mapper, bool alwaysCopy, List manifestFiles) + DeliveryManifestItem item, PathMapper mapper, bool alwaysCopy, bool alwaysHash, List manifestFiles) { var sourceDir = mapper.Map(item.SourceDir); var shotDir = mapper.Map(item.ShotDir); @@ -121,15 +122,51 @@ public sealed class DeliveryStage totalBytes += new FileInfo(dest).Length; } - // Verification reads the delivered bytes back, which works the same - // whether the entry is a link or a copy — `fsutil hardlink list` is - // not available over SMB, so link identity cannot be queried. - var digest = SequenceChecksum(sources.Select(s => Path.Combine(shotDir, Path.GetFileName(s)))); - if (item.Checksum is { Length: > 0 } && !string.Equals(digest, item.Checksum, StringComparison.OrdinalIgnoreCase)) + // Verification is proportional to what was actually done. + // + // A hard link is a second name for the same data, so hashing it + // re-reads the exact bytes validation already hashed — it cannot + // disagree, and at ~100 MB/s over SMB it costs half an hour for an + // episode to prove nothing. A copy is different: it can be + // truncated or corrupted, so it is read back in full. + string? digest; + string verifiedBy; + + if (usedCopy || alwaysHash) { - return DeliveryItemResult.Failed(item, - $"Checksum mismatch after packaging: expected {item.Checksum}, got {digest}"); + digest = SequenceChecksum(sources.Select(s => Path.Combine(shotDir, Path.GetFileName(s)))); + if (item.Checksum is { Length: > 0 } && !string.Equals(digest, item.Checksum, StringComparison.OrdinalIgnoreCase)) + { + return DeliveryItemResult.Failed(item, + $"Checksum mismatch after packaging: expected {item.Checksum}, got {digest}"); + } + verifiedBy = "hash"; } + else + { + // The one thing linking cannot rule out is the source having + // changed since it passed QC — the link would faithfully carry + // the new bytes. Timestamps answer that for the price of a stat. + if (item.ValidatedAt is DateTimeOffset validatedAt) + { + var touched = sources + .Select(s => new FileInfo(s)) + .Where(f => f.LastWriteTimeUtc > validatedAt.UtcDateTime.AddSeconds(1)) + .ToList(); + if (touched.Count > 0) + { + return DeliveryItemResult.Failed(item, + $"{touched.Count} source file(s) were modified after validation " + + $"({touched[0].Name} at {touched[0].LastWriteTimeUtc:u}, validated {validatedAt.UtcDateTime:u}) — " + + "the render changed after it passed QC"); + } + } + // Same data as the source, so the validation-time digest describes + // the delivered files exactly. + digest = item.Checksum; + verifiedBy = "link identity"; + } + _log.LogDebug("{Shot}: verified by {How}", item.ShotCode, verifiedBy); string? movDest = null; if (!string.IsNullOrWhiteSpace(item.DeliveryMovPath)) diff --git a/RenderWorker/VFXReviewWorker/Dtos.cs b/RenderWorker/VFXReviewWorker/Dtos.cs index cb60bdf..04f0acc 100644 --- a/RenderWorker/VFXReviewWorker/Dtos.cs +++ b/RenderWorker/VFXReviewWorker/Dtos.cs @@ -79,6 +79,8 @@ public sealed class DeliveryManifest public string ReportFile { get; set; } = ""; public bool IncludeMovs { get; set; } = true; public string CopyMode { get; set; } = "auto"; + /// "auto" verifies copies by hash and links by identity; "always-hash" reads everything back. + public string VerifyMode { get; set; } = "auto"; public string ChecksumAlgo { get; set; } = "xxh64"; public List ReportFormats { get; set; } = new(); public List Items { get; set; } = new(); @@ -98,6 +100,8 @@ public sealed class DeliveryManifestItem public string? Checksum { get; set; } public string? DeliveryMovPath { get; set; } public string ShotDir { get; set; } = ""; + /// When this export's validation ran — lets a linked package detect a source changed since QC. + public DateTimeOffset? ValidatedAt { get; set; } } /// PREVIEW_ONLY manifest (server-built, RenderPipeline2 §9). diff --git a/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj b/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj index 76fb9d7..05120df 100644 --- a/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj +++ b/RenderWorker/VFXReviewWorker/VFXReviewWorker.csproj @@ -15,7 +15,7 @@ every change that gets published. publish.ps1 appends the git SHA via SourceRevisionId, giving "1.1.0+feb1c7b". --> - 1.2.1 + 1.3.0 diff --git a/lib/render-pipeline/deliveries.ts b/lib/render-pipeline/deliveries.ts index c7ac2dd..d3cf0d6 100644 --- a/lib/render-pipeline/deliveries.ts +++ b/lib/render-pipeline/deliveries.ts @@ -25,6 +25,13 @@ export interface DeliveryConfig { checksumAlgo: string; reportFormats: string[]; copyMode: "auto" | "always-copy"; + /** + * "auto" verifies copies by hash and hard links by identity — a link is the + * same data as the source, so re-reading it cannot disagree with what + * validation already hashed, and at SAN speeds costs ~30 s per shot to prove + * nothing. "always-hash" reads every delivered file back regardless. + */ + verifyMode: "auto" | "always-hash"; } export const DELIVERY_CONFIG_DEFAULT: DeliveryConfig = { @@ -39,6 +46,7 @@ export const DELIVERY_CONFIG_DEFAULT: DeliveryConfig = { checksumAlgo: "xxh64", reportFormats: ["csv", "json"], copyMode: "auto", + verifyMode: "auto", }; /** @@ -333,6 +341,8 @@ export interface DeliveryManifestItem { checksum: string | null; deliveryMovPath: string | null; shotDir: string; + /** When validation last ran for this export; null when it never did. */ + validatedAt: string | null; } export interface DeliveryManifest { @@ -345,6 +355,7 @@ export interface DeliveryManifest { reportFile: string; includeMovs: boolean; copyMode: string; + verifyMode: string; checksumAlgo: string; reportFormats: string[]; items: DeliveryManifestItem[]; @@ -357,7 +368,17 @@ export async function buildDeliveryJobManifest( const pkg = await tx.deliveryPackage.findUniqueOrThrow({ where: { id: packageId }, include: { - items: { include: { export: true } }, + items: { + include: { + export: { + include: { + // Newest validation run: a linked package uses it to detect a + // source file changed after it passed QC. + validations: { orderBy: { createdAt: "desc" }, take: 1, select: { createdAt: true } }, + }, + }, + }, + }, project: { select: { code: true, showId: true } }, }, }); @@ -380,6 +401,7 @@ export async function buildDeliveryJobManifest( reportFile: applyTokens(snapshot.layout.reportFile, baseTokens), includeMovs: snapshot.includeMovs, copyMode: snapshot.copyMode ?? "auto", + verifyMode: snapshot.verifyMode ?? "auto", checksumAlgo: snapshot.checksumAlgo, reportFormats: snapshot.reportFormats, items: pkg.items.map((item) => ({ @@ -394,6 +416,7 @@ export async function buildDeliveryJobManifest( fileCount: item.export.exrFileCount, checksum: item.export.checksum, deliveryMovPath: snapshot.includeMovs ? item.export.deliveryMovPath : null, + validatedAt: item.export.validations[0]?.createdAt.toISOString() ?? null, shotDir: applyTokens(snapshot.layout.shotDir, { ...baseTokens, shotCode: item.shotCode,