docs(block): name the split packages after what the tree already calls them - #2417
Merged
Conversation
…s them The block data-flow split was planned as pier/crane/ferry/harbour. Three of those packages already exist under good names and the fourth is the verb used at every call site, so the split now introduces no new vocabulary: journal, carver, syncer, engine. journal keeps its name and its DFSJRN1 format stamp; the design plan's own objection to it is recorded rather than dropped. carver absorbs pkg/block/ chunker, which is already the same three files. syncer is the type engine already declares, narrowed to transport once carve dispatch leaves it. Also records the measured carve breakdown: buffer zeroing dominates, not chunking or hashing.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of concrete doc inconsistencies/typos introduced in the updated planning docs that should be corrected to avoid confusing readers.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates DittoFS’s block data-flow documentation to use existing in-tree package names (journal / carver / syncer / engine) instead of the previously planned nautical naming, and adjusts related plan text accordingly.
Changes:
- Renames the planned split terminology across the RFC and planning docs to match existing package/type names.
- Rewrites affected rationale sections where a simple find/replace would be misleading.
- Updates one benchmark comment to reference the “design plan” rather than the prior “pier design” wording.
File summaries
| File | Description |
|---|---|
| pkg/block/journal/transition_bench_test.go | Updates a benchmark comment to match the renamed design-plan terminology. |
| docs/internals/rfc-block-dataflow.md | Renames the RFC’s library split vocabulary to journal/carver/syncer. |
| .planning/2026-09-01-journal-library-design-PLAN.md | Renames the design plan vocabulary and rewrites supporting narrative for the new naming. |
| .planning/2026-09-01-block-dataflow-MASTER-PLAN.md | Renames master-plan references and updates sequencing/terminology for the new package names. |
Review details
Suppressed comments (1)
.planning/2026-09-01-journal-library-design-PLAN.md:485
- The rationale sentence is self-contradictory: it says the package is named
syncer“rather thansyncer”, which looks like a leftover from the earlierferryvssyncerdiscussion and makes the naming justification read incorrectly.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Renames the planned block data-flow split from
pier/crane/ferry/harbourtojournal/carver/syncer/engine, across the RFC and both planning docs.Why
Three of the four packages already exist under good names, and the fourth is the verb used at every call site. The split now introduces no new vocabulary at all:
pierjournalpkg/block/journal, and the"DFSJRN1\0"format stamp (segment.go:18)cranecarvercarve.go,CarveChunk,carveMu, twocarve_dispatch.goferrysyncerengine/syncer.goalready declarestype Syncerharbourenginepkg/block/engine, already the composition layersynceris the interesting one: todaySyncerowns both transport and carve dispatch, so the split narrows it and the name becomes accurate rather than merely inherited.What needed rewriting rather than substituting
The docs argued for the nautical names, so a find-and-replace would have left nonsense behind:
pierand explicitly rejectedjournal— "in filesystems that means a metadata write-ahead log; this is the data store itself, and the misnomer is part of whyGCwas read as touching remote refcounts." That objection is kept and recorded, not quietly dropped: it is accepted because the package already carries the name, so the misnomer exists regardless, and renaming would desynchronise the package from its own format magic.doc.gometaphor ("bytes park in a container until a boat carries them out") rewritten to state theSync/Flushdistinction directly.harbourrationale in the master plan rewritten — it argued againstharbourmasteron authority grounds, which no longer parses once the package isengine.Structural changes beyond the rename
pkg/block/chunkerfolds intocarver. It is already FastCDC + gear table +Params— precisely the three files the layout lists undercarver/— so keeping both would be two packages for one job, andParamsis a format contract that belongs with the code that carves by it. Ten files import it today; noted as mechanical but not free, to be done with the extraction.Carveis CPU-bound, but not where the plan implies: on an EPYC 7543, 53% of carve CPU is large-buffer allocation and zeroing underpackRuns, against 23% BLAKE3 and 13% FastCDC. Pooling those buffers needs no format change and no dedup trade, and is worth ~4× what removing content-defined chunking would buy. This corrects the impression left by "FastCDC plus BLAKE3 plus dedup dominate" — that phrasing omits the leading term, andBenchmarkCarvestubs the oracle withmissDeduper, so it never measured dedup at all.Verification
rgforpier|crane|ferry|harbour|piertest|cranetest|ferrytestacross the repo returns nothing.go build ./...andgo vet ./pkg/block/journal/clean. The design-plan file is renamed to match the reference the RFC already carries.Docs and one test comment only — no production code.