Skip to content

full-history: dead pre-wired surface + stale contract docs (~300 lines deletable) #841

Description

@tamirms

Split out from the #820 review's final pass over pre-existing full-history code. The freeze rework left behind API pre-wired for a handoff design that no longer exists; none of it is reachable from production, and several docs will actively misdirect the #772 read-path work.

Deletions (~300 lines):

  • The freeze-handoff mirror surface (~95 lines): ConcurrentBitmaps.Snapshot/Len, ConcurrentLedgerOffsets.Snapshot/EventIDs, and the Clone-safe/COW ownership contract that exists only for them. ConcurrentBitmaps.Snapshot's doc cites a WriteColdIndex call chain that does not exist — the cold index is built from a single-threaded Bitmaps in ingest/events.go.
  • Reader.Lookup (~90 lines): Query uses LookupKeys exclusively; Lookup(k) ≡ LookupKeys([k])[0], and ColdReader.Lookup hand-duplicates the whole resolve pipeline for one key. Deleting it retires ErrTermNotFound.
  • events.TermsFor (~45 lines): test-only; its doc names cold backfill as the consumer, which actually uses TermsForBytes.
  • The dead IndexBaseChunk/IndexFileName naming scheme — OpenColdReader's doc tells future wiring to compose a filename that never exists on disk (the real layout is geometry.Layout.TxHashIndexFilePath).
  • The 20 streaming: prefixes on error strings (config_lock ×7, config ×2, catalog ×6, geometry ×5): delete rather than rename — error context comes from the wrap chain, not self-locating strings, and the prefix is the codename fullhistory/streaming: split the streaming package into purpose-named packages #824 retires anyway. The four Info/Debug log prefixes can follow the same decision.
  • Census tail: ColdWriter.ChunkID()/Dir() + their backing fields + the unused ErrWriterClosed re-export; the unreachable early-success branch in ledgerCold.Finalize (ingest/ledgers.go:55-63); four unreachable guards in concurrent_bitmaps (Get/Snapshot nil/empty branches); three "store closed" sentinels no caller distinguishes (metastore should also translate to stores.* sentinels per the stores/errors.go contract that names it, and its two New guards duplicate rocksdb.New's while emitting the wrong package's sentinel); the eventstore cold writer is the only store that mkdirs its own bucket dir (siblings mkdir at the ingest layer; cold_format.go:52 itself says path composition is the orchestrator's job); tip knobs double-defaulted (daemon.go hardcodes what StartConfig.withDefaults already fills); validateConfig's returned earliest ledger has no consumer; test-only exports ReadColdBin, TxHashIndexLayout.ChunksPerIndex, NewConcurrentBitmaps(), ledger.ColdReader.FirstSeq.

Doc corrections (the #772-misdirection set):

  • The Reader interface still says the freeze loop uses All (reader.go:180) — the freeze re-derives from raw LCMs and never calls it.
  • Three contract comments name the deleted IngestLedgerEvents as the single writer (bitmaps.go:34, concurrent_bitmaps.go:160, concurrent_ledgeroffsets.go:11); the actual writer is HotStore.applyLedger via the post-commit hook.
  • db/event.go:162 claims full-history stores no per-event index — it stores EventIdx explicitly by the byte-stable-ID decision.
  • tuning.go:20-25 says RocksDB's default block size is 16 KiB; it is 4 KiB — making hot_store's two 4-KiB "overrides" no-ops and any future CF sizing reasoned from a 4× wrong baseline.
  • The eventstore package doc still promises a future "RecSplit-backed" cold reader; it exists and is streamhash-MPHF-backed.
  • cold_bin.go credits ReadColdBin's check as the production on-disk guard (it's scanBinHeader) and claims duplicate truncated keys are "preserved" (the streamhash build fails on them — astronomically unlikely, but the doc describes behavior the pipeline doesn't have).
  • Three packfile-laziness comments in eventstore/cold_reader.go are false — packfile.Open starts I/O and holds 2 fds immediately (the ledger store's description is the accurate one).
  • read_assembly.go:56-60: a cold false positive naming an unservable ledger becomes a "lookup incomplete" error rather than not-found — safe direction, confirm intended when Cut over to RPC v2: remove SQLite ingestion/query, serve from v2 stores #772 wires it.
  • Stale line ref at concurrent_bitmaps.go:127 (query.go:248 → 265/288-293).

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
Done

Relationships

None yet

Development

No branches or pull requests

Issue actions