Storage Engine v4 — Parent: option plumbing + magic-byte dispatch#867
Storage Engine v4 — Parent: option plumbing + magic-byte dispatch#867c1-squire-dev[bot] wants to merge 1 commit into
Conversation
| // See WithC1FV2GrantsWriter for details. | ||
| // WithEngine selects the storage engine for newly created .c1z files. | ||
| // Default is EngineSQLite (v1 format). EnginePebble enables the v3 | ||
| // engine; under default build tags it returns ErrEngineNotAvailable | ||
| // when the file is opened. | ||
| // | ||
| // Reading existing files dispatches on the file's magic byte and is | ||
| // independent of this option. | ||
| func WithEngine(engine Engine) C1ZOption { | ||
| return func(o *c1zOptions) { | ||
| o.engine = engine | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: The WithEngine doc comment is inserted directly after the WithV2GrantsWriter doc comment with no blank line, so Go treats them as a single comment block. This means WithEngine's godoc starts with "WithV2GrantsWriter toggles the slim-blob writer path for grants…" and WithV2GrantsWriter (line 312) loses its doc comment entirely. Add a blank line to separate them.
| // See WithC1FV2GrantsWriter for details. | |
| // WithEngine selects the storage engine for newly created .c1z files. | |
| // Default is EngineSQLite (v1 format). EnginePebble enables the v3 | |
| // engine; under default build tags it returns ErrEngineNotAvailable | |
| // when the file is opened. | |
| // | |
| // Reading existing files dispatches on the file's magic byte and is | |
| // independent of this option. | |
| func WithEngine(engine Engine) C1ZOption { | |
| return func(o *c1zOptions) { | |
| o.engine = engine | |
| } | |
| // WithEngine selects the storage engine for newly created .c1z files. | |
| // Default is EngineSQLite (v1 format). EnginePebble enables the v3 | |
| // engine; under default build tags it returns ErrEngineNotAvailable | |
| // when the file is opened. | |
| // | |
| // Reading existing files dispatches on the file's magic byte and is | |
| // independent of this option. | |
| func WithEngine(engine Engine) C1ZOption { | |
| return func(o *c1zOptions) { | |
| o.engine = engine | |
| } | |
| } | |
| // WithV2GrantsWriter toggles the slim-blob writer path for grants. | |
| // See WithC1FV2GrantsWriter for details. |
| v2GrantsWriter bool | ||
|
|
||
| // engine is the storage engine to use for newly created files. | ||
| // Reads dispatch on magic byte regardless of this value. Default |
There was a problem hiding this comment.
🟡 Suggestion: The zero value of Engine is "", not EngineSQLite ("sqlite"). The comment says "Default is EngineSQLite" but code that later checks engine == EngineSQLite won't match the zero value. Consider initializing this field to EngineSQLite in the constructor, or documenting that "" means "use SQLite" so downstream switch statements handle it correctly.
General PR Review: Storage Engine v4 — Parent: option plumbing + magic-byte dispatchBlocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0 Review SummaryThis PR adds foundational plumbing for the v3 storage engine: Security IssuesNone found. Correctness IssuesNone found. SuggestionsNone. |
… dispatch
Adds the SDK-side scaffolding that the storage-engine-v4 stack depends on,
with zero behavior change when the new options are unset.
- `pkg/dotc1z/format.go` — `C1ZFormat` enum (V1, V3, Unknown), the
`C1Z3FileHeader` magic bytes, `Engine` type with `EngineSQLite` /
`EnginePebble` constants, `ErrEngineNotAvailable` sentinel, and
`ReadHeaderFormat` (format-aware peer of `ReadHeader`).
- `pkg/dotc1z/c1file.go` — new `engine` field on `C1File` and
`c1zOptions`; new `WithC1FEngine` and `WithEngine` options.
- `pkg/dotc1z/format_test.go` — coverage of magic-byte detection,
seeker rewind, format String(), engine constants, sentinel.
- `docs/rfcs/0004-storage-engine-v4/tracker.md` — work tracker for
the rest of the PR stack.
The Pebble engine itself, the v3 envelope reader/writer, the codegen
plugin, and the equivalence harness land in subsequent stacked PRs.
Default-tag builds get no new dependencies; the v3 engine package is
`//go:build batonsdkv2`-gated and arrives in Stack 1.
Refs: RFC v4 (canonical at `/pebble-baton-sdk/rfc-v4.md` in the squire
planner).
c1-squire-dev
Bot
force-pushed
the
pquerna/storage-v4-parent
branch
from
4ea29f4 to
464a8f3
Compare
|
Review feedback addressed in commit 464a8f3 (force-pushed): btipling — `WithEngine` option was dropped during `c1zOptions → c1fopts` translation. Fixed: threaded through in `NewC1ZFile` (line ~362), plus regression test `TestWithEngineThreadsToC1File` that fails on the old code and passes now. pr-review bot — Two findings:
Stacks 1–5 have been rebased forward onto the new Parent and force-pushed. A combined squash PR #874 is also up for macro-level review of the whole stack at once. |
2 participants
Summary
First commit in the v3 storage-engine stack (RFC 0004). No behavior change — adds the option-plumbing API that downstream stacks build on:
dotc1z.EngineSQLite/EnginePebbleengine constants (defaultEngineSQLite).dotc1z.C1ZFormatV3constant +C1Z3FileHeader = []byte("C1Z3\x00").WithEngine(Engine)/WithC1FEngine(Engine)functional options.ReadHeaderFormat(io.ReadSeeker) C1ZFormat— magic-byte dispatch reading the first 5 bytes to distinguish v1 (C1Z\0\0) from v3 (C1Z3\0).//go:build batonsdkv2reserved for the engine package; not yet referenced (Stacks 1-5 add the gated code).tracker.md(committed underdocs/rfcs/0004-storage-engine-v4/) tracks the work remaining across the stack.Stack order
This is the parent of:
Test plan
make lintcleanmake test/pkg PKGS=./pkg/dotc1z/...passes🤖 Generated with Claude Code