perf(daemon): drop duplicate sha256 during snapshot restore (#2)
## Summary During restore, `generationObject` hashed every snapshot object again, after the snapshot adapter had already verified each object read from disk against its digest key. That second hash cost about 4 ms per 10k-row pane, roughly 8–10% of restore CPU. - `ports.SnapshotGeneration.Objects` and `SnapshotRepository.LoadCheckpoint` now document that each object matches its digest key. The adapter (`loadCheckpointLocked`) is where this is enforced. - `generationObject` no longer rehashes. It still checks kind, presence, size and `PreflightObject`, and reports a size mismatch with its own error. ## Trust boundary The digest check is now part of the port contract. There is one production implementation, `snapshot.Repository`, and it hashes on load. A future implementation has to keep that guarantee. ## Tests - Adapter: new `well-formed object with wrong digest` case. It is a same-size, valid envelope that only the hash check can reject, and it fails if that check is removed. - Daemon: new `manifest size mismatch` case. It fails if the size check is removed. - `go test -race` on adapters/snapshot, usecase/daemon and usecase/recovery; `make lint`; `make mocks` (no diff). Reviewed-on: https://git.bnema.dev/bnema/vev/pulls/2
- commit
- f449bae6a5481746b148b05eb212fb66e304e0b4
- author
- brice · · 2026-10-02 12:08 UTC
- committer
- bnema · · 2026-10-02 12:08 UTC
- parent
- 232a7db
- signature
- unsigned
4 files · +33 -15
M internal/adapters/snapshot/direct_load_test.go +15 -0
| @@ -141,6 +141,21 @@ func TestLoadCheckpointDoesNotClassifyCorruptionAsManifestVersion(t *testing.T) | |||
| 141 | 141 | require.NoError(t, os.WriteFile(repo.objectPath(pub.IncarnationID, pub.Objects[0].Digest), []byte("corrupt"), 0o600)) | |
| 142 | 142 | }, | |
| 143 | 143 | }, | |
| 144 | + | { | |
| 145 | + | name: "well-formed object with wrong digest", | |
| 146 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, _ *domain.CheckpointRef) { | |
| 147 | + | t.Helper() | |
| 148 | + | object := pub.Objects[0] | |
| 149 | + | kind, payload, err := codec.UnmarshalObject(object.Data) | |
| 150 | + | require.NoError(t, err) | |
| 151 | + | payload = append([]byte(nil), payload...) | |
| 152 | + | payload[len(payload)-1] ^= 1 | |
| 153 | + | replacement, err := codec.MarshalObject(kind, payload) | |
| 154 | + | require.NoError(t, err) | |
| 155 | + | require.Len(t, replacement.Data, len(object.Data)) | |
| 156 | + | require.NoError(t, os.WriteFile(repo.objectPath(pub.IncarnationID, object.Digest), replacement.Data, 0o600)) | |
| 157 | + | }, | |
| 158 | + | }, | |
| 144 | 159 | } { | |
| 145 | 160 | t.Run(tt.name, func(t *testing.T) { | |
| 146 | 161 | repo := NewRepository(privateDir(t)) | |
| @@ -141,6 +141,21 @@ func TestLoadCheckpointDoesNotClassifyCorruptionAsManifestVersion(t *testing.T) | |||||
| 141 | require.NoError(t, os.WriteFile(repo.objectPath(pub.IncarnationID, pub.Objects[0].Digest), []byte("corrupt"), 0o600)) | 141 | require.NoError(t, os.WriteFile(repo.objectPath(pub.IncarnationID, pub.Objects[0].Digest), []byte("corrupt"), 0o600)) | ||
| 142 | }, | 142 | }, | ||
| 143 | }, | 143 | }, | ||
| 144 | + | { | |||
| 145 | + | name: "well-formed object with wrong digest", | |||
| 146 | + | mutate: func(t *testing.T, repo *Repository, pub ports.SnapshotPublication, _ *domain.CheckpointRef) { | |||
| 147 | + | t.Helper() | |||
| 148 | + | object := pub.Objects[0] | |||
| 149 | + | kind, payload, err := codec.UnmarshalObject(object.Data) | |||
| 150 | + | require.NoError(t, err) | |||
| 151 | + | payload = append([]byte(nil), payload...) | |||
| 152 | + | payload[len(payload)-1] ^= 1 | |||
| 153 | + | replacement, err := codec.MarshalObject(kind, payload) | |||
| 154 | + | require.NoError(t, err) | |||
| 155 | + | require.Len(t, replacement.Data, len(object.Data)) | |||
| 156 | + | require.NoError(t, os.WriteFile(repo.objectPath(pub.IncarnationID, object.Digest), replacement.Data, 0o600)) | |||
| 157 | + | }, | |||
| 158 | + | }, | |||
| 144 | } { | 159 | } { | ||
| 145 | t.Run(tt.name, func(t *testing.T) { | 160 | t.Run(tt.name, func(t *testing.T) { | ||
| 146 | repo := NewRepository(privateDir(t)) | 161 | repo := NewRepository(privateDir(t)) | ||
M internal/ports/recovery.go +6 -1
| @@ -23,7 +23,10 @@ type SnapshotGeneration struct { | |||
| 23 | 23 | Generation uint64 | |
| 24 | 24 | ParentCheckpoint *domain.CheckpointRef | |
| 25 | 25 | Manifest []byte | |
| 26 | - | Objects map[SnapshotDigest][]byte | |
| 26 | + | // Objects maps each object digest to its bytes. Implementations must only | |
| 27 | + | // return objects whose SHA-256 equals their key; consumers rely on this and | |
| 28 | + | // do not rehash. | |
| 29 | + | Objects map[SnapshotDigest][]byte | |
| 27 | 30 | } | |
| 28 | 31 | ||
| 29 | 32 | type Catalogue interface { | |
| @@ -41,6 +44,8 @@ type Catalogue interface { | |||
| 41 | 44 | ||
| 42 | 45 | type SnapshotRepository interface { | |
| 43 | 46 | Publish(context.Context, SnapshotPublication) error | |
| 47 | + | // LoadCheckpoint returns a generation whose manifest matches the reference | |
| 48 | + | // digest and whose objects each match their digest key, or an error. | |
| 44 | 49 | LoadCheckpoint(context.Context, domain.IncarnationID, string, CheckpointRef) (SnapshotGeneration, error) | |
| 45 | 50 | ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error | |
| 46 | 51 | DeleteIncarnation(context.Context, domain.IncarnationID) error | |
| @@ -23,7 +23,10 @@ type SnapshotGeneration struct { | |||||
| 23 | Generation uint64 | 23 | Generation uint64 | ||
| 24 | ParentCheckpoint *domain.CheckpointRef | 24 | ParentCheckpoint *domain.CheckpointRef | ||
| 25 | Manifest []byte | 25 | Manifest []byte | ||
| 26 | - | Objects map[SnapshotDigest][]byte | 26 | + | // Objects maps each object digest to its bytes. Implementations must only |
| 27 | + | // return objects whose SHA-256 equals their key; consumers rely on this and | |||
| 28 | + | // do not rehash. | |||
| 29 | + | Objects map[SnapshotDigest][]byte | |||
| 27 | } | 30 | } | ||
| 28 | 31 | ||||
| 29 | type Catalogue interface { | 32 | type Catalogue interface { | ||
| @@ -41,6 +44,8 @@ type Catalogue interface { | |||||
| 41 | 44 | ||||
| 42 | type SnapshotRepository interface { | 45 | type SnapshotRepository interface { | ||
| 43 | Publish(context.Context, SnapshotPublication) error | 46 | Publish(context.Context, SnapshotPublication) error | ||
| 47 | + | // LoadCheckpoint returns a generation whose manifest matches the reference | |||
| 48 | + | // digest and whose objects each match their digest key, or an error. | |||
| 44 | LoadCheckpoint(context.Context, domain.IncarnationID, string, CheckpointRef) (SnapshotGeneration, error) | 49 | LoadCheckpoint(context.Context, domain.IncarnationID, string, CheckpointRef) (SnapshotGeneration, error) | ||
| 45 | ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error | 50 | ReconcileCheckpoint(context.Context, domain.IncarnationID, CheckpointRef) error | ||
| 46 | DeleteIncarnation(context.Context, domain.IncarnationID) error | 51 | DeleteIncarnation(context.Context, domain.IncarnationID) error | ||
M internal/usecase/daemon/snapshot_restore_incremental.go +6 -2
| @@ -2,7 +2,6 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | - | "crypto/sha256" | |
| 6 | 5 | "errors" | |
| 7 | 6 | "fmt" | |
| 8 | 7 | "net" | |
| @@ -424,10 +423,15 @@ func generationObject(generation ports.SnapshotGeneration, ref snapcodec.ObjectR | |||
| 424 | 423 | if ref.Kind != kind { | |
| 425 | 424 | return nil, fmt.Errorf("snapshot: object kind mismatch") | |
| 426 | 425 | } | |
| 426 | + | // The repository port guarantees every object matches its digest key, so | |
| 427 | + | // the content hash is not recomputed here; see ports.SnapshotGeneration. | |
| 427 | 428 | data, ok := generation.Objects[ref.Digest] | |
| 428 | - | if !ok || uint32(len(data)) != ref.Size || sha256.Sum256(data) != ref.Digest { | |
| 429 | + | if !ok { | |
| 429 | 430 | return nil, fmt.Errorf("snapshot: missing object") | |
| 430 | 431 | } | |
| 432 | + | if uint32(len(data)) != ref.Size { | |
| 433 | + | return nil, fmt.Errorf("snapshot: object size mismatch") | |
| 434 | + | } | |
| 431 | 435 | // The payload aliases the loaded object. A generation's objects are private | |
| 432 | 436 | // to this restore and are only decoded, never mutated, so copying here would | |
| 433 | 437 | // double the transient footprint of every restored history chunk. | |
| @@ -2,7 +2,6 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | - | "crypto/sha256" | |||
| 6 | "errors" | 5 | "errors" | ||
| 7 | "fmt" | 6 | "fmt" | ||
| 8 | "net" | 7 | "net" | ||
| @@ -424,10 +423,15 @@ func generationObject(generation ports.SnapshotGeneration, ref snapcodec.ObjectR | |||||
| 424 | if ref.Kind != kind { | 423 | if ref.Kind != kind { | ||
| 425 | return nil, fmt.Errorf("snapshot: object kind mismatch") | 424 | return nil, fmt.Errorf("snapshot: object kind mismatch") | ||
| 426 | } | 425 | } | ||
| 426 | + | // The repository port guarantees every object matches its digest key, so | |||
| 427 | + | // the content hash is not recomputed here; see ports.SnapshotGeneration. | |||
| 427 | data, ok := generation.Objects[ref.Digest] | 428 | data, ok := generation.Objects[ref.Digest] | ||
| 428 | - | if !ok || uint32(len(data)) != ref.Size || sha256.Sum256(data) != ref.Digest { | 429 | + | if !ok { |
| 429 | return nil, fmt.Errorf("snapshot: missing object") | 430 | return nil, fmt.Errorf("snapshot: missing object") | ||
| 430 | } | 431 | } | ||
| 432 | + | if uint32(len(data)) != ref.Size { | |||
| 433 | + | return nil, fmt.Errorf("snapshot: object size mismatch") | |||
| 434 | + | } | |||
| 431 | // The payload aliases the loaded object. A generation's objects are private | 435 | // The payload aliases the loaded object. A generation's objects are private | ||
| 432 | // to this restore and are only decoded, never mutated, so copying here would | 436 | // to this restore and are only decoded, never mutated, so copying here would | ||
| 433 | // double the transient footprint of every restored history chunk. | 437 | // double the transient footprint of every restored history chunk. | ||
M internal/usecase/daemon/snapshot_restore_test.go +6 -12
| @@ -607,18 +607,12 @@ func TestRestoreIncrementalFallbackAndInvalidObjectMappings(t *testing.T) { | |||
| 607 | 607 | delete(g.Objects, tailRef.Digest) | |
| 608 | 608 | g.Objects[replacement.Digest] = replacement.Data | |
| 609 | 609 | }}, | |
| 610 | - | {"wrong digest", func(g *ports.SnapshotGeneration) { | |
| 611 | - | for digest, data := range g.Objects { | |
| 612 | - | kind, payload, err := snapcodec.UnmarshalObject(data) | |
| 613 | - | require.NoError(t, err) | |
| 614 | - | payload = append([]byte(nil), payload...) | |
| 615 | - | payload[len(payload)-1] ^= 1 | |
| 616 | - | replacement, err := snapcodec.MarshalObject(kind, payload) | |
| 617 | - | require.NoError(t, err) | |
| 618 | - | require.Equal(t, len(data), len(replacement.Data)) | |
| 619 | - | g.Objects[digest] = replacement.Data | |
| 620 | - | break | |
| 621 | - | } | |
| 610 | + | {"manifest size mismatch", func(g *ports.SnapshotGeneration) { | |
| 611 | + | manifest, err := snapcodec.UnmarshalManifest(g.Manifest) | |
| 612 | + | require.NoError(t, err) | |
| 613 | + | manifest.Tabs[0].Panes[0].Tail.Size++ | |
| 614 | + | g.Manifest, err = snapcodec.MarshalManifest(manifest) | |
| 615 | + | require.NoError(t, err) | |
| 622 | 616 | }}, | |
| 623 | 617 | } { | |
| 624 | 618 | t.Run(mutate.name, func(t *testing.T) { | |
| @@ -607,18 +607,12 @@ func TestRestoreIncrementalFallbackAndInvalidObjectMappings(t *testing.T) { | |||||
| 607 | delete(g.Objects, tailRef.Digest) | 607 | delete(g.Objects, tailRef.Digest) | ||
| 608 | g.Objects[replacement.Digest] = replacement.Data | 608 | g.Objects[replacement.Digest] = replacement.Data | ||
| 609 | }}, | 609 | }}, | ||
| 610 | - | {"wrong digest", func(g *ports.SnapshotGeneration) { | 610 | + | {"manifest size mismatch", func(g *ports.SnapshotGeneration) { |
| 611 | - | for digest, data := range g.Objects { | 611 | + | manifest, err := snapcodec.UnmarshalManifest(g.Manifest) |
| 612 | - | kind, payload, err := snapcodec.UnmarshalObject(data) | 612 | + | require.NoError(t, err) |
| 613 | - | require.NoError(t, err) | 613 | + | manifest.Tabs[0].Panes[0].Tail.Size++ |
| 614 | - | payload = append([]byte(nil), payload...) | 614 | + | g.Manifest, err = snapcodec.MarshalManifest(manifest) |
| 615 | - | payload[len(payload)-1] ^= 1 | 615 | + | require.NoError(t, err) |
| 616 | - | replacement, err := snapcodec.MarshalObject(kind, payload) | |||
| 617 | - | require.NoError(t, err) | |||
| 618 | - | require.Equal(t, len(data), len(replacement.Data)) | |||
| 619 | - | g.Objects[digest] = replacement.Data | |||
| 620 | - | break | |||
| 621 | - | } | |||
| 622 | }}, | 616 | }}, | ||
| 623 | } { | 617 | } { | ||
| 624 | t.Run(mutate.name, func(t *testing.T) { | 618 | t.Run(mutate.name, func(t *testing.T) { | ||