cockroachdb / cockroachdb/cockroach

revlog: review prototype implementation

Open
#169,432 1 comment 0 reactions 0 assignees View on GitHub
A-disaster-recovery C-investigation O-agent P-3 T-disaster-recovery
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

The existing code in `pkg/revlog` and `pkg/revlog/revlogjob` (and the
surrounding sibling-job plumbing in `pkg/backup`) was developed to prove
out the revision-log concept and merged as-is so it could be consulted,
iterated on, and reasoned about at a later date. It has **not** been
reviewed with the care and attention to detail we apply to
production-ready feature implementations — invariants are unverified,
error paths are spotty, naming and structure are first-draft, and a
number of known issues have been logged informally in package-local
TODOs and review threads rather than tracked.

If and when we decide to turn this prototype into a real feature, we
will need to revisit the implementation and review it carefully — both
the code that lives in `pkg/revlog*` today and any subsequent
additions. This issue exists as a placeholder for that review.

Below is a checklist of every PR (and its constituent commits) that
has touched the revlog code path. We update this on each merge so
that — if/when we do undertake the review — we have an accurate
burndown TODO list of everything that needs attention. Check a box
when the corresponding code has been reviewed in earnest.

## Merged PRs

- [ ] #168772 — revlog: add continuous backup design doc + format spec
- [ ] fb8ea39f055 revlog: add continuous backup design doc + format spec
- [ ] #168781 — revlog: add reader and writer for the revision log format
- [ ] 93e5dcb57e4 revlog: add reader and writer for the revision log format
- [ ] #168774 — backup: stub out backup revlog entry point
- [ ] b7a3649cf1a backup: stub out backup revlog entry point
- [ ] #168932 — revlog: add restore design in continuous backup RFC
- [ ] 875de105935 revlog: add restore design in continuous backup RFC
- [ ] fa123bdb4ac fixup! revlog: add restore design in continuous backup RFC
- [ ] #169056 — backup: add revlog job implementation
- [ ] 5923c6c2cfc revlogjob: add Producer, TickManager and Driver
- [ ] d599e68cf2f revlogjob: add Run() orchestrator with PTS and progress
- [ ] 4c78a7bf072 backup: invoke revlog writer from sibling job
- [ ] e25c00d6b0f revlogjob: add inspection builtins for the revision log
- [ ] e94754b965a revlogjob: distribute writer work via DistSQL
- [ ] f9a21b0269c revlogjob: persist and resume from checkpoint state
- [ ] 92f69a7ffe0 revlog: add HLC name format and coverage manifest
- [ ] f01ad384828 revlog: add schema-descriptor delta capture
- [ ] b127a223318 revlog,backup: identity-based scope and descriptor rangefeed
- [ ] 557612001ec docs: update continuous-backup RFC for descfeed, coverage, schema
- [ ] 0b9b0df16ba revlogjob: expose DataFrontier and AddSpansAt on TickManager
- [ ] 59b2caaba29 revlogjob: open producer rangefeed with WithInitialScan
- [ ] 63f2cf1ec57 revlogjob: drive descfeed processing off checkpoints
- [ ] d4a8553a3fc revlogjob: test descfeed widening triggers a single replan
- [ ] #169371 — backup: allowlist revlog sibling's BackupDetails fields
- [ ] a0ae6b8b4c8 backup: allowlist revlog sibling's BackupDetails fields
- [ ] #169383 — revlogjob: checkpoint frontier directly instead of via stringified copy
- [ ] 141da96d906 revlogjob: checkpoint frontier directly instead of via stringified copy
- [ ] #169366 — backup: claim revlog destination via timestamped marker race
- [ ] 130e89f6ee4 backup: claim revlog destination via timestamped marker race
- [ ] #169070 — restore: implement restore from continuous backup
- [ ] 03426229ce4 restore: stub out restore from revision log
- [ ] 3147adcb829 restore: resolve revlog descriptors and validate coverage during planning
- [ ] ccadc769e17 restore: distribute revision log ticks via DistSQL
- [ ] d2a75f8a1fa restore: implement local merge algorithm for revision log restore
- [ ] dacd839ccf8 restore: complete revlog merge pipeline with SST metadata, split/scatter, and KV ingest
- [ ] #169303 — revlogjob: expand test coverage from 35% to 63%
- [ ] 900c0f8c934 revlogjob: extract descRangefeedSource for descfeed test seam
- [ ] 520aa0e98c1 revlogjob: test the descfeed event loop end-to-end via a fake source
- [ ] 834ac873030 revlogjob: test frontier construction and partition-resume derivation
- [ ] 677dff4f990 revlogjob: test multi-span tick-close gating via Driver
- [ ] 873efe2212c revlogjob: test descriptor-frontier tick-close gating
- [ ] a8b8b5ae973 revlogjob: test the production jobPersister against a real registry
- [ ] fee939821ee revlogjob: test the metaSink ↔ DecodeFlush bridge
- [ ] c9758bfd819 revlogjob: test the inspection builtins against a real revlog
- [ ] 9010558e9b5 revlogjob: cover three small uncovered code paths
- [ ] df83444e270 revlogjob: surface producer storage errors
- [ ] 260fcf432b5 revlogjob: test the PTS manager against a real provider
- [ ] 8828abbc3c5 revlogjob: pin out-of-scope tombstone policy in descfeed

## In-flight PRs

(Will move into the "Merged PRs" section as they land.)

- #169367 — revlogjob: coalesce small per-tick flushes through the coordinator

Jira issue: CRDB-63445

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.