docs: add module-by-module design documentation and CLAUDE.md - #493
Conversation
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add docs/ covering the open-source modules: architecture overview, threading model, concurrency control, transaction execution, data model & catalog, distribution & clustering, durability & recovery, range & bucket management, store handler, and the open log service. Every doc is grounded in the current source with file references, and docs/README.md defines the maintenance rule: code changes that alter documented behavior must update the corresponding doc. Proprietary components (eloq_log_service/, tx_service/raft_host_manager/, eloqstore submodule internals) are referenced only at their interface boundaries and intentionally not documented. Also add CLAUDE.md with build/test commands, an architecture primer, and pointers into docs/ for AI-assisted development. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Remove the open-log-service schema_logs drop note (tracked as tx_service#494) and the catalog-factory array code-health note (tx_service#495) from the design docs. The docs should describe current design; defects are tracked as separate issues. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d3ecb97 to
820fc92
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/05-data-model-and-catalog.md`:
- Around line 119-121: The wording is misleading about schema-version scope;
change the sentence that claims “at most two schema versions per node” to
clarify it applies per table/catalog entry (i.e., per CatalogEntry in
LocalCcShards::table_catalogs_), not across the whole node; mention that each
CatalogEntry can have at most two versions (current and dirty) and that a node
may host many CatalogEntry instances each with its own pair of versions, and
keep references to CatalogEntry and catalog_key_record.h so reviewers can locate
the text to update.
In `@docs/06-distribution-and-clustering.md`:
- Around line 71-80: The paragraph currently contradicts itself about bootstrap
values; update it to explicitly distinguish term caches vs leader-id cache by
stating that all term-related caches (leader_term_cache_,
candidate_leader_term_cache_, ng_leader_term_cache_, standby_node_term_cache_,
candidate_standby_node_term_cache_, standby_becoming_leader_term_cache_) are
initialized to -1, while ng_leader_cache_ is initialized to the local node id
(ng_leader_cache_[nid] == nid) so node i is assumed leader of NG i until told
otherwise, and adjust the surrounding sentence and examples to reflect those two
distinct defaults.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c8a25203-bb1a-4544-90a9-867ee30dac7e
📒 Files selected for processing (12)
CLAUDE.mddocs/01-architecture-overview.mddocs/02-threading-model.mddocs/03-concurrency-control.mddocs/04-transaction-execution.mddocs/05-data-model-and-catalog.mddocs/06-distribution-and-clustering.mddocs/07-durability-and-recovery.mddocs/08-range-and-bucket-management.mddocs/09-store-handler.mddocs/10-log-service.mddocs/README.md
| - Every node group has a `CatalogCcMap : TemplateCcMap<CatalogKey, CatalogRecord, true, false>` (`cc/catalog_cc_map.h`) — the `__catalog` cc map. `CatalogKey` (`catalog_key_record.h`) wraps a `TableName` (view for lookups, owning when resident in the map). `CatalogRecord` carries `schema_` / `dirty_schema_` pointers, `schema_image_` / `dirty_schema_image_` blobs and `schema_ts_`; it serves (1) schema lookup by txs, (2) bootstrap from the data store image, and (3) installing a dirty schema during DDL (header comment, `catalog_key_record.h:348`). | ||
| - The actual schema objects live once per node in `LocalCcShards::table_catalogs_` as `CatalogEntry` (`catalog_key_record.h:153`): `schema_`/`dirty_schema_` shared_ptrs + versions, guarded by a `std::shared_mutex`, with monotonic `InitSchema/SetDirtySchema/CommitDirtySchema/RejectDirtySchema`. Shards may transiently disagree on current vs dirty, but there are at most two schema versions per node. DDL is a two-phase schema flow (`AcquireAllCc` on the catalog entry, prepare/commit log records carrying old/new catalog blobs — `tx_operation.cpp:2350`); see [04-transaction-execution.md](04-transaction-execution.md). | ||
| - **Schema version checks**: data-path CC requests carry the `schema_version_` the tx resolved; `TemplateCcMap::Execute` rejects mismatches with `CcErrorCode::REQUESTED_TABLE_SCHEMA_MISMATCH` (`cc/template_cc_map.h:214`, `:1482`, `:3477`). `0` means "skip the check". |
There was a problem hiding this comment.
Clarify the schema-version scope.
Line 120 says there are “at most two schema versions per node,” but a node can host many CatalogEntrys. That limit only makes sense per table/entry, so the current wording is misleading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/05-data-model-and-catalog.md` around lines 119 - 121, The wording is
misleading about schema-version scope; change the sentence that claims “at most
two schema versions per node” to clarify it applies per table/catalog entry
(i.e., per CatalogEntry in LocalCcShards::table_catalogs_), not across the whole
node; mention that each CatalogEntry can have at most two versions (current and
dirty) and that a node may host many CatalogEntry instances each with its own
pair of versions, and keep references to CatalogEntry and catalog_key_record.h
so reviewers can locate the text to update.
Summary
Adds
docs/— module-by-module design documentation for the open-source components, plus aCLAUDE.mdfor AI-assisted development. The goal is that an engineer (or an AI coding assistant) can understand a module's design, flows, and invariants before reading the headers.docs/README.md— index, reading order, scope/exclusions, and the maintenance rule: code changes that alter documented behavior must update the corresponding doc in the same PR.01architecture overview &core/init ·02threading model (TxProcessor/brpc module, shard latch, bthread-mutex deadlock class) ·03concurrency control (CcShard/CcMap/CcEntry, NonBlockingLock, CC request catalog) ·04transaction execution (txm state machine, commit pipeline) ·05data model & catalog (TxKey type erasure, object/command model, CatalogFactory) ·06distribution & clustering (Sharder, terms, remote CC, cluster scale) ·07durability & recovery (WAL, ckpt-ts derivation, data sync pipeline, replay/orphan-lock recovery) ·08range & bucket management (slices, 14-stage range split, bucket migration) ·09store handler (DataStoreHandler contract, EloqDSS) ·10open log service.CLAUDE.md— build/test commands, architecture primer, pointers intodocs/, and the doc-maintenance rule.All content was derived from reading the current source (file/line citations throughout); no external knowledge bases were used. Proprietary components (
eloq_log_service/,tx_service/raft_host_manager/, eloqstore submodule internals) are referenced only at their RPC/interface boundaries and intentionally not documented.Notes
main; docs-only (docs/+CLAUDE.md), no code changes, CI-neutral.fix-cc-request-bthread-mutex-deadlock, thebthread_usleep-pollingWaitableCc/CkptTsCc). Until fix: avoid bthread mutex deadlock between tx processors and bthread waiters #491 merges those are slightly ahead ofmain's code.Verification
🤖 Generated with Claude Code
Summary by CodeRabbit