diff --git a/Cargo.lock b/Cargo.lock index 43c3b2e..0acb0b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -940,15 +940,6 @@ version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" -[[package]] -name = "colored" -version = "3.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "faf9468729b8cbcea668e36183cb69d317348c2e08e994829fb56ebfdfbaac34" -dependencies = [ - "windows-sys 0.48.0", -] - [[package]] name = "comfy-table" version = "7.2.2" @@ -1394,15 +1385,6 @@ dependencies = [ "miniz_oxide", ] -[[package]] -name = "float-cmp" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b09cf3155332e944990140d967ff5eceb70df778b34f77d8075db46e4704e6d8" -dependencies = [ - "num-traits", -] - [[package]] name = "fnv" version = "1.0.7" @@ -1632,16 +1614,6 @@ dependencies = [ "zerocopy", ] -[[package]] -name = "halfbrown" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c7ed2f2edad8a14c8186b847909a41fbb9c3eafa44f88bd891114ed5019da09" -dependencies = [ - "hashbrown 0.16.1", - "serde", -] - [[package]] name = "hashbrown" version = "0.12.3" @@ -1662,17 +1634,6 @@ dependencies = [ "foldhash 0.1.5", ] -[[package]] -name = "hashbrown" -version = "0.16.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "841d1cc9bed7f9236f321df977030373f4a4163ae1a7dbfe1a51a2c1a51d9100" -dependencies = [ - "allocator-api2", - "equivalent", - "foldhash 0.2.0", -] - [[package]] name = "hashbrown" version = "0.17.1" @@ -1841,9 +1802,6 @@ dependencies = [ "chdb-rust", "chrono", "clap", - "colored", - "comfy-table", - "crc32fast", "criterion", "figment", "flate2", @@ -1872,12 +1830,10 @@ dependencies = [ "serde_urlencoded", "serial_test", "sha2", - "simd-json", "tempfile", "thiserror 2.0.19", "tikv-jemallocator", "tokio", - "tower", "tower-http", "tracing", "tracing-opentelemetry", @@ -1889,7 +1845,6 @@ dependencies = [ name = "hyperbytedb-cli" version = "0.9.4-beta" dependencies = [ - "anyhow", "axum", "base64 0.22.1", "bytes", @@ -1916,7 +1871,6 @@ dependencies = [ "thiserror 2.0.19", "tokio", "toml", - "url", ] [[package]] @@ -1935,9 +1889,7 @@ dependencies = [ "reqwest 0.12.28", "serde", "serde_json", - "thiserror 2.0.19", "tokio", - "tower", "tower-http", "tracing", "tracing-subscriber", @@ -3819,20 +3771,6 @@ version = "0.3.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a219298ac11a56ea9a6d2120044824d6f01aeb034955e7af7bc16858527deea" -[[package]] -name = "simd-json" -version = "0.17.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e32d7ab2678282d21e53374fbead7119b7eacbede73685dcaac472870a29a11c" -dependencies = [ - "halfbrown", - "ref-cast", - "serde", - "serde_json", - "simdutf8", - "value-trait", -] - [[package]] name = "simdutf8" version = "0.1.5" @@ -4526,18 +4464,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba73ea9cf16a25df0c8caa16c51acb937d5712a8429db78a3ee29d5dcacd3a65" -[[package]] -name = "value-trait" -version = "0.12.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f3f4b4a98dfe54bc9ed3641af7ffcb837240269627dbd5cb047d13daa39736cc" -dependencies = [ - "float-cmp", - "halfbrown", - "itoa", - "ryu", -] - [[package]] name = "vcpkg" version = "0.2.15" diff --git a/docs/deep-dive/deep-dive-clustering.md b/docs/deep-dive/deep-dive-clustering.md index 1b68f57..d2c485b 100644 --- a/docs/deep-dive/deep-dive-clustering.md +++ b/docs/deep-dive/deep-dive-clustering.md @@ -122,7 +122,7 @@ pub struct NodeInfo { ## 3. Cluster Bootstrap -**File:** `src/application/cluster/bootstrap.rs` +**File:** `src/composition/bootstrap.rs` ### `ClusterBootstrap::init(config)` @@ -236,10 +236,10 @@ pub type HyperbytedbRaft = openraft::Raft; ### Cluster request types ```rust -// src/adapters/cluster/raft/types.rs +// src/domain/cluster/raft_request.rs (re-exported by src/adapters/cluster/raft/types.rs) pub enum ClusterRequest { SetNodeState { node_id: u64, state: NodeState }, - SchemaMutation(MutationRequest), + SchemaMutation(Box), ShardMapMutation(Box), // when [sharding] enabled } @@ -703,39 +703,23 @@ The scheduler holds a **per-region operator lock** while split/merge/rebalance/f - **Drain** performs transfer, `TransferPrimary`, then `MovePeer` (re-reads epoch after primary transfer). - **Sync manifests** include per-region WAL watermarks; failover prefers the most caught-up Active peer (leader eligible). -### Split-transfer reconciliation - -A split commits via Raft before its historical-row re-push completes. If the -destination's Raft apply lags the leader's, re-home/push RPCs answer `404` -("region range not found") or `409` (stale epoch). These failures are **not -lost**: - -- Apply-time normalization records durable intent on the child region: - `transfer_verified=false` (+ `transfer_first_seen`) whenever a split child's - primary differs from its parent's. The flag rides the Raft-committed map, so - leadership change or process restart rebuilds the work queue from a map scan. -- The leader keeps an in-memory reconciliation queue (ordering detail only) and - drains it each tick. Movement retries use **authoritative sources only** — - the old primary recorded at split time, then live peers with a non-zero - region WAL watermark (a data-less peer would verify vacuously). -- Satisfaction requires **positive proof**: every exported point confirmed - applied at the destination. Sentinel outcomes (`transfer_id == 0`, - i.e. source==destination collision; empty exports) never clear debt. -- On verified movement the leader proposes the `ClearVerified` shard-map op. - Disable with `[sharding] transfer_clear_proposals_enabled = false` on - mixed-version clusters: nodes older than the op cannot decode it from the - Raft log. -- Splits and merges are refused while either region carries outstanding debt. - Stuck entries escalate once via `TransferPrimary` to another live peer - (stage-before-commit ordering prevents committing a data-less owner), then - park with `hyperbytedb_shard_transfer_parked_total` after ~10 stalled ticks - past a minimum age — operator action required. -- Materialized-view rollup destinations (`SummingMergeTree`) are excluded from - reconciliation entirely: re-delivery would permanently double additive - aggregates. Their provisioning flows through the MV backfill path. -- Raw measurement tables are `ReplacingMergeTree(ingest_seq)` keyed by - `(series_id, time)`: retried pushes dedupe eventually, but a transient - duplicate window exists until background merges collapse rows. +### Split-transfer debt + +Splits are metadata-only: a split child keeps its parent's primary and peers, +so no rows move and no transfer is owed. The `transfer_verified` and +`transfer_first_seen` fields on a region, and the `ClearVerified` shard-map op, +remain because: + +- placement skips a flagged region, splits and merges refuse one, and + applying `AddPeer` checks the flag; +- nodes must keep decoding older Raft logs that contain `ClearVerified`. + +Nothing sets the flag any more, and nothing clears it: the post-commit +reconciliation queue that retried flagged transfers and proposed +`ClearVerified` has been removed. A region flagged by an older version stays +flagged, so placement, splits and merges keep skipping it. +`[sharding] transfer_clear_proposals_enabled` is still accepted in config +files and has no effect. ### Membership self-truth (`/health` body) diff --git a/docs/developer-guide/internals/core-modules.md b/docs/developer-guide/internals/core-modules.md index fc5aeac..0b5c4a5 100644 --- a/docs/developer-guide/internals/core-modules.md +++ b/docs/developer-guide/internals/core-modules.md @@ -61,7 +61,7 @@ Hexagonal architecture interfaces. Each trait is `Send + Sync` with `#[async_tra | `peer_ingestion_service.rs` | `PeerIngestionService` | Cluster ingest: local WAL + `ReplicationPort` fan-out. | | `line_protocol.rs` | parse/encode helpers | Line protocol parsing and encoding. | | `msgpack_ingest.rs` | `parse_msgpack_body_to_points` | MessagePack array-of-points ingest. | -| `columnar_msgpack.rs` | `ColumnarMsgpackBatch` | Feature-gated columnar MessagePack ingest. | +| `columnar_msgpack.rs` | decode helpers | Feature-gated columnar MessagePack ingest; the batch type `ColumnarMsgpackBatch` lives in `domain/columnar_batch.rs`. | | `ingest_metadata.rs` | `IngestCardinalityLimits`, `IngestSchemaCache` | Hot-path schema/tag caching and cardinality enforcement. | | `query_service.rs` | `QueryServiceImpl` | Full TimeseriesQL execution: parse → dispatch → translate → chDB → format. | | `peer_query_service.rs` | `PeerQueryService` | Cluster query: schema mutations via Raft or `ReplicationPort`. | diff --git a/docs/superpowers/plans/2026-11-03-adapter-application-inversions.md b/docs/superpowers/plans/2026-11-03-adapter-application-inversions.md new file mode 100644 index 0000000..77f219c --- /dev/null +++ b/docs/superpowers/plans/2026-11-03-adapter-application-inversions.md @@ -0,0 +1,121 @@ +# Adapter→Application Inversions and Layering Guard Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Driven adapters (storage, WAL, the sync client) stop calling into `application`. A test then enforces the layering rule, so new leaks fail CI. + +**Architecture:** Each leak is fixed by moving the code it needs to where it belongs: +- `StageTimer` is instrumentation, so it moves to the crate-level `telemetry` module, which sits outside the layers. +- Tag backfill only uses `MetadataPort`, so it becomes a provided method on that port. +- The columnar timestamp helper is a wire-format rule, so it moves to the domain. +- `SyncClient` splits into an adapter part that fetches over HTTP (through `PeerRpc`) and an application `CatchUpService` that applies what was fetched. + +Finally, `tests/architecture.rs` scans `src/` and fails on any forbidden import that is not on an explicit allowlist. + +**Tech Stack:** Rust 2024, async-trait. + +**Spec:** the deferred item "driven adapters calling application code" in `docs/superpowers/plans/2026-11-03-code-health-fixes.md`, validated at `7ea9643`. + +**Depends on:** +- code-health Task 14, which moves `ColumnarMsgpackBatch` to `domain/columnar_batch.rs` and the point codec to `domain/point_codec.rs`; +- `2026-11-03-peer-rpc-port.md` Task 1 (`PeerRpc`), for Task 3 only. + +## Census (validated) + +The adapters' production imports of `crate::application`: +- `adapters/chdb/native_adapter.rs`: + - `ingest_metadata::backfill_tag_metadata`, at `:56` and its call at `:552`, in `persist_new_series_metadata` (`:533`); + - `columnar_msgpack::columnar_timestamps_ns` (`:1299`, `:2354`; defined at `application/columnar_msgpack.rs:60`); + - `stage_timer::StageTimer` (`:1388`; defined at `application/stage_timer.rs:8`); + - `ColumnarMsgpackBatch`, which code-health Task 14 handles. +- `adapters/cluster/sync_client.rs`: + - `ingest_metadata::{IngestCardinalityLimits, prepare_batch_metadata}` (`:6`); + - `wal_append::append_points_with_prepared` (`:7`, used at `:557`); + - `runtime_settings::{RuntimeSettings, apply_or_store}` (`:32`, `:94`, `:467`). +- `adapters/cluster/raft/log_store.rs:21-23` (`MaterializedViewService`, `schema_mutation_apply`, `ShardRoutingContext`) and `raft/state_machine.rs` (`apply_schema_mutation`). **Allowed:** the Raft state machine drives the application, as HTTP handlers do. +- `adapters/http/*`: **allowed** (driving adapters). +- `adapters/wal/batching_wal.rs` and `adapters/cluster/peer_client.rs` use the point codec, which code-health Task 14 handles. + +`SyncClient` (`adapters/cluster/sync_client.rs`, 792 lines) has: +- HTTP fetches: + - `send_join_request` (`:268`, POST `/internal/membership/join`, `JoinRequest`); + - `get_manifest` (`:308`, GET `/internal/sync/manifest`, `SyncManifest`); + - the fetch half of `sync_metadata` (`:326`, GET `/internal/sync/metadata`, `MetadataSnapshot`); + - the fetch half of `wal_catchup` (`:490`, GET `/internal/sync/wal?from_seq=&max_entries=5000`, `WalSyncResponse`); + - `fetch_shard_map` (`:582`, GET `/internal/shard/map`, `ShardMap`). +- Application work: `reconnect_sync` (`:114`), `join_and_sync` (`:187`), `pick_sync_peer` (`:246`), `sync_shard_map` (`:369`), `import_metadata_entry` (`:403`), `append_catchup_entry` (`:539`) and `verify_catchup_progress` (`:606`). + +All the wire types are in `domain/cluster/sync.rs`. `SyncClient` is constructed in `application/cluster/bootstrap.rs:122` (composition after code-health Task 14) and in `adapters/http/peer_handlers.rs:913`. + +## Global Constraints + +- No behaviour change, and endpoints, payloads and error variants are unchanged. `PeerUnreachable` vs `SyncFailed` stays mapped per call as today. +- `Cargo.lock` keeps chdb-rust 2.0.0; commits carry no Claude trailers. +- The gate: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test --lib`, `cargo test --no-fail-fast --test '*'`. + +## Review Focus + +1. **A node joining with a sharded map.** It must skip WAL pull as today. The existing `a_sharded_node_syncs_without_pulling_a_peer_wal` (`sync_client.rs:725`) must still pass after the split; it moves with `CatchUpService`. +2. **Synced users keep their grants.** The existing `a_synced_user_keeps_its_grants` (`:680`) moves too and must pass. +3. **Tag backfill for a measurement that does not exist yet.** It must create it with the tag keys, as `backfill_tag_metadata`'s `else` branch does. Task 1 tests the port method on both branches. +4. **The architecture test on a new leak.** Adding `use crate::adapters::...` in `domain/` must fail the test with the file and line. Task 4 includes a self-test of the scanner. + +--- + +### Task 1: Tag backfill becomes a `MetadataPort` method; `StageTimer` moves to telemetry + +**Files:** `ports/metadata.rs`, `application/ingest_metadata.rs` (`:344` and its caller at `:454`), `adapters/chdb/native_adapter.rs`, `application/stage_timer.rs`, `telemetry.rs`, and every `StageTimer` user: `native_adapter.rs`, `adapters/http/shard_handlers.rs`, `application/peer_ingestion_service.rs`, `application/query_service.rs`. + +**Interfaces:** +- Produces: a provided trait method on `MetadataPort`: `async fn backfill_tag_keys(&self, db: &str, rp: &str, measurement: &str, tags: Vec<(String, String)>) -> Result<(), HyperbytedbError>`. Its body is today's `backfill_tag_metadata` body, using `self`. +- Produces: `crate::telemetry::StageTimer`, the same type with the same API. `application/stage_timer.rs` is deleted, not re-exported. + +- [ ] **Step 1: Write the failing tests** `backfill_adds_missing_tag_keys` and `backfill_creates_unknown_measurement` in `adapters/metadata/rocksdb_meta.rs` tests: + - existing measurement `cpu` with `tag_keys = ["host"]`, then backfilling `[("dc","x")]`, gives tag keys `["dc","host"]`; + - unknown measurement `mem`, then backfilling `[("host","a")]`, gives `tag_keys ["host"]`, `field_types` empty. +- [ ] **Step 2: Implement.** + - `backfill_tag_metadata` is deleted; its two callers call `metadata.backfill_tag_keys(..)`. + - Move `StageTimer`. + - Then run `grep -rn "crate::application" src/adapters/chdb`. Expected: only the `columnar_timestamps_ns` lines, which Task 2 handles. +- [ ] **Step 3: Run the gate, then commit** with the message "Tag backfill on the metadata port; StageTimer in telemetry". + +### Task 2: Columnar timestamps in the domain + +- [ ] **Step 1: Move the helper.** Move `columnar_timestamps_ns` (`application/columnar_msgpack.rs:60`) and any tests of it into `domain/columnar_batch.rs` (created by code-health Task 14; `Precision` is already domain, at `domain/database.rs:175`). Keep a `pub use` in `columnar_msgpack.rs`, and point `native_adapter.rs:1299`, `:2354` at the domain path. Then run `grep -rn "crate::application" src/adapters/chdb`. Expected: no output. +- [ ] **Step 2: Run the gate, then commit** with the message "Columnar timestamp rule lives with the columnar batch". + +### Task 3: Split `SyncClient` into fetch (adapter) and catch-up (application) + +**Files:** +- Create: `hyperbytedb/src/application/cluster/catch_up.rs` +- Modify: `adapters/cluster/sync_client.rs`, `ports/peer_rpc.rs`, `adapters/cluster/http_peer_rpc.rs`, and the two construction sites + +**Interfaces:** +- Add to `PeerRpc`: + - `async fn join(&self, addr: &str, req: &JoinRequest, timeout: Duration) -> Result<(), PeerRpcError>` + - `async fn sync_metadata_snapshot(&self, addr: &str, timeout: Duration) -> Result` + - `async fn sync_wal(&self, addr: &str, from_seq: u64, max_entries: u32, timeout: Duration) -> Result` + - `async fn shard_map(&self, addr: &str, timeout: Duration) -> Result` + + (`sync_manifest` exists from peer-rpc Task 5.) +- Produces: `pub struct CatchUpService` in application, holding the fields `SyncClient` has today (`sync_client.rs:19`) with `client` replaced by `rpc: SharedPeerRpc`, and the same `pub` methods: `reconnect_sync`, `join_and_sync`, `sync_shard_map`. `SyncClient` is deleted once its last caller moves. `remote_map_is_newer` (`:578`) moves with it. + +- [ ] **Step 1: Move the two existing tests** (`:680`, `:725`) and their `spawn_sync_peer` helper (`:630`) to `catch_up.rs`. They fail to compile until Step 2. +- [ ] **Step 2: Implement the `PeerRpc` methods and `CatchUpService`,** and migrate `application/cluster/bootstrap.rs:122` and `adapters/http/peer_handlers.rs:913`. Then run `grep -rn "crate::application" src/adapters/cluster/sync_client.rs src/adapters/cluster/http_peer_rpc.rs`. Expected: no output (the file is gone or HTTP-only). +- [ ] **Step 3: Run the gate,** especially `--test raft_integration --test integration`, **then commit** with the message "Catch-up sync is an application service over PeerRpc". + +### Task 4: Architecture test + +**Files:** create `hyperbytedb/tests/architecture.rs`. + +**Interfaces:** +- `fn forbidden_imports(src_root: &Path) -> Vec` returns `":: imports "` for every production (non-`#[cfg(test)] mod`) line matching `crate::(adapters|application|ports)` that breaks these rules: + - `domain/` may import none of `adapters`, `application` or `ports`; + - `ports/` may not import `adapters` or `application`; + - `application/` may not import `adapters`; + - `adapters/` may import `application` only from `adapters/http/` and `adapters/cluster/raft/{log_store,state_machine}.rs`. +- `const ALLOWLIST: &[&str]` holds `"file::target-prefix"` entries for the items `2026-11-03-peer-rpc-port.md` has not removed yet. Fill it from the test's first run, and leave a comment naming that plan beside each entry. + +- [ ] **Step 1: Write** `scanner_flags_a_domain_adapter_import` (Review Focus 4). Run `forbidden_imports` on a temp dir holding `domain/x.rs` with `use crate::adapters::wal;`, and assert one finding naming `domain/x.rs:1`. +- [ ] **Step 2: Write** `layering_rules_hold`, which asserts `forbidden_imports("src")`, minus `ALLOWLIST`, is empty. Run it, and paste the remaining findings into `ALLOWLIST`. Each one must be one of the peer-rpc plan's items; anything else is a leak to fix now. +- [ ] **Step 3: Run the gate, then commit** with the message "Architecture test enforces the layering rules". diff --git a/docs/superpowers/plans/2026-11-03-code-health-fixes.md b/docs/superpowers/plans/2026-11-03-code-health-fixes.md new file mode 100644 index 0000000..5606bb2 --- /dev/null +++ b/docs/superpowers/plans/2026-11-03-code-health-fixes.md @@ -0,0 +1,372 @@ +# Code-Health Fixes Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix every validated finding of the 2026-11-03 code-health scan: +- one bug; +- dead code and unused crates; +- duplicated helpers; +- hexagonal layering leaks; +- two avoidable costs on the write path. + +None of these changes behaviour, apart from the bug fix. + +**Architecture:** The work comes in five independent phases (A–E). Each task leaves the tree green and is its own commit. Layering fixes move code toward the rule *domain ← ports ← application ← adapters/composition*: +- pure logic moves into `domain`; +- capabilities the application needs become `ports` traits; +- wiring moves out of `application`. + +**Tech Stack:** Rust 2024 (toolchain 1.94), tokio, openraft 0.9, RocksDB, chDB (chdb-rust 2.0.0), criterion. + +**Spec:** the 2026-11-03 scan report in this session, validated against branch `fix/review-2026-11-bugs` at `7ea9643`. Every finding below was re-checked against the code, and line numbers refer to that commit. The validation evidence is listed with each task. + +**Base:** branch off `fix/review-2026-11-bugs` (PR #132), or off `main` once #132 has merged. Tasks 15 and 16 change code that #132 adds. + +## Global Constraints + +- `Cargo.lock` must keep chdb-rust **2.0.0**. `cargo clippy` may rewrite the lock; run `git checkout -- Cargo.lock` before every commit. +- Commits carry no Claude / Co-Authored-By trailers. +- The gate for every task, in CI order: `cargo fmt --check`, then `cargo clippy --all-targets -- -D warnings`, then `cargo test --lib`, then `cargo test --no-fail-fast --test '*'`, then `cargo test -p hyperbytedb-cli`. +- Raft log and HTTP wire formats must not change: + - moving a type must keep its serde representation; + - `ShardMapOp::ClearVerified`, the `transfer_verified` fields and the `[sharding] transfer_clear_proposals_enabled` config key (`config.rs:108`) **stay**. Placement, split/merge refusal and `AddPeer` apply still read them. +- Do not touch the three bugs still open from the 2026-11-02 review: `delete_series_matching` ignoring its predicate, `DROP SERIES` without `FROM`, and `ingest_seq` ordering. They have their own work. + +## Review Focus + +1. **Mixed-version Raft log.** An entry written by a node on the old version must still decode after `ClusterRequest` moves module. Task 11 pins its JSON encoding with a fixture. +2. **`DELETE` with `?rp=` in Raft mode.** It must hit the named retention policy, not the default one. Task 1 tests both the Raft and the non-Raft builders. +3. **Strings with control characters** (tab, newline) in tag values, in `WHERE` clauses, bind parameters and line-protocol writes. They must round-trip after the escapers merge. Task 6 adds a query test with a tab in a tag value. +4. **Old config files.** A `config.toml` that still sets `transfer_clear_proposals_enabled` must still load after the reconciliation code is deleted. Task 2 adds a config-load test. +5. **A field widening, then conflicting, within one batch** (int, then float, then string). The middle point must widen and the last must be dropped after the in-place change. Task 15 adds this test. + +--- + +## Phase A — Bug + +### Task 1: Raft-mode DELETE / DROP SERIES honour `?rp=`; one mutation builder + +**Validated:** +- `peer_query_service.rs:402` `mutation_request_from_statement(stmt, db, metadata, shard_routing)` has no retention-policy input. Its `Delete` arm (~416) and `DropSeries` arm (~550) call `metadata.get_default_rp`. +- `execute_raft_mutation` (`:186`) never receives `retention_policy`. +- The non-Raft inline arms of `execute_query` build the same mutations a second time with the request's policy: `Delete` at ~700–735, `DropSeries` at ~1035–1080. + +**Files:** +- Modify: `hyperbytedb/src/application/peer_query_service.rs` +- Test: the same file's `#[cfg(test)] mod tests` + +**Interfaces:** +- Produces: `async fn mutation_request_from_statement(stmt: &Statement, db: &str, retention_policy: Option<&str>, metadata: &Arc, shard_routing: Option<&Arc>) -> Result` + +- [ ] **Step 1: Write the failing test** `raft_delete_uses_the_requested_retention_policy`: + - Set up `RocksDbMetadata` with database `db`, plus a non-default policy `week` created with `create_retention_policy`, and measurement `cpu` registered in `week` with tag key `host`. + - Call `mutation_request_from_statement(&parse("DELETE FROM cpu WHERE host='a'")[0], "db", Some("week"), &meta, None)`. + - Assert it is `MutationRequest::Delete { rp, .. }` with `rp == "week"`. + - Repeat with `DROP SERIES FROM cpu WHERE host='a'` and assert `MutationRequest::DropSeries { rp, .. }` with `rp == "week"`. + - With `None` instead of `Some("week")`, assert `rp == "autogen"`. +- [ ] **Step 2: Run it and check that it fails.** Run `cargo test --lib peer_query_service::tests::raft_delete_uses_the_requested_retention_policy`. Expected: a compile error (wrong number of arguments). +- [ ] **Step 3: Add the parameter.** `Delete` and `DropSeries` use `retention_policy` when it is set, else `get_default_rp`. Thread it through: + - give `execute_raft_mutation` a `retention_policy: Option<&str>` parameter; + - pass it from `execute_query` (call at ~:662); + - replace the inline `Delete` and `DropSeries` request-building blocks with `mutation_request_from_statement(&stmt, db, retention_policy, &self.metadata, self.shard_routing.get())`. +- [ ] **Step 4: Run the test (expect PASS), then the full gate.** +- [ ] **Step 5: Commit** with the message "Raft-mode DELETE / DROP SERIES honour ?rp=; build cluster mutations in one place". + +--- + +## Phase B — Dead code + +### Task 2: Delete the unused split-transfer reconciliation in the shard scheduler + +**Validated:** +- The callers were removed in 0.9.1 (#116), when splits became metadata-only. The code's own note (`shard_scheduler.rs:261-264`) says the deletion deserves its own review. +- With the 17 `#[allow(dead_code)]` attributes removed, rustc reports these as unused (539 lines in all): + - `PendingTransfer` (:266); + - `RECONCILE_PARK_AFTER_TICKS` (:285) and `RECONCILE_MIN_AGE_SECS` (:288); + - the fields `points_sink` (:300) and `reconciliation_queue` (:325); + - `try_activate_staged_regions` (:740), `enqueue_reconciliation` (:1866) and `propose_clear_verified` (:1910); + - `push_and_drop_range` (:2877) and `request_region_rehome` (:2927); + - `TransferDisposition` (:3017) and `classify_transfer_error` (:3028); + - `now_unix` (:3170); + - `rebuild_queue_from_map` (:3179), `drain_reconciliation` (:3225), `attempt_reconciliation` (:3349), `escalate_stuck_transfer` (:3468), `remove_reconciliation` (:3497) and `pending_transfers` (:3506); + - `ReconcileOutcome` (:3515). +- `ShardScheduler::new` (:389) takes `points_sink` as a parameter; there are 8 call sites in `src/` and `tests/`. + +**Files:** +- Modify: `hyperbytedb/src/application/shard_scheduler.rs` +- Modify: every `ShardScheduler::new(` call site (find them with `grep -rn "ShardScheduler::new(" src tests`) +- Modify: `docs/deep-dive/deep-dive-clustering.md:706-738` +- Test: `hyperbytedb/src/config.rs` tests + +- [ ] **Step 1: Write the config test** `old_config_with_transfer_clear_proposals_still_loads`. Parse a TOML string containing `[sharding]\ntransfer_clear_proposals_enabled = false` into `Config`, and assert it parses with that field `false`. It passes today and pins Review Focus 4. +- [ ] **Step 2: Remove the items listed above.** Also remove every `#[allow(dead_code)]` they carried, the `points_sink` parameter of `ShardScheduler::new` and its argument at each call site, and any import left unused. Then run `grep -n "allow(dead_code)" src/application/shard_scheduler.rs`. Expected: no output. If anything is left, it is still in use: keep it and drop only its allow. +- [ ] **Step 3: Fix the stale comment.** Change the comment near `:2241` (it says ``Re-read: `drain_reconciliation` runs before the tick loop``) so it no longer names a removed function. +- [ ] **Step 4: Rewrite the documentation section.** Replace the "Split-transfer reconciliation" section of `deep-dive-clustering.md` with a short paragraph. It should say that splits are metadata-only, so no transfer debt is created, and that `transfer_verified` / `ClearVerified` remain only for routing, placement, and decoding older Raft logs. +- [ ] **Step 5: Run the full gate.** Expect no new warnings and every test passing. +- [ ] **Step 6: Commit** with the message "Remove the unused split-transfer reconciliation from the shard scheduler". + +### Task 3: Delete the other unreferenced items + +**Validated:** each of these is referenced nowhere in `hyperbytedb/src`, `tests`, `benches`, `hyperbytedb-cli`, `hyperbytedb-proxy` or `docs`, apart from its own definition or re-export: +- `application/arrow_ingest/` (the whole module: `mod.rs`, `columnar.rs`, `line_protocol.rs`, `msgpack.rs`, `points.rs`); remove `pub mod arrow_ingest;` from `application/mod.rs:1` +- `MaterializedViewService::drop_ch_objects_for_def` (`materialized_view_service.rs:473`) +- `parse_line_body_to_points_with_capacity` (`line_protocol.rs:157`) +- `ReplicationApplyQueue::with_workers` (`replication_apply.rs:183`) +- `SharedShardMapPort` (`ports/sharding.rs:97`) +- `merge_field_disc` (`domain/field_type.rs:56`) +- `SanitizedIdent::sanitize` (`domain/chdb_naming.rs:28`) +- `FieldValue::type_name` (`domain/point.rs:14`), keeping `type_name_from_discriminant` +- `LocationCache::invalidate_all` (`domain/sharding/location_cache.rs:172`) +- `ChdbQueryAdapter::with_concurrency_limit` (`adapters/chdb/query_adapter.rs:41`) +- `ChdbNativeAdapter::with_arrow_inserts` (`adapters/chdb/native_adapter.rs:306`) +- `default_pool_size` (`adapters/chdb/session.rs:74`) +- `ArrowCache::insert_batch` (`adapters/wal/arrow_cache.rs:33`) +- `with_message` (`adapters/cluster/raft/types.rs:32`) +- `RaftAdaptor` (`adapters/cluster/raft/mod.rs:25`) +- `build_coalesced_fact_view_with_row_meta` (`timeseriesql/to_clickhouse/coalesce.rs:26`, and its re-export in `to_clickhouse/mod.rs:21`) +- the CLI's `first_error` (`hyperbytedb-cli/src/client/query.rs:246`) + +- [ ] **Step 1: Delete the items,** then re-run the check: `grep -rnw "" hyperbytedb* docs` for each name. Expected: no hits. +- [ ] **Step 2: Run the full gate.** Expected: PASS. A compile error means something uses an item after all: restore that item and note it in the commit message. +- [ ] **Step 3: Commit** with the message "Remove unreferenced code". + +### Task 4: Remove unused dependencies + +**Validated:** `cargo machete` and a grep agree on: +- `hyperbytedb/Cargo.toml`: `tower` (:11), `crc32fast` (:95), `comfy-table` (:106), `colored` (:107), `simd-json` (:108); +- `hyperbytedb-proxy/Cargo.toml`: `thiserror`, `tower`; +- `hyperbytedb-cli/Cargo.toml`: `anyhow`, `url`. + +- [ ] **Step 1: Remove the entries,** then run `cargo build --workspace --all-targets`. Expected: success. +- [ ] **Step 2: Check the lock diff.** Run `git diff Cargo.lock`: only removals are expected, and chdb-rust must still be `2.0.0`. Commit this lock change; it is the one exception to resetting the lock. +- [ ] **Step 3: Run the full gate, then commit** with the message "Drop unused dependencies". + +--- + +## Phase C — Duplicates + +### Task 5: One "raw write into an MV destination" rule + +**Validated:** the same condition (`materialized && materialized_rp.is_none_or(|mr| mr == rp)`) is written out in six places: +- `ingestion_service.rs:152` and `:251`; +- `peer_ingestion_service.rs:228`, `:896` and `:1046`; +- `native_adapter.rs:691`. + +**Files:** `hyperbytedb/src/domain/measurement.rs` (impl at :29), plus the six sites above. + +**Interfaces:** +- Produces: `impl MeasurementMeta { pub fn refuses_raw_writes(&self, rp: &str) -> bool }` + +- [ ] **Step 1: Write the failing test** `refuses_raw_writes_only_for_its_own_rp` in `domain/measurement.rs`, with three cases: + - `materialized: false` gives `false` for `"autogen"`; + - `materialized: true, materialized_rp: Some("rollup")` gives `true` for `"rollup"` and `false` for `"autogen"`; + - `materialized: true, materialized_rp: None` gives `true` for any policy. +- [ ] **Step 2: Implement it,** replace the six conditions with calls to it, and run the full gate. +- [ ] **Step 3: Commit** with the message "One rule for raw writes into materialized-view destinations". + +### Task 6: One ClickHouse string-literal escaper + +**Validated:** there are four escapers. +- These three escape only `\` and `'`: + - `query_service.rs:2005` `escape_clickhouse_string_literal`; + - `adapters/http/query.rs:75` `escape_bind_string`; + - `to_clickhouse/conditions.rs:403` `quote_string`. +- `native_adapter.rs:2649` `append_quoted_string` also escapes `\n`, `\r`, `\t` and other control characters as `\xNN`. + +**Files:** +- Modify: `hyperbytedb/src/domain/chdb_naming.rs` +- Modify: the four sites above + +**Interfaces:** +- Produces: `pub fn push_string_literal_body(out: &mut String, s: &str)` (escapes, no quotes) and `pub fn string_literal(s: &str) -> String` (`'…'` quoted), both in `domain/chdb_naming.rs`, with the native adapter's escaping. + +- [ ] **Step 1: Write the failing unit test** `string_literal_escapes_quotes_backslashes_and_controls`: + - `string_literal("a'b\\c")` gives `'a\'b\\c'`; + - `string_literal("x\ty\n")` gives `'x\ty\n'`, with the backslash sequences written literally; + - `string_literal("\u{1}")` gives `'\x01'`. +- [ ] **Step 2: Write the integration test** `tag_value_with_a_tab_round_trips` in `tests/compat/query_tests.rs`, marked `#[serial(chdb)]` (Review Focus 3). + - Write `cpu,host=a\tb v=1 1`; in the line-protocol body the tag value holds a literal tab. + - Then `SELECT v FROM cpu WHERE host = 'ab'` must return 1 row. +- [ ] **Step 3: Implement the two functions,** and make the four sites call them: + - `quote_string` and `escape_clickhouse_string_literal` become `string_literal` and the body-only form; + - `escape_bind_string` becomes the body-only form; + - `append_quoted_string` delegates. +- [ ] **Step 4: Run the full gate.** Exact-SQL test expectations may change only where an input string contains a control character. If any other expectation changes, stop: the escaping semantics differ. +- [ ] **Step 5: Commit** with the message "One ClickHouse string-literal escaper". + +### Task 7: One JSON POST helper in the Raft network + +**Validated:** `adapters/cluster/raft/network.rs:117` `post_json` and `:145` `post_json_snapshot` are line-for-line identical. They differ only in the error type parameter of `RPCError`. + +- [ ] **Step 1: Merge them** into one `async fn post_json(&self, path: &str, req: &Req, option: &RPCOption) -> Result>`, and update its callers. +- [ ] **Step 2: Run the gate.** The Raft tests cover this path: `cargo test --test raft_integration`, then the full gate. +- [ ] **Step 3: Commit** with the message "One JSON POST helper for Raft RPCs". + +### Task 8: Leadership port (removes the duplicated `is_raft_leader`) + +**Validated:** four places read `metrics().current_leader` directly: +- `retention_service.rs:48` and `continuous_query_service.rs:33`: identical `is_raft_leader`, over `Option`; `None` means leader; +- `runtime/shard_heartbeat.rs:99-103`: reads `current_leader`; +- `raft_leader_callbacks.rs:44`. + +The scheduler, `drain.rs` and `peer_query_service.rs` also *propose* through Raft (`client_write`), so they keep `HyperbytedbRaft`. That part is out of scope (see the end of the plan). + +**Files:** +- Create: `hyperbytedb/src/ports/leadership.rs`, registered in `ports/mod.rs` +- Modify: `adapters/cluster/raft/mod.rs` (the impl), `retention_service.rs`, `continuous_query_service.rs`, `runtime/shard_heartbeat.rs`, and their constructor call sites (5 in all; find them with `grep -rn "RetentionService::new\|ContinuousQueryService::new\|run_region_heartbeat_reporter(" src tests`) + +**Interfaces:** +- Produces: `pub trait LeadershipPort: Send + Sync { fn current_leader(&self) -> Option; fn is_leader(&self, node_id: u64) -> bool { self.current_leader() == Some(node_id) } }`, implemented in `adapters/cluster/raft/mod.rs` as `impl LeadershipPort for HyperbytedbRaft`. +- The retention and continuous-query services take `Option>`; `None` keeps meaning "single node: always leader". `run_region_heartbeat_reporter` takes `Arc`. + +- [ ] **Step 1: Write the failing test** `retention_and_cq_gate_on_leadership`. + - Each service replaces its private `is_raft_leader` with `fn may_run(&self) -> bool`, which `None` (single node) makes `true`. The retention service checks leadership inside `run` (`retention_service.rs:58`) and has no single-tick entry, so the test calls `may_run` directly. + - Use a stub `LeadershipPort` whose leader is node 2. Construct each service as node 1 with `Some(stub)` and assert `!may_run()`; as node 2, assert `may_run()`; with `None`, assert `may_run()`. +- [ ] **Step 2: Implement the port and swap the three files.** Delete both `is_raft_leader` copies in favour of `leadership.is_leader(node_id)`. Then run `grep -n "adapters::cluster::raft" src/application/retention_service.rs src/application/continuous_query_service.rs src/application/runtime/shard_heartbeat.rs`. Expected: no output. +- [ ] **Step 3: Run the full gate, then commit** with the message "Leadership port; retention, continuous queries and heartbeats stop holding the Raft handle". + +### Task 9: One write-body parser for both ingestion services + +**Validated:** `ingestion_service.rs:192-211` and `peer_ingestion_service.rs:968-987` have the same `match format { LineProtocol | Msgpack | ColumnarMsgpack => Err(..) }` parse block, followed by the same `validate_point_count` rule. Merging the two whole `ingest()` functions is not planned: they differ in replication and sharding. + +**Interfaces:** +- Produces: `pub fn parse_write_body(body: &[u8], precision: Option<&str>, format: WritePayloadFormat, max_points: usize) -> Result, HyperbytedbError>` in `application/ingest_metadata.rs`, next to `validate_point_count` (:86). + +- [ ] **Step 1: Write the failing test** `parse_write_body_counts_only_non_line_protocol`. With `max_points = 1`: + - a 2-line line-protocol body parses, since line protocol is limited inside the parser; + - a 2-point msgpack body returns `RequestPointLimitExceeded`; + - `ColumnarMsgpack` returns `Internal`. +- [ ] **Step 2: Implement it,** replace both blocks, and run the full gate. +- [ ] **Step 3: Commit** with the message "One write-body parser for both ingestion services". + +--- + +## Phase D — Hexagonal layering + +### Task 10: Password hashing moves to the domain + +**Validated:** `hash_password` lives in `adapters/http/auth_middleware.rs:309` (argon2) and is called from application code: `query_service.rs:1702` and `:1728`, and `peer_query_service.rs:1222`. It is also called from `bootstrap.rs:709`. + +**Interfaces:** +- Produces: `pub fn hash_password(password: &str) -> Result` in `domain/user.rs`. `auth_middleware` re-exports it, so HTTP code is unchanged. + +- [ ] **Step 1: Move the function and its test** (if `auth_middleware` has one), keep a `pub use` in `auth_middleware`, and point the three application callers at `crate::domain::user::hash_password`. Then run `grep -rn "adapters::http" src/application`. Expected: no output. +- [ ] **Step 2: Run the full gate, then commit** with the message "Password hashing lives in the domain". + +### Task 11: Raft request types and create-database logic leave the adapter + +**Validated:** +- `ClusterRequest` (`adapters/cluster/raft/types.rs:11`) and `ClusterResponse` (:19) depend only on serde and domain types. Application uses them in `drain.rs`, `shard_scheduler.rs:2667`, `peer_query_service.rs` and `shard_routing.rs`. +- `apply_create_database` (`adapters/cluster/raft/state_machine.rs:19`) is called from `schema_mutation_apply.rs:170`. + +**Files:** +- Create: `hyperbytedb/src/domain/cluster/raft_request.rs` +- Modify: `adapters/cluster/raft/types.rs` (re-export), `state_machine.rs`, `schema_mutation_apply.rs`, and the application users above + +- [ ] **Step 1: Write the fixture test** `cluster_request_json_is_unchanged` (Review Focus 1). Serialize one value of every `ClusterRequest` variant with `serde_json`, and compare against JSON literals captured from the current code *before* the move: run the test once, then paste its output into the assertions. +- [ ] **Step 2: Move the types** into `domain/cluster/raft_request.rs` with `pub use` from `raft/types.rs`. Move `apply_create_database` into `schema_mutation_apply.rs`, with `state_machine.rs` calling it there. +- [ ] **Step 3: Check the imports.** Run `grep -rn "adapters::cluster::raft::types\|raft::state_machine" src/application`. Expected: no output. +- [ ] **Step 4: Run the full gate, then commit** with the message "Raft request types and create-database logic leave the Raft adapter". + +### Task 12: Application uses `ShardMapPort`, not `RocksDbShardMap` + +**Validated:** +- Application calls only `snapshot()` (46 sites) and `apply_op()` (4) on the shard map; both are in `ShardMapPort` (`ports/sharding.rs:11,28`). `RocksDbShardMap::open` appears only at construction (bootstrap and tests). +- The concrete type appears in `ShardRoutingContext::shard_map` (`shard_routing.rs:30`), `ShardScheduler::new` (`shard_scheduler.rs:390`), `runtime/shard_heartbeat.rs` and `cluster/bootstrap.rs`. The last one is composition, see Task 14. + +- [ ] **Step 1: Change the types** in `shard_routing.rs`, `shard_scheduler.rs` and `runtime/shard_heartbeat.rs` to `Arc`. Construction sites coerce `Arc` to it. +- [ ] **Step 2: Check the imports.** Run `grep -rn "RocksDbShardMap" src/application --include=*.rs | grep -v "cluster/bootstrap.rs" | grep -v "mod tests" `. Expected: only test-module hits. +- [ ] **Step 3: Run the full gate** (the sharding integration tests cover this), **then commit** with the message "Application depends on ShardMapPort". + +### Task 13: Replication acknowledgement port for flush and drain + +**Validated:** +- `flush_service.rs` uses `ReplicationLog::get_wal_ack`, `min_max_wal_ack_for_peers(&self, peer_ids: &[u64]) -> Result<(u64, u64), _>` (`replication_log.rs:213`), `min_mutation_ack` (:349) and `truncate_mutations_before` (:374). +- `cluster/drain.rs` uses `last_mutation_seq` (:344) and `get_mutation_ack` (:326). + +**Files:** +- Create: `hyperbytedb/src/ports/replication_acks.rs` +- Modify: `flush_service.rs`, `cluster/drain.rs`, `adapters/cluster/replication_log.rs` (the impl) + +**Interfaces:** +- Produces: a `pub trait ReplicationAcks: Send + Sync` with exactly those six methods. Copy their signatures verbatim from `replication_log.rs:164,213,326,344,349,374`. `impl ReplicationAcks for ReplicationLog` delegates to the inherent methods. `FlushServiceImpl::with_replication_log` takes `Arc`. + +- [ ] **Step 1: Add the trait and impl, then swap the two users.** Run `grep -rn "replication_log::ReplicationLog" src/application | grep -v "cluster/bootstrap.rs\|mod tests"`. Expected: no output. +- [ ] **Step 2: Run the full gate** (the flush-service truncation tests at `flush_service.rs:1043+` cover this), **then commit** with the message "Flush and drain depend on a replication-acks port". + +### Task 14: Wire formats move to the domain; composition leaves `application` + +**Validated:** +- The line-protocol point codec (`POINTS_FRAME_MAGIC` :48, `is_binary_points` :52, `POINT_ENCODED_SIZE_HINT` :82, `encode_bincode_into` :86 in `application/line_protocol.rs`) is used by the adapters `wal/batching_wal.rs` and `cluster/peer_client.rs`. +- `ColumnarMsgpackBatch` (`application/columnar_msgpack.rs:21`) is used by `ports/points_sink.rs`, the only port→application reference, and by `native_adapter.rs`. +- `application/cluster/{bootstrap,leader_monitor,raft_formation}.rs` build concrete adapters, and take `AppState` from `adapters::http::router`. They are composition, called from `bootstrap.rs:12` and `server.rs:447,467`, plus one test import (`tests/sharding_failover_integration.rs:13`). + +**Files:** +- Create: `hyperbytedb/src/domain/point_codec.rs` and `hyperbytedb/src/domain/columnar_batch.rs` +- Create: `hyperbytedb/src/composition/mod.rs`, then move the three files there +- Modify: `lib.rs`, `application/cluster/mod.rs`, `bootstrap.rs`, `server.rs`, the test import, and `application/line_protocol.rs` / `application/columnar_msgpack.rs` (both keep `pub use` re-exports) + +- [ ] **Step 1: Move the codec items and `ColumnarMsgpackBatch` into the domain** with re-exports, and point `batching_wal.rs`, `peer_client.rs`, `native_adapter.rs` and `ports/points_sink.rs` at the domain paths. Then run `grep -rn "crate::application" src/ports src/adapters/wal src/adapters/cluster/peer_client.rs`. Expected: no output. +- [ ] **Step 2: Move the three composition files** to `src/composition/` (`pub mod composition;` in `lib.rs`), and update their imports and callers. No re-export is left at the old path. +- [ ] **Step 3: Recount the layering.** Re-run the layer count: `python3 /scan/layers.py src 'application>adapters'`. Expected: only the out-of-scope items listed at the end of this plan remain. +- [ ] **Step 4: Run the full gate, then commit** with the message "Wire formats in the domain; composition out of application". + +--- + +## Phase E — Write-path cost + +Measured on a release build, per batch of 5,000 Telegraf-shaped points with 10 fields each: +- parsing takes 24–27 ms; +- `split_field_type_conflicts` takes **5.7 ms**, and **1.3 ms** with Task 15's change; +- `PhysicalNameGuard::check_points` takes **1.35 ms**. + +### Task 15: Field-type check without per-field allocation + +**Validated:** `ingest_metadata.rs:705` runs `known.insert(k.clone(), widened)` for every field of every point. + +- [ ] **Step 1: Write the test** `field_widens_then_conflicts_within_one_batch` (Review Focus 5). It passes today and pins the behaviour. + - Use an empty store and one batch for `m`: field `f` as `Integer(1)`, then `Float(2.0)`, then `String("x")`. + - Expect the first two kept, the third dropped, and `partial == Some(PartialWrite { dropped: 1, .. })`. +- [ ] **Step 2: Add the benchmark** `split_field_type_conflicts_5000x10` to `benches/ingestion_line_protocol.rs`, with the same body shape as above. Run `cargo bench --bench ingestion_line_protocol -- split_field_type` and record the baseline. +- [ ] **Step 3: Change the loop** to update known fields in place (`get_mut` and widen) and clone a key only when it is new. Re-run the bench: expect about a 4× reduction. Then run the full gate. +- [ ] **Step 4: Commit** with both numbers in the message. + +### Task 16: Build the batch's name sets once + +**Validated:** `PhysicalNameGuard::check_points` (`physical_names.rs`) walks every point to collect field and tag keys per measurement. `prepare_batch_metadata` (`ingest_metadata.rs` ~:735) walks every point again to build its per-measurement `(field types, tag keys)`. The guard also takes `checked.read()` once per hash (`physical_names.rs:159`). + +**Interfaces:** +- Produces: `pub struct BatchSchema { pub measurements: HashMap, BTreeSet)> }` with `pub fn from_points(points: &[Point]) -> Result`, in `ingest_metadata.rs`. It returns the same in-batch `FieldTypeConflict` that `prepare_batch_metadata` raises today. +- The ingest paths change as follows: + - `PhysicalNameGuard::check_schema(&self, metadata, db, rp, schema: &BatchSchema)` replaces `check_points`; + - `prepare_batch_metadata_with_schema(metadata, db, rp, points, &schema, limits, schema_cache)` is the new entry the two client ingest paths call; + - `prepare_batch_metadata` keeps its signature for the other callers, and builds the schema itself. + +- [ ] **Step 1: Write the test** `batch_schema_matches_prepare_batch_metadata`. Using the Task 15 batch shape plus a second measurement, assert the keys and widened types per measurement. +- [ ] **Step 2: Add the benchmark** `name_guard_5000x10` and record the baseline. +- [ ] **Step 3: Implement.** Take the guard's read lock once per measurement, call `BatchSchema::from_points` once per request in `ingestion_service.rs` and `peer_ingestion_service.rs`, and pass the schema to both. Then run `cargo test --lib physical_names ingest_metadata` and `cargo test --test compat names_that_share_storage`. Expected: PASS. +- [ ] **Step 4: Re-run the bench** (expect the guard well under 0.1 ms), **run the full gate, then commit** with the numbers. + +--- + +## Not in this plan (validated, deliberately deferred) + +Each deferred item now has its own plan in this directory: +- `2026-11-03-peer-rpc-port.md`: the peer transport and Raft proposals; +- `2026-11-03-query-analysis-module.md`: the domain using the SQL translator; +- `2026-11-03-adapter-application-inversions.md`: driven adapters calling application code, plus the architecture test; +- `2026-11-03-split-large-functions.md`: very large functions; +- `2026-11-03-lint-and-test-support-cleanup.md`: the lower-value findings. + + +- **Peer transport port.** `PeerClient` (43 uses in application), direct `reqwest` calls in 11 application files, and Raft *proposals* (`client_write` with openraft's `ForwardToLeader` handling, in `peer_query_service.rs:129`, `drain.rs:252` and the scheduler at ~:2670) need a cluster-transport port designed as a whole. That redesign touches every cluster path and needs its own plan. +- **The domain using the SQL translator:** `domain/rollup.rs`, `domain/sharding/query_merge.rs` and `domain/sharding/multi_selector.rs` import from `timeseriesql::to_clickhouse`. `timeseriesql` sits outside the four layers, and moving this means splitting the translator. +- **Driven adapters calling application code:** + - `native_adapter.rs:552` calls `backfill_tag_metadata`; + - `sync_client` calls `wal_append`; + - the Raft state machine calls `apply_schema_mutation`. That one is an adapter driving the application, the same as HTTP handlers do, and it is acceptable. +- **Splitting very large functions:** `execute_statement` (1,220 lines), the cluster `execute_query` (570) and `apply_shard_map_op` (422). That is refactoring with no defect behind it. +- **Lower-value findings:** + - clippy's 39 lock scopes and 9 redundant clones (none on a hot path); + - the 24 test-only helpers; + - the per-batch `MeasurementMeta` copies, measured at microseconds and dropped. diff --git a/docs/superpowers/plans/2026-11-03-lint-and-test-support-cleanup.md b/docs/superpowers/plans/2026-11-03-lint-and-test-support-cleanup.md new file mode 100644 index 0000000..e134d2a --- /dev/null +++ b/docs/superpowers/plans/2026-11-03-lint-and-test-support-cleanup.md @@ -0,0 +1,111 @@ +# Lint and Test-Support Cleanup Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** +- Production builds no longer contain helpers that only tests call. +- Clippy's performance findings are fixed. +- The useful lints are switched on, so new regressions fail CI. + +**Architecture:** There are three small tasks: +1. Gate test-only helpers with `#[cfg(test)]`, and mark the ones integration tests need as `#[doc(hidden)]` test support. +2. Fix redundant clones, very large futures and hot-path lock scopes. +3. Add a `[lints.clippy]` table to the crate so CI's `-D warnings` enforces them. + +**Tech Stack:** Rust 1.94, clippy. + +**Spec:** the "lower-value findings" in `docs/superpowers/plans/2026-11-03-code-health-fixes.md`. Census taken at `7ea9643`; there is no `[lints]` table or `clippy.toml` today, and `lib.rs` only sets `warn(clippy::unwrap_used, clippy::expect_used)`. + +**Depends on:** code-health Task 2, which deletes `enqueue_reconciliation`, one of the test-only items. + +## Census (validated) + +Production-unused items referenced only by **unit** tests, and not gated today. These get `#[cfg(test)]`: +- `FlushServiceImpl::with_sharded_wal_retention_bytes` (`application/flush_service.rs:149`) +- `quote_backticks_validated` (`domain/chdb_naming.rs:126`) +- `as_concrete` (`timeseriesql/ast.rs:454`) +- `DEFAULT_TAG_LOW_CARDINALITY_MAX` (`adapters/chdb/native_adapter.rs:75`) +- `append_batch_sync` (`adapters/wal/rocksdb_wal.rs:358`) +- `total_pending` (`adapters/cluster/hinted_handoff.rs:219`) +- `min_wal_ack` (`adapters/cluster/replication_log.rs:187`) +- `read_mutations_from` (`adapters/cluster/replication_log.rs:259`) + +Items **integration** tests call. They can't be `#[cfg(test)]`, because integration tests link the non-test library. These get `#[doc(hidden)]` and a `/// Test support.` line: +- `lane_count` (`forward_coalescer.rs:83`) +- `parse_columnar_msgpack_to_points` (`columnar_msgpack.rs:144`) +- `with_defaults` (`replication_apply.rs:204`) +- `recorded_heartbeat_series` (`shard_scheduler.rs:859`) +- `tick_once_at` (`continuous_query_service.rs:75`) +- `empty_arrow_stream` (`ports/query.rs:20`) +- `total_rows` (`domain/prepared_wal.rs:31`) +- `engine_probe_count` (`native_adapter.rs:487`) +- `set_backends_for_test` (`hyperbytedb-proxy/src/pool.rs:85`) + +Already gated, no action: `with_test_force_leader`, `with_test_propose_sink`, `seed_rollup_dest`, `tick_once_for_test`, `try_failover_for_test`, `movers_in_flight` (`shard_scheduler.rs`), and `decide_transition` (`cluster/heartbeat.rs:65`, a test wrapper over the production `decide_transition_with`). + +Clippy hits (run with `-W clippy::`): +- **`redundant_clone`:** + - `adapters/chdb/native_adapter.rs:1367`, `adapters/http/router.rs:205`, `application/replication_apply.rs:143`; + - `domain/sharding/ops.rs:248`, `:280`; + - `domain/sharding/query_merge.rs:317`, `:326`; + - `server.rs:286`, `timeseriesql/to_clickhouse/materialized_view.rs:172`. +- **`large_futures`:** `application/cluster/bootstrap.rs:318`, `application/query_service.rs:693`, `server.rs:36`, `server.rs:163`. +- **`needless_collect`:** `adapters/chdb/native_adapter.rs:791`. +- **`iter_with_drain`:** `adapters/wal/batching_wal.rs:130`. +- **`or_fun_call`:** `application/runtime/region_write_stats.rs:22`, `application/shard_scheduler.rs:208`, `config.rs:566`, `:572`. +- **`significant_drop_tightening`** (a nursery lint, 39 hits). This plan fixes only the write/query hot-path ones: + - `adapters/chdb/native_adapter.rs:364`, `:365`, `:415`, `:507`, `:910`, `:922`, `:1213`, `:1224`; + - `domain/sharding/location_cache.rs:243`, `:290`, `:314`; + - `application/ingest_metadata.rs:275`, `application/physical_names.rs:172`; + - `application/runtime/region_write_stats.rs:21`, `:28`; + - `adapters/cluster/forward_queue.rs:272`, `application/statement_summary.rs:136`. + +Line numbers will have moved after earlier plans; re-run the lints to locate them. + +## Global Constraints + +- No behaviour change. +- `Cargo.lock` keeps chdb-rust 2.0.0 (clippy rewrites it, so reset it before each commit); commits carry no Claude trailers. +- The gate: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test --lib`, `cargo test --no-fail-fast --test '*'`, `cargo test -p hyperbytedb-cli`. + +## Review Focus + +1. **A lock scope narrowed past a read that needs it.** Narrowing must not split a read-then-write that relied on holding the lock (check-then-insert). Task 2 leaves any such site as is with a `#[allow(clippy::significant_drop_tightening)]` and a one-line reason. +2. **The proxy and CLI crates under the new lints.** The table goes in each crate's `Cargo.toml`. Task 3 runs `cargo clippy --workspace --all-targets`. +3. **`#[cfg(test)]` on an item a benchmark uses.** Benches link the non-test library too. Task 1's grep covers `benches/`. + +--- + +### Task 1: Gate or label test-only helpers + +- [ ] **Step 1: Re-check usage.** For every item in the two census lists, run `grep -rnw hyperbytedb*/src hyperbytedb/tests hyperbytedb/benches hyperbytedb-proxy/tests`. Move any item that now has a production or bench use out of the lists. +- [ ] **Step 2: Gate and label.** Add `#[cfg(test)]` to the eight unit-test-only items (and `#[cfg(test)]` to imports only they use), and `#[doc(hidden)]` plus `/// Test support.` to the nine integration-test helpers. +- [ ] **Step 3: Run the gate,** in particular `cargo build --release` with no test cfg, to catch a gated item still used. Then commit with the message "Test-only helpers leave production builds". + +### Task 2: Fix the clippy performance findings + +- [ ] **Step 1: Collect the current hits.** Run `cargo clippy --lib -p hyperbytedb -- -A clippy::all -W clippy::redundant_clone -W clippy::large_futures -W clippy::needless_collect -W clippy::iter_with_drain -W clippy::or_fun_call -W clippy::significant_drop_tightening`, then `git checkout -- Cargo.lock`. +- [ ] **Step 2: Fix them.** + - `redundant_clone`, `needless_collect`, `iter_with_drain`, `or_fun_call`: apply clippy's suggestion. + - `large_futures`: wrap the future in `Box::pin(...)`. + - `significant_drop_tightening`: fix only the hot-path sites listed in the census, ending the guard's scope (an inner block or `drop(guard)`) right after its last use. A check-then-act site gets the `allow` described in Review Focus 1. +- [ ] **Step 3: Re-run Step 1's command.** Expected: only `significant_drop_tightening` hits outside the hot-path list. Then run the gate and commit with the message "Fix clippy performance findings". + +### Task 3: Turn the lints on + +**Files:** `hyperbytedb/Cargo.toml`, `hyperbytedb-cli/Cargo.toml`, `hyperbytedb-proxy/Cargo.toml`. + +- [ ] **Step 1: Add the lint table.** In each crate, add: + + ```toml + [lints.clippy] + redundant_clone = "warn" + large_futures = "warn" + needless_collect = "warn" + iter_with_drain = "warn" + or_fun_call = "warn" + ``` + + CI's `-D warnings` makes these errors. `significant_drop_tightening` stays off because of its false positives. +- [ ] **Step 2: Run** `cargo clippy --workspace --all-targets -- -D warnings`. Expected: clean. Fix any hit in the CLI or proxy the same way as Task 2, then reset `Cargo.lock`. +- [ ] **Step 3: Run the gate, then commit** with the message "Enable clippy performance lints". diff --git a/docs/superpowers/plans/2026-11-03-peer-rpc-port.md b/docs/superpowers/plans/2026-11-03-peer-rpc-port.md new file mode 100644 index 0000000..df1e16e --- /dev/null +++ b/docs/superpowers/plans/2026-11-03-peer-rpc-port.md @@ -0,0 +1,195 @@ +# Peer RPC and Consensus Ports Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Application code stops building HTTP requests and stops holding the concrete `PeerClient` and Raft handle. Every node-to-node call goes through two ports: `PeerRpc` for typed calls to peers, and `ConsensusPort` for Raft proposals. + +**Architecture:** +- **`PeerRpc`** has one typed method per internal endpoint. Methods return a `PeerRpcError` that keeps the HTTP status, because call sites act on 404, 405, 409, 429 and 503. A reqwest adapter implements it, reusing the pooled clients `PeerClient` already has. +- **`ConsensusPort`** extends the code-health plan's `LeadershipPort`. Its `propose` either writes to the local Raft leader or forwards to the leader, as `PeerQueryService::client_write_with_forward` does today. +- **Rollout:** call sites move one endpoint group at a time, so each task is a behaviour-neutral refactor that the sharding and Raft integration suites check. + +**Tech Stack:** Rust 2024, async-trait 0.1, futures 0.3, bytes 1, reqwest 0.12 (no new features), openraft 0.9. + +**Spec:** the deferred item "Peer transport port" in `docs/superpowers/plans/2026-11-03-code-health-fixes.md`, and the census below, taken at `7ea9643`. + +**Depends on** these code-health tasks: +- Task 2 deletes `push_and_drop_range` / `request_region_rehome`, which are the only callers of `/internal/shard/rehome`. +- Task 8 adds `LeadershipPort`. +- Task 11 moves `ClusterRequest` / `ClusterResponse` to `domain/cluster/raft_request.rs`. +- Task 12 makes application use `ShardMapPort`. +- Task 14 moves `leader_monitor.rs` and `raft_formation.rs` out of application. + +Re-run the census command in Task 1, Step 1 before starting. Line numbers will have moved. + +## Census (validated at `7ea9643`) + +These are the internal endpoints that application code calls today. "Sites" are file:line in `hyperbytedb/src/application/`; "status" lists the codes the site handles specially. + +| Endpoint | Sites | Request | Response use | Status | +|---|---|---|---|---| +| `/internal/shard/bootstrap` | `shard_routing.rs:197` | `ShardBootstrapRequest` (`domain/sharding/types.rs:460`) | none | — | +| `/internal/shard/write` | `shard_routing.rs:899` | raw body bytes, headers as sent today | `text()` | 409, 503, 429 | +| `/internal/shard/write-batch` | `forward_coalescer.rs:258` | raw body bytes | `ShardWriteBatchResponse` (`types.rs:454`) | 404, 405, 503, 429 | +| `/internal/shard/query` | `query_service.rs:2517`, `:2605` | `ShardQueryRequest` (`types.rs:468`) | Arrow IPC **stream**, read with `resp.chunk()` (`query_arrow_series.rs:325`); the error body via `text()` | 409 | +| `/internal/shard/metadata` | `shard_metadata_scatter.rs:49` | `ShardMetadataRequest` (`types.rs:488`) | body (decode kept at the site) | 409 | +| `/internal/shard/delete` | `shard_routing.rs:1087` | `ShardDeleteRequest` (`types.rs:516`) | none | 409 | +| `/internal/shard/mv-backfill` | `sharded_mv_backfill.rs:182` | `ShardMvBackfillRequest` (`types.rs:548`) | none | 409, 503 | +| `/internal/shard/transfer/raw` | `shard_transfer.rs:687` | text body (`CONTENT_TYPE` text/plain) | `Response` | 404 means fall back to the JSON form | +| `/internal/shard/transfer` | `shard_transfer.rs:705` (push), `:739` (ack) | `ShardTransferPayload` (`domain/sharding/transfer.rs:26`; `ShardTransferPayload::ack(..)` for the ack) | body via `rejection_reason` (`shard_transfer.rs:1075`) | 404 | +| `/internal/shard/heartbeat` | `runtime/shard_heartbeat.rs:203` (`post_region_heartbeat`) | `RegionHeartbeat` (`types.rs:396`) | `"stale_epoch"` reason string | — | +| `/internal/mv/handoff` | `shard_scheduler.rs:2142` | `MvHandoffRequest` (`application/mv_handoff.rs:38`; **moves to domain**) | none | 404 | +| `/internal/sync/manifest` | `shard_scheduler.rs:3120` (`peer_region_wal_watermark`) | GET | `SyncManifest` (`domain/cluster/sync.rs`) from `text()` | — | +| `/internal/sync/trigger` | `cluster/hinted_handoff.rs:59` | POST, no body (unverified: read the site) | none | — | +| `/cluster/raft/client-write` | `peer_query_service.rs:292` (`forward_client_write`), `shard_routing.rs:965` (`propose_shard_map_op_via_raft`) | `ClusterRequest` | `text()` | — | +| `/health`, `/ping` | `cluster/heartbeat.rs:254-255` (`probe_peers`) | GET | `text()` / status | — | + +Raft proposals made directly with `client_write`: +- `peer_query_service.rs:129` (`client_write_with_forward`, which retries 3 times and follows `ForwardToLeader`); +- `cluster/drain.rs:252`; +- `shard_scheduler.rs:~2667` (checks `resp.data.ok`). + +Application currently uses `PeerClient` for: +- `membership()` (`peer_client.rs:164`, which returns `&SharedMembership` from `domain/cluster/membership.rs:235`); +- `http_client()` (`:193`); +- `cluster_auth()` (`:170`); +- `pending_hints()`. + +Two port-like pieces exist already: +- `adapters/cluster/transport.rs` holds an `HttpTransport` / `ReqwestTransport` seam. Its header comment says it is not wired up, and it has no references outside the file. +- `ports/peer_http.rs` `PeerHttp` exposes `reqwest::ClientBuilder` in a port signature. + +## Global Constraints + +- The wire format of every endpoint is unchanged: path, method, headers, body and status codes. The server-side handlers are not touched. +- No new crate features. Stream the query body with `reqwest::Response::chunk()` inside `futures::stream::unfold`. +- `Cargo.lock` keeps chdb-rust 2.0.0; run `git checkout -- Cargo.lock` before each commit unless a task changes dependencies. +- Commits carry no Claude trailers. +- The gate for every task: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test --lib`, `cargo test --no-fail-fast --test '*'`, `cargo test -p hyperbytedb-cli`. + +## Review Focus + +1. **A transport error versus an HTTP error status.** "Unreachable" must stay distinct from "answered 503". Today the forward queue retries 503 and 429 but not other codes; Task 2's adapter test pins `PeerRpcError::Status { code: 503 }` against `Unreachable`. +2. **A shard query streaming a large Arrow result.** It must still be decoded chunk by chunk, not buffered whole. Task 3 tests a multi-chunk stub response. +3. **A leader change mid-proposal.** `propose` must follow `ForwardToLeader` and retry 3 times, as today. Task 6 tests it with a stub leadership/forwarder. +4. **The transfer push against an older peer without `/transfer/raw`.** A 404 must fall back to the JSON push. Task 4 keeps that branch and tests it with a stub returning 404. +5. **Shard deletes during a peer outage.** A failed delete must count as `failed` in `ShardDeleteOutcome` and not abort the fan-out. The existing `sharding_cluster_integration` delete tests cover this; Task 3 must keep them green. + +--- + +### Task 1: Error type, port skeleton and reqwest adapter; remove the unused seams + +**Files:** +- Create: `hyperbytedb/src/domain/cluster/peer_rpc.rs`, `hyperbytedb/src/ports/peer_rpc.rs`, `hyperbytedb/src/adapters/cluster/http_peer_rpc.rs` +- Delete: `hyperbytedb/src/adapters/cluster/transport.rs` (and its `pub mod transport;` at `adapters/cluster/mod.rs:9`) +- Move: `MvHandoffRequest` from `application/mv_handoff.rs:38` to `domain/sharding/types.rs`, with a `pub use` left in `mv_handoff.rs` + +**Interfaces:** +- Produces: `pub enum PeerRpcError { Status { code: u16, body: String }, Unreachable(String) }`, with `impl From for HyperbytedbError`: + - `Status` becomes `PeerUnreachable(format!("HTTP {code}: {body}"))`; + - `Unreachable(s)` becomes `PeerUnreachable(s)`. +- Produces: `#[async_trait] pub trait PeerRpc: Send + Sync`. Its methods are added by Tasks 3–5; this task adds the trait, `pub type SharedPeerRpc = Arc`, and the adapter `pub struct HttpPeerRpc { peers: Arc }` with two private helpers: + - `async fn post_json(&self, addr: &str, path: &str, req: &Req, timeout: Duration) -> Result`; + - `async fn get(&self, addr: &str, path: &str, timeout: Duration) -> Result`. + + Both map a non-2xx status to `Status { code, body: resp.text() }` and a send error to `Unreachable`. + +- [ ] **Step 1: Re-run the census** and update the table if it changed: `grep -rnE '"(http://\{[^}]*\})?/(internal|cluster|health|ping)' hyperbytedb/src/application` +- [ ] **Step 2: Write the failing adapter test** `status_and_transport_errors_stay_distinct` in `http_peer_rpc.rs`. Use an axum stub on an ephemeral port (as in `forward_coalescer.rs` tests, `:319`) that answers 503 with body `busy`. Assert: + - `post_json` returns `Status { code: 503, body: "busy" }`; + - against a closed port it returns `Unreachable(_)`. +- [ ] **Step 3: Implement the type, trait and adapter.** Delete `transport.rs`; `grep -rnw "HttpTransport\|ReqwestTransport"` must find nothing. Move `MvHandoffRequest`. +- [ ] **Step 4: Run the gate, then commit** with the message "PeerRpc port skeleton and reqwest adapter; drop the unused transport seam". + +### Task 2: Wire the port in + +**Files:** `ShardRoutingContext` (`application/shard_routing.rs:29`) and its construction sites; `ShardScheduler::new` (`shard_scheduler.rs:389`) and its 8 call sites; `application/runtime/shard_heartbeat.rs`; `application/query_service.rs` (`with_sharding`, `:125`). + +**Interfaces:** +- `ShardRoutingContext` gains `pub peer_rpc: SharedPeerRpc` and `pub membership: SharedMembership`, next to `peer_client` (which is removed in Task 7). The scheduler and heartbeat reporter receive `SharedPeerRpc` the same way. + +- [ ] **Step 1: Add the fields and parameters,** and construct `HttpPeerRpc::new(peer_client.clone())` where `PeerClient` is built today (composition code after code-health Task 14). +- [ ] **Step 2: Run the gate,** expecting no behaviour change, **then commit** with the message "Thread PeerRpc through sharding contexts". + +### Task 3: Shard data path (bootstrap, write, write-batch, query, metadata, delete) + +**Interfaces:** add to `PeerRpc`: +- `async fn shard_bootstrap(&self, addr: &str, req: &ShardBootstrapRequest, timeout: Duration) -> Result<(), PeerRpcError>` +- `async fn shard_write(&self, addr: &str, body: bytes::Bytes, timeout: Duration) -> Result<(), PeerRpcError>`. It sends exactly the headers `try_forward_shard_write_to_region` sends today. +- `async fn shard_write_batch(&self, addr: &str, body: bytes::Bytes, timeout: Duration) -> Result` +- `async fn shard_query(&self, addr: &str, req: &ShardQueryRequest, timeout: Duration) -> Result>, PeerRpcError>` +- `async fn shard_metadata(&self, addr: &str, req: &ShardMetadataRequest, timeout: Duration) -> Result` +- `async fn shard_delete(&self, addr: &str, req: &ShardDeleteRequest, timeout: Duration) -> Result<(), PeerRpcError>` + +`series_from_ipc_response` (`query_arrow_series.rs:325`) changes its first parameter to `stream: BoxStream<'static, Result>`. + +- [ ] **Step 1: Write the failing test** `shard_query_decodes_a_multi_chunk_stream` in `query_arrow_series.rs`. Feed `series_from_ipc_response` a `futures::stream::iter` of an Arrow IPC stream split into 3-byte chunks, and assert the same series as one whole chunk. +- [ ] **Step 2: Implement the six adapter methods.** The query stream is `futures::stream::unfold(resp, |mut r| async { r.chunk().await.transpose().map(|c| (c.map_err(..), r)) })`. +- [ ] **Step 3: Migrate the call sites.** Each site keeps its status handling, now written as a `match` on `PeerRpcError::Status { code, .. }` with the same codes: + - `shard_routing.rs:197`, `:899`, `:1087`; + - `forward_coalescer.rs:258`, which no longer takes a `reqwest::Client`; its `send_jobs` and `post_batch` take `&dyn PeerRpc`; + - `query_service.rs:2517`, `:2605`; + - `shard_metadata_scatter.rs:49`. +- [ ] **Step 4: Run the gate,** especially `cargo test --test sharding_cluster_integration --test sharding_integration`, **then commit** with the message "Shard data path goes through PeerRpc". + +### Task 4: Transfers, backfill, handoff and heartbeats + +**Interfaces:** add to `PeerRpc`: +- `async fn shard_transfer_push(&self, addr: &str, payload: &ShardTransferPayload, raw_body: Option, timeout: Duration) -> Result`. With a raw body it POSTs `/transfer/raw` (text/plain) first, and on a 404 falls back to the JSON payload, as `send_transfer_push` (`shard_transfer.rs:~674`) does today. +- `async fn shard_transfer_ack(&self, addr: &str, payload: &ShardTransferPayload, timeout: Duration) -> Result<(), PeerRpcError>` +- `async fn shard_mv_backfill(&self, addr: &str, req: &ShardMvBackfillRequest, timeout: Duration) -> Result<(), PeerRpcError>` +- `async fn mv_handoff(&self, addr: &str, req: &MvHandoffRequest, timeout: Duration) -> Result<(), PeerRpcError>` +- `async fn shard_heartbeat(&self, addr: &str, hb: &RegionHeartbeat, timeout: Duration) -> Result<(), PeerRpcError>` + +`rejection_reason` (`shard_transfer.rs:1075`) takes `&PeerRpcError` instead of `reqwest::Response`; its 4 tests (`:1199-1234`) are rewritten to match. + +- [ ] **Step 1: Write the failing test** `transfer_push_falls_back_to_json_on_404`. An axum stub answers 404 on `/internal/shard/transfer/raw` and 200 on `/internal/shard/transfer`. Assert one call to each path. +- [ ] **Step 2: Implement the five methods and migrate the sites:** + - `shard_transfer.rs:687`, `:705`, `:739`; + - `sharded_mv_backfill.rs:182`; + - `shard_scheduler.rs:2142`; + - `runtime/shard_heartbeat.rs:203`, keeping the `"stale_epoch"` reason mapping. +- [ ] **Step 3: Run the gate,** especially `--test sharding_mv_integration --test sharding_placement_integration`, **then commit** with the message "Transfers, backfill, handoff and heartbeats go through PeerRpc". + +### Task 5: Cluster control (sync manifest and trigger, health probes) + +**Interfaces:** add to `PeerRpc`: +- `async fn sync_manifest(&self, addr: &str, timeout: Duration) -> Result` +- `async fn sync_trigger(&self, addr: &str, timeout: Duration) -> Result<(), PeerRpcError>`. Copy the method and body from `hinted_handoff.rs:59` (not yet read). +- `async fn probe(&self, addr: &str, timeout: Duration) -> Result`. It GETs `/health` and falls back to `/ping` exactly as `probe_peers` (`cluster/heartbeat.rs:254`) does; read that function first. + +- [ ] **Step 1: Write the failing test** `probe_reports_health_body`, using an axum stub that serves `/health`. +- [ ] **Step 2: Implement and migrate:** `shard_scheduler.rs:3120` (`peer_region_wal_watermark`), `cluster/hinted_handoff.rs:59`, and `cluster/heartbeat.rs:254-255`. If `leader_monitor.rs` / `raft_formation.rs` are still in application (code-health Task 14 not landed), migrate their `:80`, `:157` and `:134` calls too. +- [ ] **Step 3: Run the gate, then commit** with the message "Cluster control calls go through PeerRpc". + +### Task 6: ConsensusPort + +**Files:** create `hyperbytedb/src/ports/consensus.rs` and `hyperbytedb/src/adapters/cluster/raft/consensus.rs`. + +**Interfaces:** +- Produces: `#[async_trait] pub trait ConsensusPort: LeadershipPort { async fn propose(&self, req: ClusterRequest) -> Result; }` +- Adapter: `pub struct RaftConsensus { raft: HyperbytedbRaft, rpc: SharedPeerRpc, membership: SharedMembership }`. It does what `client_write_with_forward` (`peer_query_service.rs:121`) does today: + - write locally when this node is leader; + - on `ForwardToLeader`, POST `/cluster/raft/client-write` to the leader through a new `PeerRpc::raft_client_write(&self, addr: &str, req: &ClusterRequest, timeout: Duration) -> Result`; + - make at most 3 attempts. + + It returns the `ClusterResponse`; callers keep checking `.ok`, as `shard_scheduler.rs:~2675` does. + +- [ ] **Step 1: Write the failing test** `propose_follows_forward_to_leader`. A stub `PeerRpc` records `raft_client_write` calls; a stub Raft is not possible, so drive `RaftConsensus` through its forwarding branch by factoring that branch into `async fn forward(&self, leader_addr: &str, req: &ClusterRequest) -> Result` and testing it directly. The 3-attempt loop is covered by `tests/raft_integration.rs`. +- [ ] **Step 2: Implement and migrate the proposers:** + - `peer_query_service.rs` (`client_write_with_forward` and `forward_client_write` are deleted; the `raft_client: reqwest::Client` field at `:44` goes); + - `cluster/drain.rs:252`; + - `shard_scheduler.rs:~2667`; + - `shard_routing.rs:960` `propose_shard_map_op_via_raft`. + + The scheduler's `raft: HyperbytedbRaft` field becomes `consensus: Arc`, and its `is_leader` (`:942`) uses it. +- [ ] **Step 3: Run the gate,** especially `--test raft_integration --test sharding_failover_integration`, **then commit** with the message "Raft proposals go through ConsensusPort". + +### Task 7: Remove `PeerClient` and reqwest from application; ports free of reqwest + +- [ ] **Step 1: Remove the remaining fields and imports.** Remove `ShardRoutingContext::peer_client` and the scheduler's `peer_client`; move `pending_hints` use in `cluster/hinted_handoff.rs` behind a `fn pending_hints(&self) -> usize` on `PeerRpc` (or keep `hinted_handoff.rs` in composition if code-health Task 14 moved it). Delete `ports/peer_http.rs` if `grep -rnw PeerHttp src` shows only the adapter impl; otherwise change its signature to not expose `reqwest`. +- [ ] **Step 2: Check what's left.** Each command must print nothing: + - `grep -rn "reqwest::\|PeerClient\|HyperbytedbRaft\|openraft::" src/application --include=*.rs | grep -v "mod tests"` + - `grep -rn "reqwest" src/ports` +- [ ] **Step 3: Remove the matching allowlist entries** from the architecture test added by `2026-11-03-adapter-application-inversions.md` Task 4, if it has landed. +- [ ] **Step 4: Run the gate, then commit** with the message "Application holds no HTTP client or Raft handle". diff --git a/docs/superpowers/plans/2026-11-03-query-analysis-module.md b/docs/superpowers/plans/2026-11-03-query-analysis-module.md new file mode 100644 index 0000000..94e427f --- /dev/null +++ b/docs/superpowers/plans/2026-11-03-query-analysis-module.md @@ -0,0 +1,75 @@ +# Query Analysis Module Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Domain code stops importing the ClickHouse SQL translator (`timeseriesql::to_clickhouse`). The pure InfluxQL AST analysis it uses moves to a translator-independent module. + +**Architecture:** Add `timeseriesql/analysis.rs` for AST facts that hold whatever the backend: +- output column names; +- whether a SELECT aggregates; +- the TOP/BOTTOM/SAMPLE selector descriptor; +- the shared merge-column names. + +`to_clickhouse` re-exports what it used to define, so the translator and its 1,500+ lines of tests are unchanged. Domain imports from `analysis`. + +**Tech Stack:** Rust 2024. + +**Spec:** the deferred item "the domain using the SQL translator" in `docs/superpowers/plans/2026-11-03-code-health-fixes.md`, validated at `7ea9643`. + +**Independent** of the other plans. + +## What domain imports today (validated) + +| Domain file | Imports | Defined at | +|---|---|---| +| `domain/rollup.rs` | `to_clickhouse::select_output_field_name` | `to_clickhouse/select.rs:859` | +| `domain/sharding/query_merge.rs:17` | `select_has_true_aggregate`, `select_output_field_name` | `select.rs:1058`, `:859` | +| `domain/sharding/multi_selector.rs:21` | `BUCKET_COL`, `MultiSelector`, `SelectorKind`, `TIME_NS_COL` | `to_clickhouse/selectors.rs:25`, `:38`, `:30`, `:27` | + +Their dependency closure, all pure AST code: +- `select_output_field_name` uses `default_field_alias` (`select.rs:872`), `influxql_name` (`:879`) and `expr_name_parts` (`:885`). +- `select_has_true_aggregate` uses `expr_contains_aggregate` (`aggregates.rs:24`), which uses `is_window_transform_call` (`aggregates.rs:9`) and `expr_contains_raw_transform` (`aggregates.rs:40`). +- `MultiSelector` and `SelectorKind` are plain data. + +`multi_selector()` (`selectors.rs:80`) uses translator helpers (`with_output_names`), and domain does not call it, so it **stays** in `to_clickhouse`. + +## Global Constraints + +- No behaviour change, and the generated SQL is byte-identical. The existing `to_clickhouse/tests.rs` must pass unchanged. +- `Cargo.lock` keeps chdb-rust 2.0.0; commits carry no Claude trailers. +- The gate: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test --lib`, `cargo test --no-fail-fast --test '*'`. + +## Review Focus + +1. **Output names of unaliased expressions** (`mean("v") * 2` named `mean`, `"a" + "b"` named `a_b`). These must be identical through the re-export. The existing translator tests cover this; Task 1 also adds direct `analysis` tests. +2. **Window transforms** (`difference`, `derivative`) are not aggregates. `select_has_true_aggregate` must still say `false` for them; Task 1 tests it. +3. **The sharded TOP/BOTTOM merge** reads `BUCKET_COL` / `TIME_NS_COL` columns the translator emits. Both must keep the same string values; Task 1 asserts the literals. + +--- + +### Task 1: Move the AST analysis into `timeseriesql/analysis.rs` + +**Files:** +- Create: `hyperbytedb/src/timeseriesql/analysis.rs`, and add `pub mod analysis;` in `timeseriesql/mod.rs` +- Modify: `to_clickhouse/select.rs`, `to_clickhouse/aggregates.rs`, `to_clickhouse/selectors.rs`, `to_clickhouse/mod.rs` (re-exports) +- Modify: `domain/rollup.rs`, `domain/sharding/query_merge.rs`, `domain/sharding/multi_selector.rs` + +**Interfaces:** `analysis.rs` exports: +- `pub fn select_output_field_name(field: &Field) -> Option` +- `pub fn default_field_alias(expr: &Expr) -> Option` +- `pub fn select_has_true_aggregate(stmt: &SelectStatement) -> bool` +- `pub fn expr_contains_aggregate(expr: &Expr) -> bool` +- `pub fn is_window_transform_call(name: &str) -> bool` +- `pub fn expr_contains_raw_transform(expr: &Expr) -> bool` +- `pub enum SelectorKind`, `pub struct MultiSelector` +- `pub const BUCKET_COL: &str = "__bucket"`, `pub const TIME_NS_COL: &str = "__tns"` + +`to_clickhouse` keeps the old paths with `pub use crate::timeseriesql::analysis::{...}` (`pub(super)` items become `pub(crate)` re-exports). + +- [ ] **Step 1: Write the failing tests** in `analysis.rs`: + - `output_names_follow_influxql`: `mean("v") * 2` gives `Some("mean")`; `"a" + "b"` gives `Some("a_b")`; a bare `"v"` gives `None`; an alias wins. + - `window_transforms_are_not_aggregates`: `SELECT difference("v") FROM m` gives `false`; `SELECT mean("v") FROM m` gives `true`. + - `merge_column_names_are_stable`: `BUCKET_COL == "__bucket"` and `TIME_NS_COL == "__tns"`. +- [ ] **Step 2: Run the tests** with `cargo test --lib timeseriesql::analysis`. Expected: FAIL, because the module doesn't exist yet. +- [ ] **Step 3: Move the items** listed above (with their private helpers `influxql_name`, `expr_name_parts`), add the re-exports, and switch the three domain files to `crate::timeseriesql::analysis`. Then run `grep -rn "to_clickhouse" src/domain`. Expected: no output. +- [ ] **Step 4: Run the gate,** expecting `to_clickhouse/tests.rs` to pass unchanged, **then commit** with the message "Domain uses AST analysis, not the SQL translator". diff --git a/docs/superpowers/plans/2026-11-03-split-large-functions.md b/docs/superpowers/plans/2026-11-03-split-large-functions.md new file mode 100644 index 0000000..3a0e5ac --- /dev/null +++ b/docs/superpowers/plans/2026-11-03-split-large-functions.md @@ -0,0 +1,112 @@ +# Split the Largest Functions Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Break up the three largest functions with no behaviour change: +- `execute_statement` (1,228 lines); +- the cluster `execute_query` (578 lines); +- `apply_shard_map_op` (426 lines). + +The cluster `execute_query`'s 18 hand-written replicate arms collapse onto the single mutation builder. + +**Architecture:** +- **Cluster `execute_query`:** a golden test first records the exact mutation every arm replicates today. The arms are then replaced by one path: run the statement locally, and on success replicate `mutation_request_from_statement(...)`. Only arms whose golden output differs from the builder keep custom code. +- **The other two:** pure moves. Each match arm becomes a named function in a focused module, and the match becomes a dispatch table. Review moved code with `git diff --color-moved=zebra`: moved lines must show as moved, not changed. + +**Tech Stack:** Rust 2024, serde_json (for golden snapshots). + +**Spec:** the deferred item "splitting very large functions" in `docs/superpowers/plans/2026-11-03-code-health-fixes.md`, validated at `7ea9643`. + +**Depends on:** code-health Task 1, which makes `mutation_request_from_statement` take `retention_policy` (Task 2 here needs it). + +## Census (validated) + +- **`application/query_service.rs:705-1932`** `async fn execute_statement(svc: &QueryServiceImpl, db: &str, stmt: &Statement, epoch: Option<&str>, query_rp: Option<&str>, statement_id: u32) -> Result` has 32 arms. The largest: + - `ShowTagValues` (@856, 200 lines), `DropSeries` (@1774, 91), `Select` (@1135, 83); + - `ShowTagKeys` (@790, 66), `ShowSeries` (@1221, 60), `Delete` (@1315, 60); + - `ShowFieldKeys` (@1056, 55), `ShowRetentionPolicies` (@1561, 44), `ShowMaterializedViews` (@1460, 40); + - `Revoke` (@1896, 36), `ShowSettings` (@1513, 35), `DropMeasurement` (@1281, 34); + - `ShowContinuousQueries` (@1402, 33), `CreateRetentionPolicyStmt` (@1655, 32). + + The file is 4,023 lines. +- **`application/peer_query_service.rs:633-1210`** `execute_query` has 18 arms of the same shape: `execute_inner_statement(...)`, then on success `self.replicate_mutation(MutationRequest::X { .. })`. + - The arms are `Delete`, `CreateContinuousQuery`, `DropContinuousQuery`, `CreateMaterializedView`, `DropMaterializedView`, `CreateRetentionPolicyStmt`, `DropRetentionPolicyStmt`, `CreateUser`, `SetPassword`, `AlterRetentionPolicyStmt`, `DropSeries`, `DropMeasurement`, `Grant`, `Revoke` and `SetSetting`, plus `CreateDatabase` and `DropDatabase`. + - Known extras: `CreateRetentionPolicyStmt` (@861) binds `ref db` from the statement, shadowing the request's `db`, and derives the shard duration; `SetSetting` (@1168) runs `runtime_settings::validate`. + - `mutation_request_from_statement` (`:402`) builds the same requests for the Raft path. +- **`domain/sharding/ops.rs:237-662`** `pub fn apply_shard_map_op(map: &mut ShardMap, op: ShardMapOp) -> Result<(), ShardMapApplyError>` has 12 arms: `Split` (66 lines), `RemovePeer` (55), `Merge` (42), `MovePeer` (40), `TransferPrimary` (38), `AddPeer` (36), `BootstrapMeasurementPresplit` (32), `PromotePeer` (29), `ActivateRegion` (26), `BootstrapMeasurement` (25), `ClearVerified` (22), `DropSpace` (8). It bumps `map.map_version` before the match. +- `PeerQueryService::new(inner, metadata, replication_port: Arc, peer_http)` (`peer_query_service.rs:48`). `ReplicationPort` (`ports/replication.rs`) has `replicate_write`, `replicate_write_sync`, `replicate_mutation`, `replicate_mutation_sync` and `replication_peer_count`, so a recording stub is possible. `MutationRequest` is `Serialize` (`domain/cluster/types.rs:9`). +- Not in scope: `FlushServiceImpl::flush` (263 lines). Its chunk paths are already extracted (`flush_prepared_chunk`), and what remains is ordering-sensitive loop control. + +## Global Constraints + +- No behaviour change, and the SQL and replicated mutations are byte-identical (except argon2 hashes, which are salted). +- `Cargo.lock` keeps chdb-rust 2.0.0; commits carry no Claude trailers. +- The gate: `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings`, `cargo test --lib`, `cargo test --no-fail-fast --test '*'`. + +## Review Focus + +1. **`CREATE RETENTION POLICY … ON otherdb` sent with `?db=mydb`.** The replicated `db` must stay whatever it is today. Task 1's golden test includes this exact statement. +2. **`CREATE USER` / `SET PASSWORD` replication.** It must carry a hash, never the plain password. The golden test asserts the `password_hash` field starts with `$argon2` and is not equal to the input. +3. **`SET ` with an invalid value.** It must still be refused before anything replicates. The golden test includes one invalid `SET` and asserts nothing was recorded. +4. **Shard-map op version bumps.** A refused op must still bump `map_version` exactly as today; the bump stays in the dispatcher. Task 4 adds `refused_op_still_bumps_version`. +5. **`SHOW TAG VALUES` with `WITH KEY =~ /re/` and a `WHERE`** (its 200-line arm). The output must be unchanged; the compat `metadata_tests` cover it and must pass untouched. + +--- + +### Task 1: Golden test of every replicated mutation + +**Files:** create `hyperbytedb/tests/compat/cluster_replication_tests.rs` (add `mod cluster_replication_tests;` in `tests/compat/main.rs`; it needs `TestContext`, which lives in the integration-test crate) and the fixture `hyperbytedb/tests/compat/testdata/replicated_mutations.json`. + +**Interfaces:** +- `struct RecordingReplication { seen: parking_lot::Mutex> }`, which implements `ReplicationPort` by recording in both `replicate_mutation` and `replicate_mutation_sync`. The write methods return `Ok` and `replication_peer_count` returns 0. + +- [ ] **Step 1: Write the test** `every_cluster_mutation_replicates_as_before`. + - Build `PeerQueryService::new(ctx.query_service.clone(), ctx.metadata.clone(), Arc::new(RecordingReplication::default()), &ClusterAuth::new(None)?)`, with `ctx` from `TestContext::new_no_chdb()`; that is non-Raft, as `raft` is never set. + - Run one statement per arm (18), plus `CREATE RETENTION POLICY r ON otherdb DURATION 1d REPLICATION 1` sent with `db = "mydb"`, plus an invalid `SET`. + - Serialize the recorded list with `serde_json`, masking every `password_hash` to `""` after asserting it starts with `$argon2`. + - Compare against the fixture. On the first run, write the fixture when the env var `UPDATE_GOLDEN=1` is set. +- [ ] **Step 2: Generate and check the fixture.** Run it on the unchanged code with `UPDATE_GOLDEN=1`, inspect the fixture by eye, and run again without the env var. Expected: PASS. +- [ ] **Step 3: Commit** with the message "Golden test of replicated cluster mutations". + +### Task 2: Collapse the replicate arms onto the builder + +- [ ] **Step 1: Replace the arms.** In `execute_query`, replace the 18 arms with: + + ```rust + stmt if is_cluster_mutation(&stmt) => { + let result = self.execute_inner_statement(db, stmt_query, epoch, retention_policy, caller, statement_id).await; + if result.error.is_none() { + let req = mutation_request_from_statement(&stmt, db, retention_policy, &self.metadata, self.shard_routing.get()).await?; + self.replicate_mutation(req).await?; + } + result + } + ``` + + Keep the `_ =>` fallback. +- [ ] **Step 2: Run the golden test.** For each statement whose output differs, the builder disagrees with the inline arm. Make the builder match the inline arm, since that is what has been replicating in non-Raft clusters, unless the inline behaviour is the bug code-health Task 1 fixed. Note each change in the commit message. Re-run until PASS. +- [ ] **Step 3: Run the gate,** especially `--test raft_integration --test sync_quorum_integration --test security_auth`, **then commit** with the message "Cluster statements replicate through one builder". + +### Task 3: `execute_statement` becomes a dispatcher over statement modules + +**Files:** `application/query_service.rs` becomes `application/query_service/mod.rs`, plus these new files under `application/query_service/statements/`: +- `show_schema.rs`: `ShowTagValues`, `ShowTagKeys`, `ShowSeries`, `ShowFieldKeys`, and any other `Show*` over measurement schema; +- `show_catalog.rs`: `ShowRetentionPolicies`, `ShowMaterializedViews`, `ShowContinuousQueries`, `ShowSettings`, and `ShowDatabases` / `ShowUsers` if present; +- `ddl.rs`: create/drop/alter of databases, retention policies, measurements, continuous queries and materialized views; +- `delete.rs`: `Delete`, `DropSeries`; +- `users.rs`: `CreateUser`, `DropUser`, `SetPassword`, `Grant`, `Revoke`; +- `select.rs`: `Select`. + +**Interfaces:** every arm of 20 lines or more becomes `pub(super) async fn (svc: &QueryServiceImpl, db: &str, , epoch: Option<&str>, query_rp: Option<&str>, statement_id: u32) -> Result`. Drop any parameter the body does not use. Arms under 20 lines stay inline. + +- [ ] **Step 1: Convert the file to a directory module** (`git mv`), then move arms one module at a time. After each module, run `cargo test --lib` and `cargo test --test compat`. +- [ ] **Step 2: Check the move is clean.** Run `git diff --color-moved=zebra -M origin/ -- src/application/query_service*`. Every moved body must show as moved; changed lines may only be the new function signatures, call sites and `use` lines. +- [ ] **Step 3: Check the size target.** `execute_statement` must be under 200 lines (count with `awk`). Run the full gate, then commit with the message "Split execute_statement into statement modules". + +### Task 4: `apply_shard_map_op` becomes a dispatcher + +**Interfaces:** each arm becomes `fn apply_(map: &mut ShardMap, ) -> Result<(), ShardMapApplyError>` in `domain/sharding/ops.rs` (or `ops/apply.rs` if the file passes 1,000 lines). The `map_version` bump stays in `apply_shard_map_op`, before the dispatch. + +- [ ] **Step 1: Write** `refused_op_still_bumps_version` (Review Focus 4). Apply a `Merge` whose `epoch` does not match the left region's (refused with `ShardMapApplyError::StaleEpoch("Merge")`, `ops.rs:~393`) to a map at version 5, assert that `Err`, and assert `map.map_version == 6`. It passes today. +- [ ] **Step 2: Move the 12 arms,** and check with `--color-moved` as in Task 3. +- [ ] **Step 3: Run the gate,** especially `cargo test --lib domain::sharding` and the sharding integration tests, **then commit** with the message "Split apply_shard_map_op per op". diff --git a/hyperbytedb-cli/Cargo.toml b/hyperbytedb-cli/Cargo.toml index 32dbbec..835e240 100644 --- a/hyperbytedb-cli/Cargo.toml +++ b/hyperbytedb-cli/Cargo.toml @@ -19,11 +19,9 @@ serde_urlencoded = "0.7" rustyline = { version = "15", features = ["derive"] } parking_lot = "0.12" comfy-table = "7" -anyhow = "1" thiserror = "2" dirs = "6" rpassword = "7" -url = "2" toml = "0.8" flate2 = "1" chrono = { version = "0.4", default-features = false, features = ["clock"] } diff --git a/hyperbytedb-cli/src/client/query.rs b/hyperbytedb-cli/src/client/query.rs index 7678302..0e67f03 100644 --- a/hyperbytedb-cli/src/client/query.rs +++ b/hyperbytedb-cli/src/client/query.rs @@ -243,10 +243,6 @@ impl QueryResponse { self.results.iter().any(|r| r.error.is_some()) } - pub fn first_error(&self) -> Option<&str> { - self.results.iter().find_map(|r| r.error.as_deref()) - } - pub fn format_errors(&self) -> String { let errors: Vec = self .results diff --git a/hyperbytedb-proxy/Cargo.toml b/hyperbytedb-proxy/Cargo.toml index 97b1c6f..3581a27 100644 --- a/hyperbytedb-proxy/Cargo.toml +++ b/hyperbytedb-proxy/Cargo.toml @@ -20,7 +20,6 @@ harness = false [dependencies] axum = { version = "0.8", features = ["macros"] } tokio = { version = "1", features = ["macros", "rt-multi-thread", "net", "signal", "sync", "time"] } -tower = "0.5" tower-http = { version = "0.6", features = ["trace"] } reqwest = { version = "0.12", default-features = false, features = ["rustls-tls", "stream", "http2"] } @@ -38,7 +37,6 @@ metrics = "0.24" metrics-exporter-prometheus = { version = "0.16", default-features = false } anyhow = "1" -thiserror = "2" futures = "0.3" [dev-dependencies] diff --git a/hyperbytedb/Cargo.toml b/hyperbytedb/Cargo.toml index 670efc5..3f31c69 100644 --- a/hyperbytedb/Cargo.toml +++ b/hyperbytedb/Cargo.toml @@ -8,7 +8,6 @@ foldhash = "0.2" # HTTP axum = { version = "0.8", features = ["macros", "http2"] } axum-server = { version = "0.7", features = ["tls-rustls"] } -tower = "0.5" tower-http = { version = "0.6", features = ["trace", "cors", "set-header", "timeout", "compression-gzip"] } # Async runtime (`full` pulls in unused extras like `process` and `io-std`) @@ -92,7 +91,6 @@ regex = "1" # Crypto / checksums sha2 = "0.10" -crc32fast = "1" # Raft consensus openraft = { version = "0.9", features = ["serde"] } @@ -103,9 +101,6 @@ uuid = { version = "1", features = ["v4"] } futures = "0.3" async-trait = "0.1" indexmap = "2.13.0" -comfy-table = "7.2.2" -colored = "3.1.1" -simd-json = { version = "0.17.0", features = ["serde_impl"] } humantime = "2" lru = "0.12" opentelemetry = { version = "0.33.0", optional = true } diff --git a/hyperbytedb/benches/ingestion_line_protocol.rs b/hyperbytedb/benches/ingestion_line_protocol.rs index 92ea6b5..be8a82c 100644 --- a/hyperbytedb/benches/ingestion_line_protocol.rs +++ b/hyperbytedb/benches/ingestion_line_protocol.rs @@ -10,7 +10,7 @@ use hyperbytedb::adapters::metadata::rocksdb_meta::RocksDbMetadata; use hyperbytedb::adapters::wal::batching_wal::BatchingWal; use hyperbytedb::adapters::wal::rocksdb_wal::RocksDbWal; use hyperbytedb::application::ingest_metadata::{ - IngestCardinalityLimits, IngestSchemaCache, prepare_batch_metadata, + IngestCardinalityLimits, IngestSchemaCache, prepare_batch_metadata, split_field_type_conflicts, }; use hyperbytedb::application::line_protocol::parse_line_body_to_points; use hyperbytedb::domain::wal::WalEntry; @@ -385,8 +385,81 @@ fn bench_wal_batched_concurrent(c: &mut Criterion) { group.finish(); } +/// 5000 Telegraf-shaped `cpu` points with 10 float fields each. +fn telegraf_cpu_body(n: usize) -> Vec { + let mut body = String::new(); + for i in 0..n { + body.push_str(&format!( + "cpu,host=h{},cpu=cpu{} usage_user={}.5,usage_system=1.5,usage_idle=90.1,usage_iowait=0.1,usage_irq=0.0,usage_softirq=0.2,usage_steal=0.0,usage_guest=0.0,usage_nice=0.0,usage_guest_nice=0.0 {}\n", + i % 50, + i % 8, + i, + 1_700_000_000_000_000_000i64 + i as i64 + )); + } + body.into_bytes() +} + +/// The per-batch field-type check every line-protocol write runs before +/// the schema cache. +fn bench_split_field_types(c: &mut Criterion) { + let body = telegraf_cpu_body(5000); + let points = parse_line_body_to_points(&body, None).expect("parse"); + let rt = Runtime::new().expect("runtime"); + let tmpdir = tempfile::tempdir().expect("tmpdir"); + let metadata: Arc = + Arc::new(RocksDbMetadata::open(tmpdir.path()).expect("meta")); + rt.block_on(metadata.create_database("benchdb")).unwrap(); + let mut group = c.benchmark_group("line_protocol_field_types"); + group.throughput(Throughput::Elements(5000)); + group.bench_function("split_field_type_conflicts_5000x10", |b| { + b.iter_batched( + || points.clone(), + |pts| { + rt.block_on(split_field_type_conflicts( + &metadata, "benchdb", "autogen", pts, + )) + .expect("split") + }, + criterion::BatchSize::LargeInput, + ); + }); + group.finish(); +} + +/// The client write path's name-collision guard on a steady-state batch +/// (every name already checked). +fn bench_name_guard(c: &mut Criterion) { + use hyperbytedb::application::physical_names::PhysicalNameGuard; + let body = telegraf_cpu_body(5000); + let points = parse_line_body_to_points(&body, None).expect("parse"); + let rt = Runtime::new().expect("runtime"); + let tmpdir = tempfile::tempdir().expect("tmpdir"); + let metadata: Arc = + Arc::new(RocksDbMetadata::open(tmpdir.path()).expect("meta")); + rt.block_on(metadata.create_database("benchdb")).unwrap(); + let guard = PhysicalNameGuard::new(); + // The request builds the batch schema once for metadata registration; + // the guard's own cost is checking it. + let schema = hyperbytedb::application::ingest_metadata::BatchSchema::from_points(&points) + .expect("schema"); + rt.block_on(guard.check_schema(&metadata, "benchdb", "autogen", &schema)) + .expect("guard"); + let mut group = c.benchmark_group("line_protocol_name_guard"); + group.throughput(Throughput::Elements(5000)); + group.bench_function("name_guard_5000x10", |b| { + b.iter(|| { + rt.block_on(guard.check_schema(&metadata, "benchdb", "autogen", black_box(&schema))) + .expect("guard") + }); + }); + group.finish(); +} + criterion_group!( benches, + bench_split_field_types, + bench_name_guard, bench_parse, bench_metadata, bench_wal, diff --git a/hyperbytedb/src/adapters/chdb/connection_pool.rs b/hyperbytedb/src/adapters/chdb/connection_pool.rs index 972922f..edeecc1 100644 --- a/hyperbytedb/src/adapters/chdb/connection_pool.rs +++ b/hyperbytedb/src/adapters/chdb/connection_pool.rs @@ -15,7 +15,6 @@ use crate::error::HyperbytedbError; pub const MIN_POOL_SIZE: usize = 1; pub const MAX_POOL_SIZE: usize = 128; -pub const DEFAULT_POOL_SIZE: usize = 4; pub const DEFAULT_WRITE_POOL_SIZE: usize = 4; /// Query pool size when nothing sets one: one connection per CPU this diff --git a/hyperbytedb/src/adapters/chdb/native_adapter.rs b/hyperbytedb/src/adapters/chdb/native_adapter.rs index e7e746c..26dcd5f 100644 --- a/hyperbytedb/src/adapters/chdb/native_adapter.rs +++ b/hyperbytedb/src/adapters/chdb/native_adapter.rs @@ -302,12 +302,6 @@ impl ChdbNativeAdapter { } } - /// Override the insert path (Arrow vs SQL `VALUES`). Defaults to Arrow. - pub fn with_arrow_inserts(mut self, enabled: bool) -> Self { - self.use_arrow = enabled; - self - } - /// Re-warm schema caches from metadata (e.g. after peer metadata sync). pub async fn refresh_schemas_from_metadata(&self) -> Result { let warmed = self.warm_schemas_from_metadata().await?; @@ -688,12 +682,7 @@ impl ChdbNativeAdapter { .await? { // Reject raw ingestion into materialized view destinations. - if meas_meta.materialized - && meas_meta - .materialized_rp - .as_deref() - .is_none_or(|mr| mr == key.rp) - { + if meas_meta.refuses_raw_writes(&key.rp) { return Err(HyperbytedbError::WriteToMaterializedDestination( key.measurement.clone(), )); @@ -1293,7 +1282,7 @@ impl ChdbNativeAdapter { db: &str, rp: &str, origin_node_id: u64, - wire: &crate::application::columnar_msgpack::ColumnarMsgpackBatch, + wire: &crate::domain::columnar_batch::ColumnarMsgpackBatch, precision: Option<&str>, ) -> Result { use crate::application::columnar_msgpack::columnar_timestamps_ns; @@ -1533,7 +1522,7 @@ impl PointsSinkPort for ChdbNativeAdapter { db: &str, rp: &str, origin_node_id: u64, - wire: &crate::application::columnar_msgpack::ColumnarMsgpackBatch, + wire: &crate::domain::columnar_batch::ColumnarMsgpackBatch, precision: Option<&str>, ) -> Result { self.build_prepared_wal_slot_from_columnar(db, rp, origin_node_id, wire, precision) @@ -2348,7 +2337,7 @@ fn build_field_column>(points: &[P], logical: &str, disc: u8) - /// (nullable). Returns the batch plus the observed `(min_time, max_time)`. #[cfg(feature = "columnar-ingest")] fn columnar_representative_point( - wire: &crate::application::columnar_msgpack::ColumnarMsgpackBatch, + wire: &crate::domain::columnar_batch::ColumnarMsgpackBatch, precision: Option<&str>, ) -> Result { use crate::application::columnar_msgpack::columnar_timestamps_ns; @@ -2648,19 +2637,7 @@ fn build_series_insert_sql(ensured: &EnsuredTable, new_series: &[(u64, &Point)]) /// Quote a string for ClickHouse: wrap in single quotes; escape `\` and `'`. fn append_quoted_string(out: &mut String, s: &str) { out.push('\''); - for ch in s.chars() { - match ch { - '\\' => out.push_str("\\\\"), - '\'' => out.push_str("\\'"), - '\n' => out.push_str("\\n"), - '\r' => out.push_str("\\r"), - '\t' => out.push_str("\\t"), - c if (c as u32) < 0x20 => { - let _ = write!(out, "\\x{:02x}", c as u32); - } - c => out.push(c), - } - } + crate::domain::chdb_naming::push_string_literal_body(out, s); out.push('\''); } diff --git a/hyperbytedb/src/adapters/chdb/query_adapter.rs b/hyperbytedb/src/adapters/chdb/query_adapter.rs index 52df416..3aecdc7 100644 --- a/hyperbytedb/src/adapters/chdb/query_adapter.rs +++ b/hyperbytedb/src/adapters/chdb/query_adapter.rs @@ -36,17 +36,6 @@ impl ChdbQueryAdapter { }) } - /// Build the adapter, eagerly opening the pool and capping concurrent - /// queries at `max_concurrent` (`0` disables the cap). - pub fn with_concurrency_limit( - data_path: &str, - max_concurrent: usize, - pool_size: usize, - ) -> Result { - let session = SharedSession::new_eager(data_path, pool_size)?; - Ok(Self::from_shared(session, max_concurrent)) - } - /// Build the adapter from an already-constructed [`SharedSession`]. /// Used by [`crate::bootstrap`] so the read adapter and (when enabled) the /// native-write adapter point at the same pool. diff --git a/hyperbytedb/src/adapters/chdb/session.rs b/hyperbytedb/src/adapters/chdb/session.rs index 919337d..80326dd 100644 --- a/hyperbytedb/src/adapters/chdb/session.rs +++ b/hyperbytedb/src/adapters/chdb/session.rs @@ -12,9 +12,7 @@ use chdb_rust::connection::Connection; use chdb_rust::format::OutputFormat; use chdb_rust::query_result::QueryResult; -use crate::adapters::chdb::connection_pool::{ - ChdbConnectionPool, DEFAULT_POOL_SIZE, MIN_POOL_SIZE, clamp_pool_size, -}; +use crate::adapters::chdb::connection_pool::{ChdbConnectionPool, MIN_POOL_SIZE, clamp_pool_size}; use crate::error::HyperbytedbError; /// Execute SQL on a connection with an explicit output format. @@ -70,11 +68,6 @@ impl SharedSession { }) } - /// Default production pool size when not configured otherwise. - pub fn default_pool_size() -> usize { - DEFAULT_POOL_SIZE - } - pub fn data_path(&self) -> &str { &self.data_path } diff --git a/hyperbytedb/src/adapters/cluster/peer_client.rs b/hyperbytedb/src/adapters/cluster/peer_client.rs index 69bdc4c..eea9c13 100644 --- a/hyperbytedb/src/adapters/cluster/peer_client.rs +++ b/hyperbytedb/src/adapters/cluster/peer_client.rs @@ -1091,8 +1091,8 @@ fn coalesce_ok( } // A merged body is one encoding or the other: frames and line protocol // cannot share a body. - if crate::application::line_protocol::is_binary_points(&a.body) - != crate::application::line_protocol::is_binary_points(&b.body) + if crate::domain::point_codec::is_binary_points(&a.body) + != crate::domain::point_codec::is_binary_points(&b.body) { return false; } diff --git a/hyperbytedb/src/adapters/cluster/raft/mod.rs b/hyperbytedb/src/adapters/cluster/raft/mod.rs index 8600a0d..3c88f8e 100644 --- a/hyperbytedb/src/adapters/cluster/raft/mod.rs +++ b/hyperbytedb/src/adapters/cluster/raft/mod.rs @@ -22,4 +22,9 @@ openraft::declare_raft_types!( ); pub type HyperbytedbRaft = openraft::Raft; -pub type RaftAdaptor = openraft::storage::Adaptor; + +impl crate::ports::leadership::LeadershipPort for HyperbytedbRaft { + fn current_leader(&self) -> Option { + self.metrics().borrow().current_leader + } +} diff --git a/hyperbytedb/src/adapters/cluster/raft/network.rs b/hyperbytedb/src/adapters/cluster/raft/network.rs index 059457f..d50ef4c 100644 --- a/hyperbytedb/src/adapters/cluster/raft/network.rs +++ b/hyperbytedb/src/adapters/cluster/raft/network.rs @@ -114,40 +114,18 @@ impl NetworkConnection { if ttl.is_zero() { default } else { ttl } } - async fn post_json( + /// POST `req` as JSON and decode the JSON reply. Every RPC shares it; + /// `E` is the RPC's own Raft error type. + async fn post_json( &self, path: &str, req: &Req, option: &RPCOption, - ) -> Result>> { - let timeout = Self::effective_timeout(option, self.default_timeout); - let resp = self - .send(path, req, timeout) - .await - .map_err(|e| RPCError::Network(NetworkError::new(&e)))?; - - let status = resp.status(); - let body = resp - .bytes() - .await - .map_err(|e| RPCError::Network(NetworkError::new(&e)))?; - - if !status.is_success() { - let msg = String::from_utf8_lossy(&body).to_string(); - return Err(RPCError::Network(NetworkError::new( - &std::io::Error::other(format!("HTTP {}: {}", status, msg)), - ))); - } - - serde_json::from_slice(&body).map_err(|e| RPCError::Network(NetworkError::new(&e))) - } - - async fn post_json_snapshot( - &self, - path: &str, - req: &Req, - option: &RPCOption, - ) -> Result>> + ) -> Result> + where + Req: serde::Serialize, + Resp: serde::de::DeserializeOwned, + E: std::error::Error, { let timeout = Self::effective_timeout(option, self.default_timeout); let resp = self @@ -189,7 +167,7 @@ impl RaftNetwork for NetworkConnection { InstallSnapshotResponse, RPCError>, > { - self.post_json_snapshot("/internal/raft/snapshot", &rpc, &option) + self.post_json("/internal/raft/snapshot", &rpc, &option) .await } diff --git a/hyperbytedb/src/adapters/cluster/raft/state_machine.rs b/hyperbytedb/src/adapters/cluster/raft/state_machine.rs index 42c51cb..351c802 100644 --- a/hyperbytedb/src/adapters/cluster/raft/state_machine.rs +++ b/hyperbytedb/src/adapters/cluster/raft/state_machine.rs @@ -4,7 +4,6 @@ use openraft::{LogId, StoredMembership}; use crate::domain::cluster::membership::ClusterMembership; use crate::domain::cluster::types::MutationRequest; -use crate::domain::database::RetentionPolicy; use crate::ports::metadata::MetadataPort; /// Snapshot-serializable state machine data. @@ -15,21 +14,6 @@ pub struct StateMachineData { pub cluster_membership: ClusterMembership, } -/// Apply CREATE DATABASE locally (idempotent, honors WITH retention policy on peers). -pub async fn apply_create_database( - metadata: &Arc, - name: &str, - rp: Option, -) -> Result<(), crate::error::HyperbytedbError> { - if let Some(rp) = rp { - let stmt = - crate::domain::database::create_database_statement_from_rp(name.to_string(), &rp); - metadata.create_database_with(&stmt).await - } else { - metadata.create_database(name).await - } -} - /// Apply a schema mutation to the local metadata store (metadata-only fallback). pub async fn apply_schema_mutation( metadata: &Arc, diff --git a/hyperbytedb/src/adapters/cluster/raft/types.rs b/hyperbytedb/src/adapters/cluster/raft/types.rs index 154e840..3e7e53d 100644 --- a/hyperbytedb/src/adapters/cluster/raft/types.rs +++ b/hyperbytedb/src/adapters/cluster/raft/types.rs @@ -1,45 +1,4 @@ -use serde::{Deserialize, Serialize}; +//! Raft request types; they live in the domain +//! ([`crate::domain::cluster::raft_request`]). -use crate::domain::cluster::membership::NodeState; -use crate::domain::cluster::types::MutationRequest; -use crate::domain::sharding::ShardMapOp; - -/// Application-level request that goes through Raft consensus. -/// Only cluster coordination and schema DDL use Raft -- high-throughput -/// time-series writes stay on the existing fan-out data plane. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub enum ClusterRequest { - SetNodeState { node_id: u64, state: NodeState }, - SchemaMutation(Box), - ShardMapMutation(Box), -} - -/// Response returned after a ClusterRequest is committed and applied. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ClusterResponse { - pub ok: bool, - pub message: Option, -} - -impl ClusterResponse { - pub fn success() -> Self { - Self { - ok: true, - message: None, - } - } - - pub fn with_message(msg: impl Into) -> Self { - Self { - ok: true, - message: Some(msg.into()), - } - } - - pub fn error(msg: impl Into) -> Self { - Self { - ok: false, - message: Some(msg.into()), - } - } -} +pub use crate::domain::cluster::raft_request::{ClusterRequest, ClusterResponse}; diff --git a/hyperbytedb/src/adapters/cluster/replication_log.rs b/hyperbytedb/src/adapters/cluster/replication_log.rs index 7238572..a44db36 100644 --- a/hyperbytedb/src/adapters/cluster/replication_log.rs +++ b/hyperbytedb/src/adapters/cluster/replication_log.rs @@ -436,6 +436,27 @@ impl ReplicationLog { } } +impl crate::ports::replication_acks::ReplicationAcks for ReplicationLog { + fn get_wal_ack(&self, peer_id: u64) -> Result { + ReplicationLog::get_wal_ack(self, peer_id) + } + fn min_max_wal_ack_for_peers(&self, peer_ids: &[u64]) -> Result<(u64, u64), HyperbytedbError> { + ReplicationLog::min_max_wal_ack_for_peers(self, peer_ids) + } + fn get_mutation_ack(&self, peer_id: u64) -> Result { + ReplicationLog::get_mutation_ack(self, peer_id) + } + fn last_mutation_seq(&self) -> u64 { + ReplicationLog::last_mutation_seq(self) + } + fn min_mutation_ack(&self) -> Result, HyperbytedbError> { + ReplicationLog::min_mutation_ack(self) + } + fn truncate_mutations_before(&self, seq: u64) -> Result<(), HyperbytedbError> { + ReplicationLog::truncate_mutations_before(self, seq) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/hyperbytedb/src/adapters/http/auth_middleware.rs b/hyperbytedb/src/adapters/http/auth_middleware.rs index 2799e64..15c062d 100644 --- a/hyperbytedb/src/adapters/http/auth_middleware.rs +++ b/hyperbytedb/src/adapters/http/auth_middleware.rs @@ -306,17 +306,7 @@ pub async fn rate_limit_query_layer( next.run(request).await } -pub fn hash_password(password: &str) -> Result { - use argon2::Argon2; - use argon2::password_hash::{PasswordHasher, SaltString}; - let salt = SaltString::generate(&mut argon2::password_hash::rand_core::OsRng); - let hash = Argon2::default() - .hash_password(password.as_bytes(), &salt) - .map_err(|e| { - crate::error::HyperbytedbError::Internal(format!("password hash failed: {e}").into()) - })?; - Ok(hash.to_string()) -} +pub use crate::domain::user::hash_password; #[cfg(test)] mod tests { diff --git a/hyperbytedb/src/adapters/http/peer_handlers.rs b/hyperbytedb/src/adapters/http/peer_handlers.rs index d95bf1e..24a9aca 100644 --- a/hyperbytedb/src/adapters/http/peer_handlers.rs +++ b/hyperbytedb/src/adapters/http/peer_handlers.rs @@ -22,7 +22,6 @@ use crate::domain::cluster::sync::{ }; use crate::domain::cluster::types::{MutationReplicateRequest, MutationRequest}; use crate::ports::metadata::MetadataPort; -use crate::ports::sharding::ShardMapPort as _; use super::router::AppState; @@ -263,12 +262,12 @@ fn group_replicate_parts( outcomes[i] = ReplicatePartOutcome::NotARegionPeer; continue; } - let binary = crate::application::line_protocol::is_binary_points(&part.body); + let binary = crate::domain::point_codec::is_binary_points(&part.body); let group = groups.iter_mut().find(|g| { g.database == part.database && g.retention_policy == part.retention_policy && g.precision == part.precision - && crate::application::line_protocol::is_binary_points(&g.body) == binary + && crate::domain::point_codec::is_binary_points(&g.body) == binary }); match group { Some(g) => { diff --git a/hyperbytedb/src/adapters/http/shard_handlers.rs b/hyperbytedb/src/adapters/http/shard_handlers.rs index b6e288c..7fb7a64 100644 --- a/hyperbytedb/src/adapters/http/shard_handlers.rs +++ b/hyperbytedb/src/adapters/http/shard_handlers.rs @@ -28,7 +28,6 @@ use crate::domain::sharding::{ }; use crate::error::HyperbytedbError; use crate::ports::replication::OutboundReplicationBatch; -use crate::ports::sharding::ShardMapPort; use super::router::AppState; diff --git a/hyperbytedb/src/adapters/wal/arrow_cache.rs b/hyperbytedb/src/adapters/wal/arrow_cache.rs index d8666fe..b5cc1f2 100644 --- a/hyperbytedb/src/adapters/wal/arrow_cache.rs +++ b/hyperbytedb/src/adapters/wal/arrow_cache.rs @@ -30,15 +30,6 @@ impl WalArrowCache { gauge!(ENTRIES_GAUGE).set(map.len() as f64); } - pub fn insert_batch(&self, seqs: &[u64], slots: Vec) { - debug_assert_eq!(seqs.len(), slots.len()); - let mut map = self.entries.write(); - for (seq, slot) in seqs.iter().zip(slots) { - map.insert(*seq, slot); - } - gauge!(ENTRIES_GAUGE).set(map.len() as f64); - } - /// Move up to `max_entries` contiguous prepared slots from `from` through /// `to_inclusive`. /// diff --git a/hyperbytedb/src/adapters/wal/batching_wal.rs b/hyperbytedb/src/adapters/wal/batching_wal.rs index 2b14d2a..d2fc732 100644 --- a/hyperbytedb/src/adapters/wal/batching_wal.rs +++ b/hyperbytedb/src/adapters/wal/batching_wal.rs @@ -194,9 +194,9 @@ impl BatchingWal { let pre_encoded = if self.inner.wal_format() == WalFormat::Bincode { let mut bytes = Vec::with_capacity( 64 + bundle.entry.points.len() - * crate::application::line_protocol::POINT_ENCODED_SIZE_HINT, + * crate::domain::point_codec::POINT_ENCODED_SIZE_HINT, ); - crate::application::line_protocol::encode_bincode_into(&mut bytes, &bundle.entry) + crate::domain::point_codec::encode_bincode_into(&mut bytes, &bundle.entry) .map_err(|e| HyperbytedbError::Wal(crate::error::ChainedError::from_error(e)))?; Some(bytes) } else { diff --git a/hyperbytedb/src/application/arrow_ingest/columnar.rs b/hyperbytedb/src/application/arrow_ingest/columnar.rs deleted file mode 100644 index 2a8feeb..0000000 --- a/hyperbytedb/src/application/arrow_ingest/columnar.rs +++ /dev/null @@ -1,33 +0,0 @@ -use crate::adapters::chdb::native_adapter::ChdbNativeAdapter; -use crate::application::columnar_msgpack::ColumnarMsgpackBatch; -use crate::application::columnar_msgpack::columnar_batch_to_record_batch; -use crate::domain::wal::WalEntry; -use crate::error::HyperbytedbError; -use crate::ports::wal::WalAppendBundle; - -pub async fn columnar_to_prepared_slot( - sink: &ChdbNativeAdapter, - db: &str, - rp: &str, - wire: &ColumnarMsgpackBatch, - precision: Option<&str>, - origin_node_id: u64, -) -> Result { - // Validate the wire batch can be converted to Arrow before building the - // chDB fact-table prepared slot (shared timestamp/field semantics). - let _ = columnar_batch_to_record_batch(wire, precision)?; - - let prepared = sink - .build_prepared_wal_slot_from_columnar(db, rp, origin_node_id, wire, precision) - .await?; - let entry = WalEntry { - database: db.to_string(), - retention_policy: rp.to_string(), - points: Vec::new(), - origin_node_id, - }; - Ok(WalAppendBundle { - entry, - prepared: Some(prepared), - }) -} diff --git a/hyperbytedb/src/application/arrow_ingest/line_protocol.rs b/hyperbytedb/src/application/arrow_ingest/line_protocol.rs deleted file mode 100644 index 8d90d9d..0000000 --- a/hyperbytedb/src/application/arrow_ingest/line_protocol.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::adapters::chdb::native_adapter::ChdbNativeAdapter; -use crate::application::arrow_ingest::points::points_to_prepared_bundle; -use crate::application::line_protocol::parse_line_body_to_points; -use crate::error::HyperbytedbError; -use crate::ports::wal::WalAppendBundle; - -pub async fn line_body_to_prepared_slot( - sink: &ChdbNativeAdapter, - db: &str, - rp: &str, - body: &[u8], - precision: Option<&str>, - origin_node_id: u64, -) -> Result { - let points = parse_line_body_to_points(body, precision)?; - points_to_prepared_bundle(sink, db, rp, origin_node_id, points).await -} diff --git a/hyperbytedb/src/application/arrow_ingest/mod.rs b/hyperbytedb/src/application/arrow_ingest/mod.rs deleted file mode 100644 index 0415990..0000000 --- a/hyperbytedb/src/application/arrow_ingest/mod.rs +++ /dev/null @@ -1,11 +0,0 @@ -//! Build prepared WAL slots at ingest time. - -mod columnar; -mod line_protocol; -mod msgpack; -mod points; - -pub use columnar::columnar_to_prepared_slot; -pub use line_protocol::line_body_to_prepared_slot; -pub use msgpack::msgpack_body_to_prepared_slot; -pub use points::points_to_prepared_slot; diff --git a/hyperbytedb/src/application/arrow_ingest/msgpack.rs b/hyperbytedb/src/application/arrow_ingest/msgpack.rs deleted file mode 100644 index f624401..0000000 --- a/hyperbytedb/src/application/arrow_ingest/msgpack.rs +++ /dev/null @@ -1,17 +0,0 @@ -use crate::adapters::chdb::native_adapter::ChdbNativeAdapter; -use crate::application::arrow_ingest::points::points_to_prepared_bundle; -use crate::application::msgpack_ingest::parse_msgpack_body_to_points; -use crate::error::HyperbytedbError; -use crate::ports::wal::WalAppendBundle; - -pub async fn msgpack_body_to_prepared_slot( - sink: &ChdbNativeAdapter, - db: &str, - rp: &str, - body: &[u8], - precision: Option<&str>, - origin_node_id: u64, -) -> Result { - let points = parse_msgpack_body_to_points(body, precision)?; - points_to_prepared_bundle(sink, db, rp, origin_node_id, points).await -} diff --git a/hyperbytedb/src/application/arrow_ingest/points.rs b/hyperbytedb/src/application/arrow_ingest/points.rs deleted file mode 100644 index ba4e7bf..0000000 --- a/hyperbytedb/src/application/arrow_ingest/points.rs +++ /dev/null @@ -1,39 +0,0 @@ -use crate::adapters::chdb::native_adapter::ChdbNativeAdapter; -use crate::domain::point::Point; -use crate::domain::prepared_wal::PreparedWalSlot; -use crate::domain::wal::WalEntry; -use crate::error::HyperbytedbError; -use crate::ports::wal::WalAppendBundle; - -pub async fn points_to_prepared_bundle( - sink: &ChdbNativeAdapter, - db: &str, - rp: &str, - origin_node_id: u64, - points: Vec, -) -> Result { - let entry = WalEntry { - database: db.to_string(), - retention_policy: rp.to_string(), - points: points.clone(), - origin_node_id, - }; - let prepared = sink - .build_prepared_wal_slot(db, rp, origin_node_id, &points) - .await?; - Ok(WalAppendBundle { - entry, - prepared: Some(prepared), - }) -} - -pub async fn points_to_prepared_slot( - sink: &ChdbNativeAdapter, - db: &str, - rp: &str, - origin_node_id: u64, - points: &[Point], -) -> Result { - sink.build_prepared_wal_slot(db, rp, origin_node_id, points) - .await -} diff --git a/hyperbytedb/src/application/cluster/drain.rs b/hyperbytedb/src/application/cluster/drain.rs index fdb600a..f3f9375 100644 --- a/hyperbytedb/src/application/cluster/drain.rs +++ b/hyperbytedb/src/application/cluster/drain.rs @@ -5,7 +5,6 @@ use std::time::Duration; use crate::adapters::cluster::peer_client::PeerClient; use crate::adapters::cluster::raft::HyperbytedbRaft; -use crate::adapters::cluster::replication_log::ReplicationLog; use crate::application::shard_routing::ShardRoutingContext; use crate::application::shard_transfer::run_region_transfer; use crate::domain::cluster::membership::{NodeState, SharedMembership}; @@ -15,7 +14,7 @@ use crate::ports::flush::FlushPort; use crate::ports::metadata::MetadataPort; use crate::ports::points_sink::PointsSinkPort; use crate::ports::query::QueryPort; -use crate::ports::sharding::ShardMapPort; +use crate::ports::replication_acks::ReplicationAcks; use crate::ports::wal::WalPort; /// Runs the two node-lifecycle procedures that hand off this node's primary @@ -27,7 +26,7 @@ pub struct DrainService { node_id: u64, membership: SharedMembership, flush_service: Arc, - replication_log: Arc, + replication_log: Arc, wal: Arc, shard_routing: Option>, peer_client: Option>, @@ -51,7 +50,7 @@ impl DrainService { node_id: u64, membership: SharedMembership, flush_service: Arc, - replication_log: Arc, + replication_log: Arc, wal: Arc, ) -> Self { Self { @@ -242,7 +241,7 @@ impl DrainService { .await?; } - use crate::adapters::cluster::raft::types::ClusterRequest; + use crate::domain::cluster::raft_request::ClusterRequest; let tp = ShardMapOp::TransferPrimary { key: key.clone(), region_id: region.region_id, @@ -433,8 +432,10 @@ mod tests { } let wal = RocksDbWal::open(dir.path().join("wal")).expect("open test wal"); - let replication_log = - ReplicationLog::open(dir.path().join("repl")).expect("open test replication log"); + let replication_log = crate::adapters::cluster::replication_log::ReplicationLog::open( + dir.path().join("repl"), + ) + .expect("open test replication log"); DrainService::new( 1, diff --git a/hyperbytedb/src/application/cluster/mod.rs b/hyperbytedb/src/application/cluster/mod.rs index ab713bc..a132a4b 100644 --- a/hyperbytedb/src/application/cluster/mod.rs +++ b/hyperbytedb/src/application/cluster/mod.rs @@ -1,7 +1,4 @@ -pub mod bootstrap; pub mod drain; pub mod heartbeat; pub mod hinted_handoff; -pub mod leader_monitor; -pub mod raft_formation; pub mod sync_manifest; diff --git a/hyperbytedb/src/application/columnar_msgpack.rs b/hyperbytedb/src/application/columnar_msgpack.rs index c9c7473..804f1db 100644 --- a/hyperbytedb/src/application/columnar_msgpack.rs +++ b/hyperbytedb/src/application/columnar_msgpack.rs @@ -2,7 +2,6 @@ //! //! See [docs/benchmarks.md § Columnar MessagePack](../../../../docs/benchmarks.md#columnar-messagepack-write-format-v1). -use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use std::fmt::Write as _; @@ -16,16 +15,7 @@ use crate::error::HyperbytedbError; /// `Content-Type` for columnar msgpack v1 writes. pub const CONTENT_TYPE: &str = "application/vnd.hyperbytedb.columnar-msgpack.v1"; -/// Wire map for columnar msgpack v1 (MessagePack map, not a row array). -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ColumnarMsgpackBatch { - pub measurement: String, - #[serde(default)] - pub tags: BTreeMap, - pub field: String, - pub values: Vec, - pub timestamps: Option>, -} +pub use crate::domain::columnar_batch::ColumnarMsgpackBatch; /// Decode the wire format from raw bytes. pub fn decode_columnar_batch(body: &[u8]) -> Result { diff --git a/hyperbytedb/src/application/continuous_query_service.rs b/hyperbytedb/src/application/continuous_query_service.rs index 3688bd3..1d66100 100644 --- a/hyperbytedb/src/application/continuous_query_service.rs +++ b/hyperbytedb/src/application/continuous_query_service.rs @@ -2,16 +2,16 @@ use metrics::counter; use std::sync::Arc; use tokio::sync::watch; -use crate::adapters::cluster::raft::HyperbytedbRaft; use crate::domain::cq_schedule::should_run; use crate::error::HyperbytedbError; +use crate::ports::leadership::LeadershipPort; use crate::ports::metadata::MetadataPort; use crate::ports::query::QueryService; pub struct ContinuousQueryService { metadata: Arc, query_service: Arc, - raft: Option, + leadership: Option>, node_id: u64, } @@ -19,25 +19,19 @@ impl ContinuousQueryService { pub fn new( metadata: Arc, query_service: Arc, - raft: Option, + leadership: Option>, node_id: u64, ) -> Self { Self { metadata, query_service, - raft, + leadership, node_id, } } fn is_raft_leader(&self) -> bool { - match &self.raft { - Some(raft) => { - let metrics = raft.metrics().borrow().clone(); - metrics.current_leader == Some(self.node_id) - } - None => true, - } + crate::ports::leadership::may_run(self.leadership.as_deref(), self.node_id) } pub async fn run( @@ -50,7 +44,7 @@ impl ContinuousQueryService { tracing::info!( check_interval = ?check_interval, - raft_gated = self.raft.is_some(), + raft_gated = self.leadership.is_some(), node_id = self.node_id, "continuous query service started" ); diff --git a/hyperbytedb/src/application/flush_service.rs b/hyperbytedb/src/application/flush_service.rs index fba1214..846be02 100644 --- a/hyperbytedb/src/application/flush_service.rs +++ b/hyperbytedb/src/application/flush_service.rs @@ -4,7 +4,7 @@ use std::sync::Arc; use tokio::sync::Mutex; use tokio::sync::watch; -use crate::adapters::cluster::replication_log::ReplicationLog; +use crate::ports::replication_acks::ReplicationAcks; use crate::domain::arrow_coalesce::coalesce_prepared_batches; use crate::domain::cluster::membership::SharedMembership; @@ -48,7 +48,7 @@ pub struct FlushServiceImpl { wal: Arc, last_flushed: Arc>, max_points_per_batch: usize, - replication_log: Option>, + replication_log: Option>, membership: Option, node_id: u64, /// Highest WAL sequence with `origin_node_id == self.node_id` (i.e. a @@ -151,7 +151,7 @@ impl FlushServiceImpl { self } - pub fn with_replication_log(mut self, repl_log: Arc) -> Self { + pub fn with_replication_log(mut self, repl_log: Arc) -> Self { self.replication_log = Some(repl_log); self } diff --git a/hyperbytedb/src/application/ingest_metadata.rs b/hyperbytedb/src/application/ingest_metadata.rs index cdbc91a..04fcc7f 100644 --- a/hyperbytedb/src/application/ingest_metadata.rs +++ b/hyperbytedb/src/application/ingest_metadata.rs @@ -95,6 +95,43 @@ pub fn validate_point_count(count: usize, max_points: usize) -> Result<(), Hyper Ok(()) } +/// Parse a `/write` body into points. Line protocol is limited inside its +/// parser; the other formats are counted here against `max_points`. +/// +/// # Errors +/// A parse error, `RequestPointLimitExceeded`, or `Internal` for a +/// columnar batch (which has its own path). +pub fn parse_write_body( + body: &[u8], + precision: Option<&str>, + format: crate::ports::ingestion::WritePayloadFormat, + max_points: usize, +) -> Result, HyperbytedbError> { + use crate::ports::ingestion::WritePayloadFormat; + let points = match format { + WritePayloadFormat::LineProtocol => { + crate::application::line_protocol::parse_line_body_to_points_limited( + body, precision, max_points, + )? + } + WritePayloadFormat::Msgpack => { + crate::application::msgpack_ingest::parse_msgpack_body_to_points_limited( + body, precision, max_points, + )? + } + #[cfg(feature = "columnar-ingest")] + WritePayloadFormat::ColumnarMsgpack => { + return Err(HyperbytedbError::Internal( + "columnar ingest fell through fast path".into(), + )); + } + }; + if !matches!(format, WritePayloadFormat::LineProtocol) { + validate_point_count(points.len(), max_points)?; + } + Ok(points) +} + /// Maximum number of cached tag-value hashes. Beyond this the set is /// evicted to prevent unbounded memory growth under high cardinality. /// Cache misses are harmless (fall back to a RocksDB lookup). @@ -696,13 +733,20 @@ pub async fn split_field_type_conflicts( first.get_or_insert(e); continue; } + // Update in place: cloning every field key of every point (to + // re-insert it) cost ~4x this loop on a Telegraf-shaped batch. for (k, v) in &point.fields { let disc = v.type_discriminant(); - let widened = known - .get(k) - .and_then(|have| crate::domain::field_type::widen_field_disc(*have, disc)) - .unwrap_or(disc); - known.insert(k.clone(), widened); + match known.get_mut(k) { + Some(have) => { + if let Some(w) = crate::domain::field_type::widen_field_disc(*have, disc) { + *have = w; + } + } + None => { + known.insert(k.clone(), disc); + } + } } kept.push(point); } @@ -717,6 +761,65 @@ pub async fn split_field_type_conflicts( Ok((kept, partial)) } +/// The names and field types one batch writes, per measurement: field +/// types (widened across the batch) and tag keys. Built once per request +/// and shared by the name guard and [`prepare_batch_metadata_with_schema`]. +#[derive(Debug, Default)] +pub struct BatchSchema { + pub measurements: HashMap, BTreeSet)>, +} + +impl BatchSchema { + /// # Errors + /// `FieldTypeConflict` for a field given two types the batch cannot + /// hold together. + pub fn from_points(points: &[Point]) -> Result { + let mut measurements: HashMap, BTreeSet)> = + HashMap::new(); + for point in points { + // Clone a name only the first time it is seen: this loop runs per + // point, and cloning the measurement and every tag key on every + // point was most of prepare_batch_metadata's own time. + if !measurements.contains_key(&point.measurement) { + measurements.insert(point.measurement.clone(), (HashMap::new(), BTreeSet::new())); + } + let Some(entry) = measurements.get_mut(&point.measurement) else { + continue; + }; + for (k, v) in &point.fields { + let disc = v.type_discriminant(); + match entry.0.get_mut(k) { + Some(existing) => { + // Two types the batch cannot hold together: refused, as a + // conflict with the stored type is. Keeping the first + // type stored the other values as null. + let Some(w) = crate::domain::field_type::widen_field_disc(*existing, disc) + else { + return Err(HyperbytedbError::FieldTypeConflict { + field: k.clone(), + measurement: point.measurement.clone(), + got: FieldValue::type_name_from_discriminant(disc).to_string(), + expected: FieldValue::type_name_from_discriminant(*existing) + .to_string(), + }); + }; + *existing = w; + } + None => { + entry.0.insert(k.clone(), disc); + } + } + } + for k in point.tags.keys() { + if !entry.1.contains(k) { + entry.1.insert(k.clone()); + } + } + } + Ok(Self { measurements }) + } +} + pub async fn prepare_batch_metadata( metadata: &Arc, db: &str, @@ -724,6 +827,32 @@ pub async fn prepare_batch_metadata( points: &[Point], limits: &IngestCardinalityLimits, schema_cache: Option<&IngestSchemaCache>, +) -> Result<(), HyperbytedbError> { + prepare_batch_metadata_inner(metadata, db, rp, points, None, limits, schema_cache).await +} + +/// [`prepare_batch_metadata`] with the batch's schema already built (the +/// client write paths build it once for the name guard too). +pub async fn prepare_batch_metadata_with_schema( + metadata: &Arc, + db: &str, + rp: &str, + points: &[Point], + schema: &BatchSchema, + limits: &IngestCardinalityLimits, + schema_cache: Option<&IngestSchemaCache>, +) -> Result<(), HyperbytedbError> { + prepare_batch_metadata_inner(metadata, db, rp, points, Some(schema), limits, schema_cache).await +} + +async fn prepare_batch_metadata_inner( + metadata: &Arc, + db: &str, + rp: &str, + points: &[Point], + schema: Option<&BatchSchema>, + limits: &IngestCardinalityLimits, + schema_cache: Option<&IngestSchemaCache>, ) -> Result<(), HyperbytedbError> { // Series registration short-circuit: steady-state batches whose series are // all already known skip the RocksDB registration round-trip entirely. @@ -750,48 +879,15 @@ pub async fn prepare_batch_metadata( None => register_series_from_points(metadata, db, rp, points).await?, } - let mut measurements: HashMap, BTreeSet)> = HashMap::new(); - - for point in points { - // Clone a name only the first time it is seen: this loop runs per - // point, and cloning the measurement and every tag key on every - // point was most of prepare_batch_metadata's own time. - if !measurements.contains_key(&point.measurement) { - measurements.insert(point.measurement.clone(), (HashMap::new(), BTreeSet::new())); - } - let Some(entry) = measurements.get_mut(&point.measurement) else { - continue; - }; - for (k, v) in &point.fields { - let disc = v.type_discriminant(); - match entry.0.get_mut(k) { - Some(existing) => { - // Two types the batch cannot hold together: refused, as a - // conflict with the stored type is. Keeping the first - // type stored the other values as null. - let Some(w) = crate::domain::field_type::widen_field_disc(*existing, disc) - else { - return Err(HyperbytedbError::FieldTypeConflict { - field: k.clone(), - measurement: point.measurement.clone(), - got: FieldValue::type_name_from_discriminant(disc).to_string(), - expected: FieldValue::type_name_from_discriminant(*existing) - .to_string(), - }); - }; - *existing = w; - } - None => { - entry.0.insert(k.clone(), disc); - } - } + let built; + let schema = match schema { + Some(schema) => schema, + None => { + built = BatchSchema::from_points(points)?; + &built } - for k in point.tags.keys() { - if !entry.1.contains(k) { - entry.1.insert(k.clone()); - } - } - } + }; + let measurements = &schema.measurements; if let Some(sc) = schema_cache { let all_schemas_known = measurements @@ -876,7 +972,7 @@ pub async fn prepare_batch_metadata( } } - for (meas_name, (field_types, tag_keys)) in &measurements { + for (meas_name, (field_types, tag_keys)) in measurements { if limits.max_tag_values_per_measurement > 0 { for tag_key in tag_keys.iter() { let count = metadata @@ -920,7 +1016,7 @@ pub async fn prepare_batch_metadata( let mut all_metas: Vec = Vec::with_capacity(measurements.len()); let mut all_tags: Vec<(String, Vec<(String, String)>)> = Vec::with_capacity(measurements.len()); - for (meas_name, (field_types, _tag_keys)) in &measurements { + for (meas_name, (field_types, _tag_keys)) in measurements { let merged_fields = merged_field_types(metadata, db, rp, meas_name, field_types).await?; let merged = merged_tag_keys(metadata, db, rp, meas_name, &measurements[meas_name].1).await?; @@ -950,7 +1046,7 @@ pub async fn prepare_batch_metadata( .await?; if let Some(sc) = schema_cache { - for (name, (fields, tags)) in &measurements { + for (name, (fields, tags)) in measurements { let merged_fields = merged_field_types(metadata, db, rp, name, fields).await?; let merged = merged_tag_keys(metadata, db, rp, name, tags).await?; sc.mark_schema(db, rp, name, &merged_fields, &merged); @@ -991,6 +1087,96 @@ mod tests { } } + #[test] + fn parse_write_body_refuses_bodies_over_the_limit() { + use crate::ports::ingestion::WritePayloadFormat; + // Line protocol is limited inside its parser; msgpack is counted + // after parsing. Both refuse a body over the limit. + let lp = b"cpu v=1 1\ncpu v=2 2"; + assert_eq!( + parse_write_body(lp, None, WritePayloadFormat::LineProtocol, 2) + .unwrap() + .len(), + 2 + ); + assert!(matches!( + parse_write_body(lp, None, WritePayloadFormat::LineProtocol, 1), + Err(HyperbytedbError::RequestPointLimitExceeded { .. }) + )); + let mp = rmp_serde::to_vec_named(&vec![ + point("cpu", &[("v", FieldValue::Float(1.0))], 1), + point("cpu", &[("v", FieldValue::Float(2.0))], 2), + ]) + .unwrap(); + assert!(matches!( + parse_write_body(&mp, None, WritePayloadFormat::Msgpack, 1), + Err(HyperbytedbError::RequestPointLimitExceeded { .. }) + )); + #[cfg(feature = "columnar-ingest")] + assert!(matches!( + parse_write_body(b"", None, WritePayloadFormat::ColumnarMsgpack, 1), + Err(HyperbytedbError::Internal(_)) + )); + } + + #[test] + fn batch_schema_matches_prepare_batch_metadata() { + let mut p = point("m", &[("f", FieldValue::Integer(1))], 1); + p.tags.insert("dc".into(), "x".into()); + let points = vec![ + p, + point("m", &[("f", FieldValue::Float(2.0))], 2), + point("n", &[("g", FieldValue::Boolean(true))], 3), + ]; + let schema = BatchSchema::from_points(&points).unwrap(); + let (m_fields, m_tags) = &schema.measurements["m"]; + assert_eq!(m_fields["f"], FieldValue::Float(0.0).type_discriminant()); + assert_eq!( + m_tags.iter().cloned().collect::>(), + vec!["dc".to_string(), "host".to_string()] + ); + let (n_fields, _) = &schema.measurements["n"]; + assert_eq!(n_fields["g"], FieldValue::Boolean(true).type_discriminant()); + // Two types the batch cannot hold together are refused, as before. + let clash = vec![ + point("m", &[("f", FieldValue::Float(1.0))], 1), + point("m", &[("f", FieldValue::String("x".into()))], 2), + ]; + assert!(matches!( + BatchSchema::from_points(&clash), + Err(HyperbytedbError::FieldTypeConflict { .. }) + )); + } + + /// A field that widens (integer, then float) and then conflicts (a + /// string) within one batch: the widening point is kept and the + /// conflicting one dropped, before any type was stored. + #[tokio::test] + async fn field_widens_then_conflicts_within_one_batch() { + let dir = tempfile::tempdir().unwrap(); + let meta: Arc = Arc::new(RocksDbMetadata::open(dir.path()).unwrap()); + meta.create_database("db").await.unwrap(); + let points = vec![ + point("m", &[("f", FieldValue::Integer(1))], 1), + point("m", &[("f", FieldValue::Float(2.0))], 2), + point("m", &[("f", FieldValue::String("x".into()))], 3), + // Unsigned widens from integer but not from float: dropped only + // if the widening to float above was kept. + point("m", &[("f", FieldValue::UInteger(4))], 4), + ]; + let (kept, partial) = split_field_type_conflicts(&meta, "db", "autogen", points) + .await + .unwrap(); + assert_eq!( + kept.iter().map(|p| p.timestamp).collect::>(), + vec![1, 2] + ); + assert!(matches!( + partial, + Some(HyperbytedbError::PartialWrite { dropped: 2, .. }) + )); + } + /// Conflicting points are split off; compatible types widen; within a /// batch the first type of a new field wins, as in InfluxDB. #[tokio::test] diff --git a/hyperbytedb/src/application/ingestion_service.rs b/hyperbytedb/src/application/ingestion_service.rs index 103b0e5..a5107a1 100644 --- a/hyperbytedb/src/application/ingestion_service.rs +++ b/hyperbytedb/src/application/ingestion_service.rs @@ -5,10 +5,8 @@ use std::sync::Arc; #[cfg(feature = "columnar-ingest")] use crate::application::ingest_metadata::prepare_columnar_metadata; use crate::application::ingest_metadata::{ - IngestCardinalityLimits, IngestSchemaCache, prepare_batch_metadata, validate_point_count, + IngestCardinalityLimits, IngestSchemaCache, validate_point_count, }; -use crate::application::line_protocol::parse_line_body_to_points_limited; -use crate::application::msgpack_ingest::parse_msgpack_body_to_points_limited; use crate::application::physical_names::PhysicalNameGuard; use crate::application::wal_append::{ ColumnarWalAppend, append_columnar_with_prepared, append_points_with_prepared, @@ -149,11 +147,7 @@ impl IngestionPort for IngestionServiceImpl { .metadata .get_measurement(db, &retention_policy, &wire.measurement) .await? - && meta.materialized - && meta - .materialized_rp - .as_deref() - .is_none_or(|mr| mr == retention_policy) + && meta.refuses_raw_writes(&retention_policy) { return Err(HyperbytedbError::WriteToMaterializedDestination( wire.measurement.clone(), @@ -189,26 +183,15 @@ impl IngestionPort for IngestionServiceImpl { return Ok(()); } - let points = match format { - WritePayloadFormat::LineProtocol => { - parse_line_body_to_points_limited(body, precision, self.max_points_per_request)? - } - WritePayloadFormat::Msgpack => { - parse_msgpack_body_to_points_limited(body, precision, self.max_points_per_request)? - } - #[cfg(feature = "columnar-ingest")] - WritePayloadFormat::ColumnarMsgpack => { - return Err(HyperbytedbError::Internal( - "columnar ingest fell through fast path".into(), - )); - } - }; + let points = crate::application::ingest_metadata::parse_write_body( + body, + precision, + format, + self.max_points_per_request, + )?; if points.is_empty() { return Ok(()); } - if !matches!(format, WritePayloadFormat::LineProtocol) { - validate_point_count(points.len(), self.max_points_per_request)?; - } // Points whose fields conflict with a stored type are dropped and the // rest written, as InfluxDB does; the error says how many. @@ -226,14 +209,18 @@ impl IngestionPort for IngestionServiceImpl { let t2 = std::time::Instant::now(); histogram!("hyperbytedb_ingest_parse_seconds").record((t2 - t1).as_secs_f64()); + // Built once: the name guard and the metadata registration both + // need each measurement's names. + let schema = crate::application::ingest_metadata::BatchSchema::from_points(&points)?; self.name_guard - .check_points(&self.metadata, db, &retention_policy, &points) + .check_schema(&self.metadata, db, &retention_policy, &schema) .await?; - prepare_batch_metadata( + crate::application::ingest_metadata::prepare_batch_metadata_with_schema( &self.metadata, db, &retention_policy, &points, + &schema, &self.limits, Some(&self.schema_cache), ) @@ -248,11 +235,7 @@ impl IngestionPort for IngestionServiceImpl { .metadata .get_measurement(db, &retention_policy, &point.measurement) .await? - && meta.materialized - && meta - .materialized_rp - .as_deref() - .is_none_or(|mr| mr == retention_policy) + && meta.refuses_raw_writes(&retention_policy) { return Err(HyperbytedbError::WriteToMaterializedDestination( point.measurement.clone(), diff --git a/hyperbytedb/src/application/line_protocol.rs b/hyperbytedb/src/application/line_protocol.rs index fa533ae..a064592 100644 --- a/hyperbytedb/src/application/line_protocol.rs +++ b/hyperbytedb/src/application/line_protocol.rs @@ -4,6 +4,9 @@ use influxdb_line_protocol::{FieldValue as LpFieldValue, parse_lines}; use crate::domain::database::Precision; use crate::domain::point::{FieldValue, Point}; +pub use crate::domain::point_codec::{ + POINT_ENCODED_SIZE_HINT, POINTS_FRAME_MAGIC, encode_bincode_into, is_binary_points, +}; use crate::error::HyperbytedbError; const PARSE_ERROR_SNIPPET_LEN: usize = 200; @@ -43,16 +46,6 @@ pub fn parse_line_body_to_points_limited( parse_line_body_inner(body, precision, cap, Some(limit)) } -/// Start of a binary points frame. Line protocol never starts with NUL, so a -/// body is binary exactly when it starts with this. -pub const POINTS_FRAME_MAGIC: &[u8; 4] = b"\0HBP"; - -/// Whether `body` is binary points frames rather than line protocol. -#[must_use] -pub fn is_binary_points(body: &[u8]) -> bool { - body.starts_with(POINTS_FRAME_MAGIC) -} - /// Encode points for node-to-node forwarding and replication: one frame, /// `magic | u32 LE length | bincode(Vec)`. /// @@ -77,19 +70,6 @@ pub fn encode_points_binary(points: &[Point]) -> Result, HyperbytedbErro Ok(out) } -/// Starting buffer size per point for one-pass bincode encoding: a point with -/// a few short tags and one field. Only a hint; the buffer grows as needed. -pub const POINT_ENCODED_SIZE_HINT: usize = 96; - -/// Append `value`'s bincode encoding to `out` in one pass. Same bytes as -/// `bincode::serialize`, without its separate sizing pass. -pub fn encode_bincode_into( - out: &mut Vec, - value: &T, -) -> Result<(), bincode::Error> { - bincode::serialize_into(out, value) -} - /// Decode one or more frames. Whitespace between frames is skipped, so the /// newline replication coalescing inserts between bodies is harmless. fn decode_points_frames(body: &[u8], limit: usize) -> Result, HyperbytedbError> { @@ -153,15 +133,6 @@ fn estimate_points_capacity(body: &[u8]) -> usize { count.max(1) } -/// Like [`parse_line_body_to_points`] with an explicit capacity hint for the points vector. -pub fn parse_line_body_to_points_with_capacity( - body: &[u8], - precision: Option<&str>, - points_capacity: usize, -) -> Result, HyperbytedbError> { - parse_line_body_inner(body, precision, points_capacity, None) -} - fn parse_line_body_inner( body: &[u8], precision: Option<&str>, diff --git a/hyperbytedb/src/application/materialized_view_service.rs b/hyperbytedb/src/application/materialized_view_service.rs index e06bbf5..9e828c2 100644 --- a/hyperbytedb/src/application/materialized_view_service.rs +++ b/hyperbytedb/src/application/materialized_view_service.rs @@ -469,17 +469,6 @@ impl MaterializedViewService { } } - /// Drop ClickHouse MV objects for a definition without touching metadata. - pub async fn drop_ch_objects_for_def( - &self, - def: &MaterializedViewDef, - ) -> Result<(), HyperbytedbError> { - let fact_mv = quoted_fact_mv_name(&def.database, &def.dest_rp, &def.name); - let series_mv = quoted_series_mv_name(&def.database, &def.dest_rp, &def.name); - self.drop_ch_mv_objects(fact_mv.as_str(), series_mv.as_str()) - .await - } - async fn reconcile_one(&self, def: &MaterializedViewDef) -> Result { let exists_sql = format!( "SELECT count() FROM system.tables WHERE database = 'default' AND name = '{}' FORMAT TabSeparated", diff --git a/hyperbytedb/src/application/mod.rs b/hyperbytedb/src/application/mod.rs index 4bf7f34..9e7eadd 100644 --- a/hyperbytedb/src/application/mod.rs +++ b/hyperbytedb/src/application/mod.rs @@ -1,4 +1,3 @@ -pub mod arrow_ingest; pub mod backup; pub mod cluster; pub mod columnar_msgpack; diff --git a/hyperbytedb/src/application/peer_ingestion_service.rs b/hyperbytedb/src/application/peer_ingestion_service.rs index f01c5f6..6ceb0ba 100644 --- a/hyperbytedb/src/application/peer_ingestion_service.rs +++ b/hyperbytedb/src/application/peer_ingestion_service.rs @@ -11,10 +11,7 @@ use crate::application::ingest_metadata::{ IngestCardinalityLimits, IngestSchemaCache, prepare_batch_metadata, split_field_type_conflicts, validate_point_count, }; -use crate::application::line_protocol::{ - encode_points_to_line_protocol, parse_line_body_to_points_limited, -}; -use crate::application::msgpack_ingest::parse_msgpack_body_to_points_limited; +use crate::application::line_protocol::encode_points_to_line_protocol; use crate::application::replication_dispatch::dispatch_outbound_replication; use crate::application::shard_routing::{ ShardRoutingContext, ensure_measurement_bootstrapped, forward_shard_writes_by_node, @@ -29,7 +26,6 @@ use crate::error::HyperbytedbError; use crate::ports::ingestion::{IngestionPort, WritePayloadFormat}; use crate::ports::points_sink::PointsSinkPort; use crate::ports::replication::{OutboundReplicationBatch, ReplicationPort}; -use crate::ports::sharding::ShardMapPort; use crate::ports::wal::WalPort; /// Ingestion service for clustered (master-master) mode. Writes are always @@ -225,8 +221,7 @@ impl PeerIngestionService { // flush pipeline cannot serve (stuck-flush liveness gaps). for meas in &measurements { if let Some(meta) = self.metadata.get_measurement(db, rp, meas).await? - && meta.materialized - && meta.materialized_rp.as_deref().is_none_or(|mr| mr == rp) + && meta.refuses_raw_writes(rp) { return Err(HyperbytedbError::WriteToMaterializedDestination( meas.clone(), @@ -893,11 +888,7 @@ impl IngestionPort for PeerIngestionService { .metadata .get_measurement(db, &retention_policy, &wire.measurement) .await? - && meta.materialized - && meta - .materialized_rp - .as_deref() - .is_none_or(|mr| mr == retention_policy) + && meta.refuses_raw_writes(&retention_policy) { return Err(HyperbytedbError::WriteToMaterializedDestination( wire.measurement.clone(), @@ -965,26 +956,15 @@ impl IngestionPort for PeerIngestionService { return result; } - let points = match format { - WritePayloadFormat::LineProtocol => { - parse_line_body_to_points_limited(body, precision, self.max_points_per_request)? - } - WritePayloadFormat::Msgpack => { - parse_msgpack_body_to_points_limited(body, precision, self.max_points_per_request)? - } - #[cfg(feature = "columnar-ingest")] - WritePayloadFormat::ColumnarMsgpack => { - return Err(HyperbytedbError::Internal( - "columnar ingest fell through fast path".into(), - )); - } - }; + let points = crate::application::ingest_metadata::parse_write_body( + body, + precision, + format, + self.max_points_per_request, + )?; if points.is_empty() { return Ok(()); } - if !matches!(format, WritePayloadFormat::LineProtocol) { - validate_point_count(points.len(), self.max_points_per_request)?; - } // Points whose fields conflict with a stored type are dropped and the // rest written, as InfluxDB does; the error says how many. @@ -994,8 +974,11 @@ impl IngestionPort for PeerIngestionService { return partial.map_or(Ok(()), Err); } + // Built once: the name guard and the metadata registration both + // need each measurement's names. + let schema = crate::application::ingest_metadata::BatchSchema::from_points(&points)?; self.name_guard - .check_points(&self.metadata, db, &retention_policy, &points) + .check_schema(&self.metadata, db, &retention_policy, &schema) .await?; let precision_val = Precision::from_str_opt(precision); @@ -1024,11 +1007,12 @@ impl IngestionPort for PeerIngestionService { return with_dropped(result, partial); } - prepare_batch_metadata( + crate::application::ingest_metadata::prepare_batch_metadata_with_schema( &self.metadata, db, &retention_policy, &points, + &schema, &self.limits, Some(&self.schema_cache), ) @@ -1043,11 +1027,7 @@ impl IngestionPort for PeerIngestionService { .metadata .get_measurement(db, &retention_policy, &point.measurement) .await? - && meta.materialized - && meta - .materialized_rp - .as_deref() - .is_none_or(|mr| mr == retention_policy) + && meta.refuses_raw_writes(&retention_policy) { return Err(HyperbytedbError::WriteToMaterializedDestination( point.measurement.clone(), diff --git a/hyperbytedb/src/application/peer_query_service.rs b/hyperbytedb/src/application/peer_query_service.rs index f03f816..179d804 100644 --- a/hyperbytedb/src/application/peer_query_service.rs +++ b/hyperbytedb/src/application/peer_query_service.rs @@ -5,13 +5,13 @@ use openraft::BasicNode; use openraft::error::ForwardToLeader; use crate::adapters::cluster::raft::HyperbytedbRaft; -use crate::adapters::cluster::raft::types::{ClusterRequest, ClusterResponse}; use crate::application::materialized_view_service::def_from_statement; use crate::application::predicate_sql::build_predicate_sql; use crate::application::query_service::check_authorization; use crate::application::shard_metadata_scatter::scatter_measurement_meta; use crate::application::shard_routing::ShardRoutingContext; use crate::domain::cluster::membership::ClusterMembership; +use crate::domain::cluster::raft_request::{ClusterRequest, ClusterResponse}; use crate::domain::cluster::types::MutationRequest; use crate::domain::database::{RetentionPolicy, retention_policy_from_create}; use crate::domain::materialized_view::MaterializedViewDef; @@ -186,6 +186,7 @@ impl PeerQueryService { async fn execute_raft_mutation( &self, db: &str, + retention_policy: Option<&str>, caller: Option<&crate::domain::user::StoredUser>, statement_id: u32, stmt: &Statement, @@ -199,8 +200,14 @@ impl PeerQueryService { error: Some(e.to_string()), }); } - match mutation_request_from_statement(stmt, db, &self.metadata, self.shard_routing.get()) - .await + match mutation_request_from_statement( + stmt, + db, + retention_policy, + &self.metadata, + self.shard_routing.get(), + ) + .await { Ok(req) => { self.replicate_mutation(req).await?; @@ -402,6 +409,7 @@ async fn resolve_mv_dest_rp( async fn mutation_request_from_statement( stmt: &Statement, db: &str, + retention_policy: Option<&str>, metadata: &Arc, shard_routing: Option<&Arc>, ) -> Result { @@ -412,7 +420,10 @@ async fn mutation_request_from_statement( }), Statement::DropDatabase(name) => Ok(MutationRequest::DropDatabase(name.clone())), Statement::Delete(del) => { - let del_rp = metadata.get_default_rp(db).await?; + let del_rp = match retention_policy { + Some(rp) => rp.to_string(), + None => metadata.get_default_rp(db).await?, + }; let predicate_sql = crate::application::predicate_sql::build_delete_predicate( metadata, db, @@ -543,7 +554,10 @@ async fn mutation_request_from_statement( MeasurementName::Name(n) => Some(n.clone()), MeasurementName::Regex(_) => None, }); - let drop_rp = metadata.get_default_rp(target_db).await?; + let drop_rp = match retention_policy { + Some(rp) => rp.to_string(), + None => metadata.get_default_rp(target_db).await?, + }; let predicate_sql = match (measurement.as_deref(), s.condition.as_ref()) { // Replicas delete the rows this selects. (Some(meas), cond) => { @@ -659,7 +673,7 @@ impl QueryService for PeerQueryService { let stmt_query = stmt_queries.get(i).map(String::as_str).unwrap_or(query); if use_raft && is_cluster_mutation(&stmt) { results.push( - self.execute_raft_mutation(db, caller, statement_id, &stmt) + self.execute_raft_mutation(db, retention_policy, caller, statement_id, &stmt) .await?, ); continue; @@ -702,7 +716,7 @@ impl QueryService for PeerQueryService { } result } - Statement::Delete(ref del) => { + Statement::Delete(_) => { let result = self .execute_inner_statement( db, @@ -714,27 +728,15 @@ impl QueryService for PeerQueryService { ) .await; if result.error.is_none() { - let del_rp = if let Some(rp) = retention_policy { - rp.to_string() - } else { - self.metadata.get_default_rp(db).await? - }; - let predicate_sql = - crate::application::predicate_sql::build_delete_predicate( - &self.metadata, - db, - &del_rp, - &del.from, - del.condition.as_ref(), - ) - .await?; - self.replicate_mutation(MutationRequest::Delete { - database: db.to_string(), - rp: del_rp, - measurement: del.from.clone(), - predicate_sql, - }) + let req = mutation_request_from_statement( + &stmt, + db, + retention_policy, + &self.metadata, + self.shard_routing.get(), + ) .await?; + self.replicate_mutation(req).await?; } result } @@ -1036,7 +1038,7 @@ impl QueryService for PeerQueryService { } result } - Statement::DropSeries(ref s) => { + Statement::DropSeries(_) => { let result = self .execute_inner_statement( db, @@ -1048,41 +1050,15 @@ impl QueryService for PeerQueryService { ) .await; if result.error.is_none() { - let target_db = s.database.as_deref().unwrap_or(db); - let measurement = s.from.as_ref().and_then(|n| match n { - MeasurementName::Name(n) => Some(n.clone()), - MeasurementName::Regex(_) => None, - }); - let ds_rp = if let Some(rp) = retention_policy { - rp.to_string() - } else { - self.metadata.get_default_rp(target_db).await? - }; - let predicate_sql = match (measurement.as_deref(), s.condition.as_ref()) { - // Replicas delete the rows this selects. - (Some(meas), cond) => { - crate::application::predicate_sql::build_delete_predicate( - &self.metadata, - target_db, - &ds_rp, - meas, - cond, - ) - .await? - } - (None, Some(cond)) => { - build_predicate_sql(&self.metadata, target_db, &ds_rp, "", cond) - .await? - } - (None, None) => String::new(), - }; - self.replicate_mutation(MutationRequest::DropSeries { - database: target_db.to_string(), - rp: ds_rp, - measurement, - predicate_sql, - }) + let req = mutation_request_from_statement( + &stmt, + db, + retention_policy, + &self.metadata, + self.shard_routing.get(), + ) .await?; + self.replicate_mutation(req).await?; } result } @@ -1219,7 +1195,7 @@ impl QueryService for PeerQueryService { } fn hash_password_for_replication(password: &str) -> Result { - crate::adapters::http::auth_middleware::hash_password(password) + crate::domain::user::hash_password(password) .map_err(|e| HyperbytedbError::Internal(crate::error::ChainedError::from_error(e))) } @@ -1245,6 +1221,234 @@ fn execute_or_error( mod tests { use super::*; + /// In Raft mode a DELETE / DROP SERIES built its mutation with the + /// database's default retention policy, ignoring `?rp=`: a delete on + /// `week` removed rows from `autogen` instead. + #[tokio::test] + async fn raft_delete_uses_the_requested_retention_policy() { + use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; + let dir = tempfile::tempdir().unwrap(); + let meta: Arc = Arc::new(RocksDbMetadata::open(dir.path()).unwrap()); + meta.create_database("db").await.unwrap(); + let mut week = RetentionPolicy::autogen(); + week.name = "week".to_string(); + week.is_default = false; + meta.create_retention_policy("db", week).await.unwrap(); + meta.register_measurement( + "db", + "week", + &crate::domain::measurement::MeasurementMeta { + name: "cpu".into(), + tag_keys: vec!["host".into()], + ..Default::default() + }, + ) + .await + .unwrap(); + + let rp_of = |req: MutationRequest| match req { + MutationRequest::Delete { rp, .. } | MutationRequest::DropSeries { rp, .. } => rp, + other => panic!("unexpected {other:?}"), + }; + for q in [ + "DELETE FROM cpu WHERE host='a'", + "DROP SERIES FROM cpu WHERE host='a'", + ] { + let stmt = &crate::timeseriesql::parse(q).unwrap()[0]; + let req = mutation_request_from_statement(stmt, "db", Some("week"), &meta, None) + .await + .unwrap(); + assert_eq!(rp_of(req), "week", "{q}"); + let req = mutation_request_from_statement(stmt, "db", None, &meta, None) + .await + .unwrap(); + assert_eq!(rp_of(req), "autogen", "{q}"); + } + } + + /// A replication port that does nothing: the Raft path below never + /// uses it. + struct NoReplication; + #[async_trait::async_trait] + impl ReplicationPort for NoReplication { + fn replicate_write( + self: Arc, + _batch: crate::ports::replication::OutboundReplicationBatch, + ) -> Result<(), HyperbytedbError> { + Ok(()) + } + async fn replicate_write_sync( + self: Arc, + _batch: crate::ports::replication::OutboundReplicationBatch, + _required_acks: usize, + _timeout: std::time::Duration, + ) -> Result<(), HyperbytedbError> { + Ok(()) + } + fn replicate_mutation(self: Arc, _req: MutationRequest, _targets: Option>) {} + async fn replicate_mutation_sync( + self: Arc, + _req: MutationRequest, + ) -> Result<(), HyperbytedbError> { + Ok(()) + } + async fn replication_peer_count(&self, _self_node_id: u64) -> usize { + 0 + } + } + + /// The Raft-mode DELETE wiring end to end: `?rp=week` must reach the + /// mutation through `execute_query` and `execute_raft_mutation`, and the + /// state machine must delete the rows in `week`, not in `autogen`. + #[tokio::test] + #[serial_test::serial(chdb)] + async fn raft_delete_with_rp_deletes_from_that_rp() { + use crate::adapters::chdb::native_adapter::ChdbNativeAdapter; + use crate::adapters::chdb::query_adapter::ChdbQueryAdapter; + use crate::adapters::chdb::session::SharedSession; + use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; + use crate::adapters::wal::rocksdb_wal::RocksDbWal; + use crate::application::flush_service::FlushServiceImpl; + use crate::application::ingestion_service::IngestionServiceImpl; + use crate::application::materialized_view_service::MaterializedViewService; + use crate::application::query_service::QueryServiceImpl; + use crate::composition::bootstrap::ClusterBootstrap; + use crate::ports::ingestion::{IngestionPort, WritePayloadFormat}; + use crate::ports::points_sink::PointsSinkPort; + use crate::ports::query::QueryPort; + use crate::ports::wal::WalPort; + + let dir = tempfile::tempdir().unwrap(); + let (meta_dir, wal_dir, chdb_dir) = ( + dir.path().join("meta"), + dir.path().join("wal"), + dir.path().join("chdb"), + ); + for p in [&meta_dir, &wal_dir, &chdb_dir] { + std::fs::create_dir_all(p).unwrap(); + } + let chdb = SharedSession::new_eager(chdb_dir.to_str().unwrap(), 1).unwrap(); + let query_port: Arc = + Arc::new(ChdbQueryAdapter::from_shared(chdb.clone(), 0)); + let sink: Arc = Arc::new(ChdbNativeAdapter::new(chdb)); + let wal: Arc = Arc::new(RocksDbWal::open(&wal_dir).unwrap()); + let metadata: Arc = Arc::new(RocksDbMetadata::open(&meta_dir).unwrap()); + let mv_service = Arc::new(MaterializedViewService::new( + metadata.clone(), + query_port.clone(), + sink.clone(), + )); + + metadata.create_database("db").await.unwrap(); + let mut week = RetentionPolicy::autogen(); + week.name = "week".to_string(); + week.is_default = false; + metadata.create_retention_policy("db", week).await.unwrap(); + let ingestion = IngestionServiceImpl::with_sink( + wal.clone(), + None, + metadata.clone(), + Default::default(), + 0, + ); + for rp in ["autogen", "week"] { + ingestion + .ingest( + "db", + Some(rp), + None, + b"cpu,host=a v=1 1000000000", + WritePayloadFormat::LineProtocol, + ) + .await + .unwrap(); + } + FlushServiceImpl::new(wal.clone(), 0, sink.clone()) + .flush() + .await + .unwrap(); + + let mut cluster_cfg = crate::config::HyperbytedbConfig::load(None) + .unwrap() + .cluster; + cluster_cfg.enabled = true; + cluster_cfg.node_id = 1; + cluster_cfg.cluster_addr = "127.0.0.1:18087".into(); + cluster_cfg.replication_log_dir = dir.path().join("repl").to_string_lossy().into(); + cluster_cfg.raft_dir = dir.path().join("raft").to_string_lossy().into(); + cluster_cfg.raft_heartbeat_interval_ms = Some(200); + cluster_cfg.raft_election_timeout_ms = Some(500); + let bootstrap = + ClusterBootstrap::init(&cluster_cfg, 1000, Default::default(), Default::default()) + .unwrap(); + let raft = bootstrap + .start_raft( + &cluster_cfg, + metadata.clone(), + mv_service, + sink.clone(), + wal.clone(), + None, + None, + ) + .await + .unwrap(); + for _ in 0..50 { + if raft.metrics().borrow().current_leader == Some(1) { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(100)).await; + } + + let base: Arc = Arc::new(QueryServiceImpl::new( + query_port, + metadata.clone(), + wal, + 30, + sink, + )); + let peer = PeerQueryService::new( + base.clone(), + metadata.clone(), + Arc::new(NoReplication), + &crate::adapters::cluster::cluster_auth::ClusterAuth::new(None).unwrap(), + ); + peer.set_raft(raft); + + let resp = peer + .execute_query( + "db", + "DELETE FROM cpu WHERE host='a'", + None, + Some("week"), + None, + ) + .await + .unwrap(); + assert!( + resp.results[0].error.is_none(), + "{:?}", + resp.results[0].error + ); + + let rows = |rp: &'static str| { + let base = base.clone(); + async move { + let q = format!(r#"SELECT v FROM "{rp}"."cpu""#); + let resp = base + .execute_query("db", &q, None, None, None) + .await + .unwrap(); + resp.results[0] + .series + .as_ref() + .map_or(0, |s| s.iter().map(|s| s.values.len()).sum::()) + } + }; + assert_eq!(rows("week").await, 0, "the DELETE named ?rp=week"); + assert_eq!(rows("autogen").await, 1, "autogen must be untouched"); + } + #[test] fn resolve_leader_addr_prefers_forward_node() { let forward = ForwardToLeader { diff --git a/hyperbytedb/src/application/physical_names.rs b/hyperbytedb/src/application/physical_names.rs index 784c0bc..de399e9 100644 --- a/hyperbytedb/src/application/physical_names.rs +++ b/hyperbytedb/src/application/physical_names.rs @@ -16,12 +16,13 @@ //! flushed. Replication and shard transfers do not check: they carry data //! an entry node already accepted. -use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::hash::{Hash, Hasher}; use std::sync::Arc; use parking_lot::RwLock; +use crate::application::ingest_metadata::BatchSchema; use crate::domain::chdb_naming::{ field_column_name, tag_column_name, unquoted_series_table_name, unquoted_table_name, }; @@ -62,48 +63,45 @@ impl PhysicalNameGuard { Self::default() } - /// Refuse `points` if any of their names maps onto storage that another + /// Refuse the batch if any of its names maps onto storage that another /// name already owns. /// /// # Errors /// [`HyperbytedbError::NameCollision`] for a colliding name, or a /// metadata error. - pub async fn check_points( + pub async fn check_schema( &self, metadata: &Arc, db: &str, rp: &str, - points: &[Point], + schema: &BatchSchema, ) -> Result<(), HyperbytedbError> { - let mut by_meas: BTreeMap<&str, BatchNames<'_>> = BTreeMap::new(); - let mut last: Option<&str> = None; - for p in points { - // Batches are mostly runs of one measurement. - if last != Some(p.measurement.as_str()) { - by_meas.entry(p.measurement.as_str()).or_default(); - last = Some(p.measurement.as_str()); - } - let Some(names) = by_meas.get_mut(p.measurement.as_str()) else { - continue; + for (measurement, (fields, tags)) in &schema.measurements { + let names = BatchNames { + fields: fields.keys().map(String::as_str).collect(), + tags: tags.iter().map(String::as_str).collect(), }; - for k in p.fields.keys() { - if !names.fields.contains(k.as_str()) { - names.fields.insert(k); - } - } - for k in p.tags.keys() { - if !names.tags.contains(k.as_str()) { - names.tags.insert(k); - } - } - } - for (measurement, names) in &by_meas { - self.check_measurement(metadata, db, rp, measurement, names) + self.check_measurement(metadata, db, rp, measurement, &names) .await?; } Ok(()) } + /// [`Self::check_schema`] for points whose schema is not built yet. + /// + /// # Errors + /// As [`Self::check_schema`], plus an in-batch `FieldTypeConflict`. + pub async fn check_points( + &self, + metadata: &Arc, + db: &str, + rp: &str, + points: &[Point], + ) -> Result<(), HyperbytedbError> { + let schema = BatchSchema::from_points(points)?; + self.check_schema(metadata, db, rp, &schema).await + } + /// [`Self::check_points`] for a columnar batch: one measurement, one /// field, and its tags. /// @@ -156,7 +154,11 @@ impl PhysicalNameGuard { { self.checked.write().clear(); } - if hashes.iter().all(|h| self.checked.read().contains(h)) { + let all_checked = { + let checked = self.checked.read(); + hashes.iter().all(|h| checked.contains(h)) + }; + if all_checked { return Ok(()); } @@ -287,6 +289,7 @@ mod tests { use crate::domain::database::RetentionPolicy; use crate::domain::measurement::MeasurementMeta; use crate::domain::point::FieldValue; + use std::collections::BTreeMap; fn point(meas: &str, field: &str, tag: &str) -> Point { Point { diff --git a/hyperbytedb/src/application/query_service.rs b/hyperbytedb/src/application/query_service.rs index efdcb24..370c6f1 100644 --- a/hyperbytedb/src/application/query_service.rs +++ b/hyperbytedb/src/application/query_service.rs @@ -37,7 +37,6 @@ use crate::error::HyperbytedbError; use crate::ports::metadata::MetadataPort; use crate::ports::query::{CqRunResult, QueryPort, QueryService}; use crate::ports::replication::{OutboundReplicationBatch, ReplicationPort}; -use crate::ports::sharding::ShardMapPort; use crate::timeseriesql::ast::*; use crate::timeseriesql::to_clickhouse; use crate::timeseriesql::to_clickhouse::select_has_true_aggregate; @@ -1699,10 +1698,9 @@ async fn execute_statement( password, admin, } => { - let password_hash = crate::adapters::http::auth_middleware::hash_password(password) - .map_err(|e| { - HyperbytedbError::Internal(crate::error::ChainedError::from_error(e)) - })?; + let password_hash = crate::domain::user::hash_password(password).map_err(|e| { + HyperbytedbError::Internal(crate::error::ChainedError::from_error(e)) + })?; crate::application::schema_mutation_apply::create_user( svc.metadata.as_ref(), username, @@ -1725,10 +1723,9 @@ async fn execute_statement( }) } Statement::SetPassword { username, password } => { - let password_hash = crate::adapters::http::auth_middleware::hash_password(password) - .map_err(|e| { - HyperbytedbError::Internal(crate::error::ChainedError::from_error(e)) - })?; + let password_hash = crate::domain::user::hash_password(password).map_err(|e| { + HyperbytedbError::Internal(crate::error::ChainedError::from_error(e)) + })?; crate::application::schema_mutation_apply::set_password( svc.metadata.as_ref(), username, @@ -2002,10 +1999,6 @@ fn series_results_to_json_each_row(series: &[SeriesResult]) -> Result String { - s.replace('\\', "\\\\").replace('\'', "\\'") -} - fn inject_tombstone_predicates(sql: String, tombstones: &[(String, String)]) -> String { if tombstones.is_empty() { return sql; @@ -2275,7 +2268,8 @@ async fn execute_select_from_source( } stamp_aggregate_time(sub_stmt, &mut inner, Some("ns"), time_min); let inner_json = series_results_to_json_each_row(&inner)?; - let escaped = escape_clickhouse_string_literal(&inner_json); + let mut escaped = String::with_capacity(inner_json.len()); + crate::domain::chdb_naming::push_string_literal_body(&mut escaped, &inner_json); let sub_source_sql = format!( "(SELECT * EXCEPT (`__time`), fromUnixTimestamp64Nano(toInt64(`__time`)) AS `time` FROM format(JSONEachRow, '{escaped}'))" ); diff --git a/hyperbytedb/src/application/replication_apply.rs b/hyperbytedb/src/application/replication_apply.rs index 428bb0d..c79003e 100644 --- a/hyperbytedb/src/application/replication_apply.rs +++ b/hyperbytedb/src/application/replication_apply.rs @@ -180,27 +180,6 @@ impl ReplicationApplyQueue { Arc::new(Self { tx }) } - pub fn with_workers( - depth: usize, - num_workers: usize, - metadata: Arc, - wal: Arc, - limits: IngestCardinalityLimits, - ) -> Arc { - Self::with_workers_and_sink( - depth, - num_workers, - metadata, - wal, - None, - limits, - 0, - false, - 0, - None, - ) - } - pub fn with_defaults( metadata: Arc, wal: Arc, diff --git a/hyperbytedb/src/application/retention_service.rs b/hyperbytedb/src/application/retention_service.rs index 2a36988..73962dd 100644 --- a/hyperbytedb/src/application/retention_service.rs +++ b/hyperbytedb/src/application/retention_service.rs @@ -2,9 +2,9 @@ use metrics::counter; use std::sync::Arc; use tokio::sync::watch; -use crate::adapters::cluster::raft::HyperbytedbRaft; use crate::domain::chdb_naming::{quoted_series_table_name, quoted_table_name}; use crate::error::HyperbytedbError; +use crate::ports::leadership::LeadershipPort; use crate::ports::metadata::MetadataPort; use crate::ports::query::QueryPort; use crate::ports::sharding::ShardMapPort; @@ -12,7 +12,7 @@ use crate::ports::sharding::ShardMapPort; pub struct RetentionService { metadata: Arc, query: Arc, - raft: Option, + leadership: Option>, sharding_enabled: bool, shard_map: Option>, node_id: u64, @@ -22,13 +22,13 @@ impl RetentionService { pub fn new( metadata: Arc, query: Arc, - raft: Option, + leadership: Option>, node_id: u64, ) -> Self { Self { metadata, query, - raft, + leadership, sharding_enabled: false, shard_map: None, node_id, @@ -46,13 +46,7 @@ impl RetentionService { } fn is_raft_leader(&self) -> bool { - match &self.raft { - Some(raft) => { - let metrics = raft.metrics().borrow().clone(); - metrics.current_leader == Some(self.node_id) - } - None => true, - } + crate::ports::leadership::may_run(self.leadership.as_deref(), self.node_id) } pub async fn run(&self, interval: std::time::Duration, mut shutdown_rx: watch::Receiver) { @@ -61,7 +55,7 @@ impl RetentionService { tracing::info!( interval = ?interval, - raft_gated = self.raft.is_some(), + raft_gated = self.leadership.is_some(), node_id = self.node_id, "retention service started" ); diff --git a/hyperbytedb/src/application/runtime/shard_heartbeat.rs b/hyperbytedb/src/application/runtime/shard_heartbeat.rs index 0896fd4..7e1568a 100644 --- a/hyperbytedb/src/application/runtime/shard_heartbeat.rs +++ b/hyperbytedb/src/application/runtime/shard_heartbeat.rs @@ -9,12 +9,11 @@ use reqwest::StatusCode; use tokio::sync::watch; use crate::adapters::cluster::peer_client::PeerClient; -use crate::adapters::cluster::raft::HyperbytedbRaft; -use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::application::runtime::RegionWriteStats; use crate::domain::chdb_naming::unquoted_table_name; use crate::domain::cluster::membership::SharedMembership; use crate::domain::sharding::{RegionHeartbeat, ShardMap, ShardRegion}; +use crate::ports::leadership::LeadershipPort; use crate::ports::metadata::MetadataPort; use crate::ports::query::QueryPort; use crate::ports::sharding::ShardMapPort; @@ -39,8 +38,8 @@ pub async fn run_region_heartbeat_reporter( node_id: u64, peer_client: Arc, membership: SharedMembership, - shard_map: Arc, - raft: HyperbytedbRaft, + shard_map: Arc, + leadership: Arc, metadata: Arc, query_port: Arc, region_write_stats: Arc, @@ -60,8 +59,8 @@ pub async fn run_region_heartbeat_reporter( node_id, &peer_client, &membership, - &shard_map, - &raft, + shard_map.as_ref(), + leadership.as_ref(), metadata.as_ref(), query_port.as_ref(), region_write_stats.as_ref(), @@ -86,8 +85,8 @@ async fn report_once( node_id: u64, peer_client: &PeerClient, membership: &SharedMembership, - shard_map: &RocksDbShardMap, - raft: &HyperbytedbRaft, + shard_map: &dyn ShardMapPort, + leadership: &dyn LeadershipPort, metadata: &dyn MetadataPort, query_port: &dyn QueryPort, region_write_stats: &RegionWriteStats, @@ -96,10 +95,8 @@ async fn report_once( byte_estimates: &mut ByteEstimateCache, ) -> Result<(), String> { let map = shard_map.snapshot().await.map_err(|e| e.to_string())?; - let leader_id = raft - .metrics() - .borrow() - .current_leader + let leader_id = leadership + .current_leader() .ok_or_else(|| "no raft leader".to_string())?; let leader_addr = { diff --git a/hyperbytedb/src/application/schema_mutation_apply.rs b/hyperbytedb/src/application/schema_mutation_apply.rs index 5019033..ad7145a 100644 --- a/hyperbytedb/src/application/schema_mutation_apply.rs +++ b/hyperbytedb/src/application/schema_mutation_apply.rs @@ -49,6 +49,21 @@ pub fn drop_space_for( } /// Dependencies required to apply schema mutations with full local side effects. +/// Apply CREATE DATABASE locally (idempotent, honors WITH retention policy on peers). +pub async fn apply_create_database( + metadata: &Arc, + name: &str, + rp: Option, +) -> Result<(), crate::error::HyperbytedbError> { + if let Some(rp) = rp { + let stmt = + crate::domain::database::create_database_statement_from_rp(name.to_string(), &rp); + metadata.create_database_with(&stmt).await + } else { + metadata.create_database(name).await + } +} + pub struct SchemaMutationDeps<'a> { pub metadata: &'a Arc, pub mv_service: Option<&'a MaterializedViewService>, @@ -167,10 +182,7 @@ async fn apply_schema_mutation_inner( } = deps; match mutation { MutationRequest::CreateDatabase { name, rp } => { - crate::adapters::cluster::raft::state_machine::apply_create_database( - metadata, &name, rp, - ) - .await + apply_create_database(metadata, &name, rp).await } MutationRequest::DropDatabase(name) => { let sink = points_sink.or_else(|| mv_service.map(|mv| mv.points_sink())); diff --git a/hyperbytedb/src/application/shard_metadata_scatter.rs b/hyperbytedb/src/application/shard_metadata_scatter.rs index 2d8de66..9458629 100644 --- a/hyperbytedb/src/application/shard_metadata_scatter.rs +++ b/hyperbytedb/src/application/shard_metadata_scatter.rs @@ -15,7 +15,6 @@ use crate::domain::series::SeriesKey; use crate::domain::sharding::{ShardMetadataKind, ShardMetadataRequest, ShardRegion}; use crate::error::HyperbytedbError; use crate::ports::metadata::{MeasurementMeta, MetadataPort}; -use crate::ports::sharding::ShardMapPort; async fn fetch_remote_metadata( ctx: &ShardRoutingContext, diff --git a/hyperbytedb/src/application/shard_routing.rs b/hyperbytedb/src/application/shard_routing.rs index 4d0e0ed..b8f3358 100644 --- a/hyperbytedb/src/application/shard_routing.rs +++ b/hyperbytedb/src/application/shard_routing.rs @@ -8,14 +8,13 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use metrics::{counter, histogram}; use crate::adapters::cluster::peer_client::PeerClient; -use crate::adapters::cluster::raft::types::{ClusterRequest, ClusterResponse}; -use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::application::runtime::RegionWriteStats; use crate::application::shard_peer_resolution::{ RegionTargetRole, ScatterKind, active_region_peer_targets, is_serving_node, peer_addr, resolve_region_peers, }; use crate::config::ShardingConfig; +use crate::domain::cluster::raft_request::{ClusterRequest, ClusterResponse}; use crate::domain::point::Point; use crate::domain::series::series_id_for_point; use crate::domain::sharding::ShardLocationCache; @@ -27,7 +26,7 @@ use crate::ports::metadata::MetadataPort; use crate::ports::sharding::ShardMapPort; pub struct ShardRoutingContext { - pub shard_map: Arc, + pub shard_map: Arc, pub location_cache: Arc, pub config: ShardingConfig, pub node_id: u64, diff --git a/hyperbytedb/src/application/shard_scheduler.rs b/hyperbytedb/src/application/shard_scheduler.rs index c5526d9..b219414 100644 --- a/hyperbytedb/src/application/shard_scheduler.rs +++ b/hyperbytedb/src/application/shard_scheduler.rs @@ -7,14 +7,10 @@ use tokio::sync::watch; use crate::adapters::cluster::peer_client::PeerClient; use crate::adapters::cluster::raft::HyperbytedbRaft; -use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::application::runtime_settings as rs; use crate::application::shard_peer_resolution::is_active_peer; use crate::application::shard_routing::effective_replication_factor; -use crate::application::shard_transfer::{ - TRANSFER_CHUNK_POINTS, complete_region_transfer, push_region_transfer_data, - stage_region_transfer_data, -}; +use crate::application::shard_transfer::{TRANSFER_CHUNK_POINTS, stage_region_transfer_data}; use crate::config::ShardingConfig; use crate::domain::cluster::membership::{ NodeInfo, NodeState, SharedMembership, holds_placement, is_departing, is_serving, @@ -28,7 +24,6 @@ use crate::domain::sharding::{ }; use crate::error::HyperbytedbError; use crate::ports::metadata::MetadataPort; -use crate::ports::points_sink::PointsSinkPort; use crate::ports::query::QueryPort; use crate::ports::sharding::ShardMapPort; use crate::ports::wal::WalPort; @@ -253,51 +248,16 @@ fn max_region_peer_heartbeat_bytes(hb: &[RegionHeartbeatRow], region: &ShardRegi .unwrap_or(0) } -/// One outstanding post-commit split-transfer movement awaiting verified -/// re-push. Ordering detail only — durable intent lives in the shard map's -/// `transfer_verified` flag, so leadership change or process restart rebuilds -/// the queue from a map scan. -#[derive(Debug, Clone)] -/// Dead since splits became metadata-only: the split was the only caller, -/// and nothing creates transfer debt any more. Retained deliberately — -/// removing the reconciliation machinery is a large deletion that deserves -/// its own review, not a side effect of an unrelated change. -#[allow(dead_code)] -pub(crate) struct PendingTransfer { - pub key: MeasurementKey, - pub region_id: u64, - pub start: u64, - pub end: u64, - /// Source that provably held the rows at split time (the old primary). - /// Candidate order at drain time is: current primary (authoritative - /// owner), then this node, then remaining live peers. - pub recorded_source: u64, - pub resolve_at_drain: bool, - pub ticks_without_progress: u32, - /// Parked entries stop consuming retries; the map flag stays false so - /// the debt remains visible and re-enqueued after restart. - pub parked: bool, - pub first_seen: i64, -} - -/// Ticks without applied-count progress before a stuck entry escalates. -#[allow(dead_code)] -const RECONCILE_PARK_AFTER_TICKS: u32 = 10; -/// Minimum debt age (persisted via `transfer_first_seen`) before parking. -#[allow(dead_code)] -const RECONCILE_MIN_AGE_SECS: i64 = 300; - pub struct ShardScheduler { /// This node's runtime settings: `SET` overrides for the gates below. runtime_settings: Arc, - shard_map: Arc, + shard_map: Arc, membership: SharedMembership, raft: HyperbytedbRaft, peer_client: Option>, metadata: Arc, wal: Arc, query_port: Option>, - points_sink: Option>, node_id: u64, config: ShardingConfig, max_points_per_request: usize, @@ -320,9 +280,6 @@ pub struct ShardScheduler { placement_attempt_seq: std::sync::atomic::AtomicU64, /// (db, rp, measurement, region_id) -> unix secs when primary first seen unhealthy unhealthy_primaries: tokio::sync::RwLock>, - /// Post-commit split-transfer reconciliation queue. Idempotent per - /// (key, region_id); rebuilt from the map scan on leadership change. - reconciliation_queue: std::sync::Mutex>, /// Materialized-view rollup destinations (SummingMergeTree spaces), /// refreshed from metadata each drain. Re-delivery there would /// permanently corrupt additive aggregates, so those spaces are never @@ -387,14 +344,13 @@ pub struct ShardScheduler { impl ShardScheduler { #[allow(clippy::too_many_arguments)] pub fn new( - shard_map: Arc, + shard_map: Arc, membership: SharedMembership, raft: HyperbytedbRaft, peer_client: Option>, metadata: Arc, wal: Arc, query_port: Option>, - points_sink: Option>, node_id: u64, config: ShardingConfig, max_points_per_request: usize, @@ -412,7 +368,6 @@ impl ShardScheduler { metadata, wal, query_port, - points_sink, node_id, config, max_points_per_request, @@ -424,7 +379,6 @@ impl ShardScheduler { placement_last_attempted: tokio::sync::RwLock::new(HashMap::new()), placement_attempt_seq: std::sync::atomic::AtomicU64::new(0), unhealthy_primaries: tokio::sync::RwLock::new(HashMap::new()), - reconciliation_queue: std::sync::Mutex::new(Vec::new()), rollup_dests: tokio::sync::RwLock::new(HashMap::new()), rollup_handoffs: Arc::new(tokio::sync::Mutex::new(HashMap::new())), peer_map_versions: tokio::sync::RwLock::new(HashMap::new()), @@ -724,56 +678,6 @@ impl ShardScheduler { } } - /// Cut over any staging region whose data has finished moving. - /// - /// A split publishes its child NOT serving: until this fires the donor - /// still holds and serves the range, so readers never see a region that is - /// missing rows. Activation is gated on the movement being verified -- - /// `transfer_verified` is the reconciliation machinery's own completion - /// signal -- so the child only becomes routable once it actually holds the - /// data. A child that kept the donor's primary carries no debt and - /// activates on the next tick. - /// - /// The flip is one committed op, so every node's location cache applies it - /// in the same apply: no node sees a half-applied handover. - #[allow(dead_code)] - async fn try_activate_staged_regions(&self, map: &ShardMap) { - for space in map.spaces.values() { - for region in &space.regions { - if region.is_serving() { - continue; - } - if region.transfer_outstanding() { - continue; - } - match self - .propose(ShardMapOp::ActivateRegion { - key: space.key.clone(), - region_id: region.region_id, - epoch: region.epoch, - }) - .await - { - Ok(()) => { - counter!("hyperbytedb_shard_region_activations_total").increment(1); - tracing::info!( - region_id = region.region_id, - primary = region.primary, - "staging region cut over to serving" - ); - } - Err(e) => { - tracing::warn!( - region_id = region.region_id, - error = %e, - "staging region cutover failed; will retry" - ); - } - } - } - } - } - async fn clear_placement_stall(&self, region_id: u64) { self.placement_stalls.write().await.remove(®ion_id); } @@ -804,7 +708,7 @@ impl ShardScheduler { /// Reload the materialized view destinations [`Self::is_rollup_dest`] /// answers from. Until this ran every tick the set stayed empty in - /// production (its only caller was the dead reconciliation drain), and + /// production (its only caller was the split-transfer reconciliation drain), and /// placement moved destination regions: on n3 a moved destination's rows /// reached the flush, which refuses writes to a destination, and held /// the WAL on every retry. @@ -1848,86 +1752,6 @@ impl ShardScheduler { SplitOutcome::Refused(reason) } - /// Propose clearing a verified split-transfer debt. No-op success when - /// the region carries no outstanding flag. Gated behind - /// `transfer_clear_proposals_enabled` for mixed-version rolling upgrades: - /// old binaries cannot decode the `ClearVerified` op from the Raft log. - // Unreachable in production since splits became metadata-only: the split - // was its only caller. Nothing now creates transfer debt either -- - // apply-time normalization flags a child only when its primary differs - // from the parent's, which a TiKV-style split never does, and - // TransferPrimary does not set it. So the reconciliation queue, its drain, - // and ClearVerified are all currently dead paths. - // - // Retained rather than deleted in the same change as the behaviour switch: - // removing that machinery is a large, separate deletion that deserves its - // own review. The test below still covers the rollup-destination exclusion. - #[allow(dead_code)] - fn enqueue_reconciliation(&self, entry: PendingTransfer) { - match self.rollup_dests.try_read() { - Ok(set) if set.contains_key(&entry.key) => { - // SummingMergeTree destinations must never receive re-delivered - // rows (additive aggregates would permanently double-count). - // Their primary changes are provisioned by the MV backfill path. - tracing::info!( - db = %entry.key.db, - rp = %entry.key.rp, - measurement = %entry.key.measurement, - region_id = entry.region_id, - "rollup destination space excluded from transfer reconciliation" - ); - return; - } - Ok(_) => {} - Err(_) => { - tracing::warn!( - db = %entry.key.db, - rp = %entry.key.rp, - measurement = %entry.key.measurement, - region_id = entry.region_id, - "rollup dest set contended; skipping transfer enqueue" - ); - return; - } - } - let mut q = self - .reconciliation_queue - .lock() - .unwrap_or_else(|p| p.into_inner()); - if let Some(existing) = q - .iter_mut() - .find(|e| e.key == entry.key && e.region_id == entry.region_id) - { - existing.recorded_source = entry.recorded_source; - existing.resolve_at_drain = false; - existing.parked = false; - return; - } - q.push(entry); - } - - #[allow(dead_code)] - async fn propose_clear_verified( - &self, - key: &MeasurementKey, - region: &ShardRegion, - ) -> Result<(), HyperbytedbError> { - if !region.transfer_outstanding() { - return Ok(()); - } - if !self.config.transfer_clear_proposals_enabled { - return Err(HyperbytedbError::ShardMap( - "ClearVerified proposals disabled (upgrade window)".into(), - )); - } - self.propose(ShardMapOp::ClearVerified { - key: key.clone(), - region_id: region.region_id, - epoch: region.epoch, - }) - .await - } - async fn compute_split_key( &self, key: &MeasurementKey, @@ -2238,8 +2062,8 @@ impl ShardScheduler { if self.peer_client.is_none() { return Ok(false); } - // Re-read: `drain_reconciliation` runs before the tick loop and can - // bump epochs, so the caller's snapshot may already be stale. + // Re-read: earlier steps of the tick can bump epochs, so the caller's + // snapshot may already be stale. let map = self.shard_map.snapshot().await?; metrics::gauge!("hyperbytedb_shard_placement_map_version").set(map.map_version as f64); let membership_version = self.membership.read().await.version; @@ -2664,7 +2488,7 @@ impl ShardScheduler { sink.lock().unwrap().push(op); return Ok(()); } - use crate::adapters::cluster::raft::types::ClusterRequest; + use crate::domain::cluster::raft_request::ClusterRequest; let req = ClusterRequest::ShardMapMutation(Box::new(op)); let resp = self .raft @@ -2872,98 +2696,6 @@ pub fn current_region<'a>( .find(|r| r.region_id == region_id) } -#[allow(clippy::too_many_arguments)] -#[allow(dead_code)] -async fn push_and_drop_range( - peer_client: &Arc, - metadata: &Arc, - wal: &Arc, - query_port: Option<&Arc>, - sink: Option<&Arc>, - source_node_id: u64, - key: &MeasurementKey, - range: &ShardRegion, - dest_primary: u64, - max_points: usize, -) -> Result<(), HyperbytedbError> { - // push_region_transfer_data fails unless the destination confirmed every - // exported point; complete_region_transfer then acks and drops the source - // copy. The range is owned by another region after a split, so dropping is - // safe here — and only here. - let outcome = push_region_transfer_data( - peer_client, - metadata, - wal, - query_port, - source_node_id, - key, - range, - dest_primary, - max_points, - ) - .await?; - if outcome.transfer_id == 0 { - // transfer_id == 0 ⇔ source == destination: ownership moved between - // resolution and execution. This is a collision signal for the - // reconciliation queue — never silent success, and it must not - // authorize dropping the range from the node that owns it. - return Err(HyperbytedbError::TransferCollision); - } - complete_region_transfer( - peer_client, - metadata, - sink, - source_node_id, - key, - range, - dest_primary, - outcome.transfer_id, - true, - ) - .await -} - -#[allow(clippy::too_many_arguments)] -async fn request_region_rehome( - peer_client: &PeerClient, - membership: &SharedMembership, - target_node: u64, - key: &MeasurementKey, - range: &ShardRegion, - dest_primary: u64, - drop_source: bool, -) -> Result<(), HyperbytedbError> { - let addr = { - let m = membership.read().await; - m.get_node(target_node).map(|n| n.addr.clone()) - } - .ok_or_else(|| { - HyperbytedbError::PeerUnreachable(format!("rehome target node {target_node} unknown")) - })?; - let req = ShardRehomeRequest { - db: key.db.clone(), - rp: key.rp.clone(), - measurement: key.measurement.clone(), - start: range.start, - end: range.end, - region_id: range.region_id, - epoch: range.epoch, - dest_primary, - drop_source, - stage: false, - max_bytes_per_sec: 0, - dest_skip_wal_through: 0, - }; - post_rehome( - peer_client.http_client(), - &format!("http://{addr}/internal/shard/rehome"), - &req, - Duration::from_secs(REHOME_IDLE_SECS), - ) - .await?; - Ok(()) -} - /// Ask the current primary to stage `[start, end)` onto a joiner that is /// not yet a committed peer. async fn request_region_stage( @@ -3011,39 +2743,6 @@ async fn request_region_stage( Ok(body.get("bytes").and_then(|v| v.as_u64()).unwrap_or(0)) } -/// Reconciliation disposition for a failed post-commit transfer movement. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -#[allow(dead_code)] -pub(crate) enum TransferDisposition { - /// Transient (transport, apply lag 404/409, 5xx): retry next drain. - Retryable, - /// Source/destination identity collapsed or moved (400 dest-is-self, - /// collision): re-resolve roles and retry. - Reresolve, - /// Nothing to retry (logic error): park with alarm. - Park, -} - -#[allow(dead_code)] -pub(crate) fn classify_transfer_error(err: &HyperbytedbError) -> TransferDisposition { - match err { - HyperbytedbError::PeerUnreachable(_) | HyperbytedbError::ReplicationTimeout(_) => { - TransferDisposition::Retryable - } - HyperbytedbError::TransferRejected { status, .. } => match status { - 400 => TransferDisposition::Reresolve, - 404 | 409 => TransferDisposition::Retryable, - s if *s >= 500 => TransferDisposition::Retryable, - _ => TransferDisposition::Park, - }, - HyperbytedbError::TransferCollision => TransferDisposition::Reresolve, - // Map-state errors are typically transient snapshots; bounded by the - // progress-based parking below. - HyperbytedbError::ShardMap(_) => TransferDisposition::Retryable, - _ => TransferDisposition::Park, - } -} - async fn pick_best_primary( membership: &SharedMembership, region: &ShardRegion, @@ -3166,360 +2865,7 @@ async fn pick_alternate_primary( .find(|id| peer_is_active_alternate(&m, region, *id, self_id)) } -#[allow(dead_code)] -fn now_unix() -> i64 { - chrono::Utc::now().timestamp() -} - -impl ShardScheduler { - /// Rebuild reconciliation entries from the committed map scan: any - /// flagged child without an in-memory entry gets one. Makes leader change - /// and process restart equivalent — durable intent lives in the map. - #[allow(dead_code)] - fn rebuild_queue_from_map(&self, map: &super::super::domain::sharding::types::ShardMap) { - let dests = match self.rollup_dests.try_read() { - Ok(guard) => guard, - Err(_) => { - tracing::warn!("rollup dest set contended; skipping transfer queue rebuild"); - return; - } - }; - let mut q = self - .reconciliation_queue - .lock() - .unwrap_or_else(|p| p.into_inner()); - for space in map.spaces.values() { - if dests.contains_key(&space.key) { - continue; - } - for region in &space.regions { - if !region.transfer_outstanding() { - continue; - } - let exists = q - .iter() - .any(|e| e.key == space.key && e.region_id == region.region_id); - if exists { - continue; - } - // The old primary is unknown post-restart; the current primary - // is both destination and best initial source guess. - q.push(PendingTransfer { - key: space.key.clone(), - region_id: region.region_id, - start: region.start, - end: region.end, - recorded_source: region.primary, - resolve_at_drain: false, - ticks_without_progress: 0, - parked: false, - first_seen: region.transfer_first_seen.unwrap_or_else(now_unix), - }); - } - } - } - - /// Drain due split-transfer reconciliation entries. Called once per - /// leader tick, before scheduling heuristics. - #[allow(dead_code)] - async fn drain_reconciliation(&self, map: &super::super::domain::sharding::types::ShardMap) { - self.refresh_rollup_dests().await; - self.rebuild_queue_from_map(map); - - let pending = self.pending_transfers(); - metrics::gauge!("hyperbytedb_shard_transfer_queue_pending") - .set(pending.iter().filter(|e| !e.parked).count() as f64); - let now_ts = now_unix(); - let oldest_age = pending - .iter() - .map(|e| now_ts - e.first_seen) - .max() - .unwrap_or(0); - metrics::gauge!("hyperbytedb_shard_transfer_queue_oldest_age_secs") - .set(oldest_age.max(0) as f64); - - for entry in pending { - let Some(space) = map.space(&entry.key.db, &entry.key.rp, &entry.key.measurement) - else { - self.remove_reconciliation(&entry.key, entry.region_id); - continue; - }; - let region = space - .regions - .iter() - .find(|r| { - r.region_id == entry.region_id - || (entry.resolve_at_drain && r.start == entry.start && r.end == entry.end) - }) - .cloned(); - let Some(region) = region else { - // Region vanished (merged away / manual surgery): debt gone. - self.remove_reconciliation(&entry.key, entry.region_id); - continue; - }; - if !region.transfer_outstanding() { - // Cleared elsewhere. - self.remove_reconciliation(&entry.key, region.region_id); - continue; - } - if entry.parked || self.region_has_operator(region.region_id) { - continue; - } - let Some(operator) = self.acquire_operator(region.region_id) else { - continue; - }; - - let outcome = self - .attempt_reconciliation(&entry.key, ®ion, &entry) - .await; - drop(operator); - - match outcome { - ReconcileOutcome::Cleared => { - self.remove_reconciliation(&entry.key, region.region_id); - } - ReconcileOutcome::Stalled => { - let mut q = self - .reconciliation_queue - .lock() - .unwrap_or_else(|p| p.into_inner()); - if let Some(e) = q - .iter_mut() - .find(|e| e.key == entry.key && e.region_id == region.region_id) - { - e.ticks_without_progress += 1; - let age = now_ts - e.first_seen; - if e.ticks_without_progress >= RECONCILE_PARK_AFTER_TICKS - && age >= RECONCILE_MIN_AGE_SECS - { - e.parked = true; - counter!("hyperbytedb_shard_transfer_parked_total").increment(1); - tracing::warn!( - db = %entry.key.db, - measurement = %entry.key.measurement, - region_id = region.region_id, - age_secs = age, - "split-transfer reconciliation parked; operator action required" - ); - } - } - } - ReconcileOutcome::Escalate => { - match self.escalate_stuck_transfer(&entry.key, ®ion).await { - Ok(()) => { - let mut q = self - .reconciliation_queue - .lock() - .unwrap_or_else(|p| p.into_inner()); - if let Some(e) = q - .iter_mut() - .find(|e| e.key == entry.key && e.region_id == region.region_id) - { - // Ownership moved via TransferPrimary; give the - // new topology a fresh retry budget. - e.ticks_without_progress = 0; - } - } - Err(err) => { - tracing::warn!( - region_id = region.region_id, - error = %err, - "reconciliation escalation failed" - ); - let mut q = self - .reconciliation_queue - .lock() - .unwrap_or_else(|p| p.into_inner()); - if let Some(e) = q - .iter_mut() - .find(|e| e.key == entry.key && e.region_id == region.region_id) - { - e.parked = true; - counter!("hyperbytedb_shard_transfer_parked_total").increment(1); - } - } - } - } - } - } - } - - /// One reconciliation attempt against the current committed region state. - #[allow(dead_code)] - async fn attempt_reconciliation( - &self, - key: &MeasurementKey, - region: &ShardRegion, - entry: &PendingTransfer, - ) -> ReconcileOutcome { - let dest = region.primary; - let Some(pc) = self.peer_client.as_ref() else { - return ReconcileOutcome::Stalled; - }; - - // Authoritative-source order: the node that provably held the rows at - // split time first, then remaining live peers with region coverage - // (watermark > 0). The destination itself is never a source. - let m = self.membership.read().await; - let mut sources: Vec = vec![entry.recorded_source]; - sources.extend(region.peers.iter().copied()); - sources.retain(|id| { - *id != dest - && (*id == self.node_id - || m.get_node(*id) - .is_some_and(|n| n.state == NodeState::Active)) - }); - drop(m); - - for source in sources { - // Coverage proof for non-recorded sources: a peer whose region - // watermark is 0 holds no rows for this region and would verify - // vacuously (the 0==0 hazard documented on rebalance). - if source != entry.recorded_source && source != self.node_id { - // Unknown counts as "no proof of coverage" here, same as a - // real zero: this path is choosing a source to copy FROM, so - // the conservative answer is to skip it. - let wm = peer_region_wal_watermark(pc, source, region.region_id).await; - if wm.unwrap_or(0) == 0 { - continue; - } - } - - let result = if source == self.node_id { - push_and_drop_range( - pc, - &self.metadata, - &self.wal, - self.query_port.as_ref(), - self.points_sink.as_ref(), - self.node_id, - key, - region, - dest, - self.max_points_per_request, - ) - .await - } else { - // `region.is_serving()`, not `true`: a staging region's range is - // still served by its donor, so the donor must keep its rows - // until cutover. Dropping first is what left 13 staging regions - // serving from a donor that had already deleted their rows. - // After activation the donor's leftover rows are inert -- every - // region query injects a `series_id` range predicate -- and are - // reclaimed by a later GC. - request_region_rehome( - pc, - &self.membership, - source, - key, - region, - dest, - region.is_serving(), - ) - .await - }; - - match result { - Ok(()) => { - counter!("hyperbytedb_shard_transfer_retries_total").increment(1); - return match self.propose_clear_verified(key, region).await { - Ok(()) => ReconcileOutcome::Cleared, - Err(e) => { - tracing::warn!( - region_id = region.region_id, - error = %e, - "verified re-push complete; ClearVerified proposal failed" - ); - // Keep the entry so verification is not lost; do - // NOT re-push every tick against a flag we could - // not clear. - ReconcileOutcome::Stalled - } - }; - } - Err(e) => { - tracing::debug!( - region_id = region.region_id, - source_node = source, - error = %e, - "reconciliation movement attempt failed" - ); - match classify_transfer_error(&e) { - TransferDisposition::Reresolve => continue, - TransferDisposition::Retryable => { - counter!("hyperbytedb_shard_transfer_retries_total").increment(1); - return ReconcileOutcome::Stalled; - } - TransferDisposition::Park => { - return ReconcileOutcome::Escalate; - } - } - } - } - } - ReconcileOutcome::Stalled - } - - /// Break a stuck transfer by moving ownership to another live peer via - /// `TransferPrimary`. The existing transfer-primary path stages rows from - /// the current authoritative primary before committing ownership, so this - /// cannot commit a data-less node (mirrors `try_rebalance` ordering). - #[allow(dead_code)] - async fn escalate_stuck_transfer( - &self, - key: &MeasurementKey, - region: &ShardRegion, - ) -> Result<(), HyperbytedbError> { - let Some(new_primary) = - pick_alternate_primary(&self.membership, region, self.node_id).await - else { - return Err(HyperbytedbError::PeerUnreachable( - "no live alternate peer for reconciliation escalation".into(), - )); - }; - tracing::warn!( - db = %key.db, - measurement = %key.measurement, - region_id = region.region_id, - new_primary, - "escalating stuck split-transfer via TransferPrimary" - ); - self.propose(ShardMapOp::TransferPrimary { - key: key.clone(), - region_id: region.region_id, - new_primary, - epoch: region.epoch, - }) - .await - } - - #[allow(dead_code)] - fn remove_reconciliation(&self, key: &MeasurementKey, region_id: u64) { - let mut q = self - .reconciliation_queue - .lock() - .unwrap_or_else(|p| p.into_inner()); - q.retain(|e| !(e.key == *key && e.region_id == region_id)); - } - - #[allow(dead_code)] - fn pending_transfers(&self) -> Vec { - self.reconciliation_queue - .lock() - .unwrap_or_else(|p| p.into_inner()) - .clone() - } -} - -#[allow(dead_code)] -enum ReconcileOutcome { - /// Verified and the map flag cleared: drop the entry. - Cleared, - /// No progress this tick; bounded retry continues. - Stalled, - /// Terminal/repeatedly-stuck: move ownership or park. - Escalate, -} +impl ShardScheduler {} fn peer_is_active_alternate( membership: &crate::domain::cluster::membership::ClusterMembership, @@ -3586,6 +2932,13 @@ impl Drop for ClearOnDrop<'_> { #[cfg(test)] mod tests { + /// Seconds since the epoch, for regions' `transfer_first_seen`. + fn now_unix() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_secs() as i64) + } + use super::*; use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::domain::cluster::membership::{ClusterMembership, NodeInfo, new_shared}; @@ -3821,8 +3174,8 @@ mod tests { use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::adapters::wal::rocksdb_wal::RocksDbWal; - use crate::application::cluster::bootstrap::ClusterBootstrap; use crate::application::materialized_view_service::MaterializedViewService; + use crate::composition::bootstrap::ClusterBootstrap; use crate::domain::sharding::{MeasurementKey, ShardLocationCache, ShardMapOp}; use crate::ports::points_sink::PointsSinkPort; @@ -3924,7 +3277,6 @@ mod tests { metadata, wal, None, - Some(sink), 99, sharding, 0, @@ -3980,8 +3332,8 @@ mod tests { use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::adapters::wal::rocksdb_wal::RocksDbWal; - use crate::application::cluster::bootstrap::ClusterBootstrap; use crate::application::materialized_view_service::MaterializedViewService; + use crate::composition::bootstrap::ClusterBootstrap; use crate::domain::sharding::{MeasurementKey, ShardLocationCache, ShardMapOp}; use crate::ports::points_sink::PointsSinkPort; @@ -4074,7 +3426,6 @@ mod tests { metadata, wal, None, - Some(sink), 1, sharding, 0, @@ -4120,8 +3471,8 @@ mod tests { use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::adapters::wal::rocksdb_wal::RocksDbWal; - use crate::application::cluster::bootstrap::ClusterBootstrap; use crate::application::materialized_view_service::MaterializedViewService; + use crate::composition::bootstrap::ClusterBootstrap; use crate::domain::sharding::{MeasurementKey, ShardLocationCache, ShardMapOp}; use crate::ports::points_sink::PointsSinkPort; @@ -4216,7 +3567,6 @@ mod tests { metadata, wal, None, - Some(sink), 1, sharding, 0, @@ -4258,93 +3608,7 @@ mod tests { ); } - // ── reconciliation queue (S.1–S.7) ────────────────────────────────── - - fn pending_entry(key: &MeasurementKey, region_id: u64, first_seen: i64) -> PendingTransfer { - PendingTransfer { - key: key.clone(), - region_id, - start: 0, - end: u64::MAX, - recorded_source: 1, - resolve_at_drain: false, - ticks_without_progress: 0, - parked: false, - first_seen, - } - } - - #[test] - fn classify_transfer_error_dispositions() { - use HyperbytedbError::{PeerUnreachable, ShardMap, TransferCollision, TransferRejected}; - assert_eq!( - classify_transfer_error(&PeerUnreachable("down".into())), - TransferDisposition::Retryable - ); - assert_eq!( - classify_transfer_error(&TransferRejected { - status: 404, - reason: "test".into() - }), - TransferDisposition::Retryable - ); - assert_eq!( - classify_transfer_error(&TransferRejected { - status: 409, - reason: "test".into() - }), - TransferDisposition::Retryable - ); - assert_eq!( - classify_transfer_error(&TransferRejected { - status: 503, - reason: "test".into() - }), - TransferDisposition::Retryable - ); - assert_eq!( - classify_transfer_error(&TransferRejected { - status: 400, - reason: "test".into(), - }), - TransferDisposition::Reresolve - ); - assert_eq!( - classify_transfer_error(&TransferCollision), - TransferDisposition::Reresolve - ); - assert_eq!( - classify_transfer_error(&TransferRejected { - status: 403, - reason: "test".into(), - }), - TransferDisposition::Park - ); - assert_eq!( - classify_transfer_error(&ShardMap("snapshot".into())), - TransferDisposition::Retryable - ); - } - - #[test] - fn enqueue_is_idempotent_per_key_and_region() { - // Exercise the queue merge semantics through a manually-built slice: - // identical (key, region_id) updates in place instead of duplicating. - let mut q: Vec = Vec::new(); - let key = MeasurementKey::new("db", "autogen", "cpu"); - let e1 = pending_entry(&key, 7, 100); - let e2 = pending_entry(&key, 7, 200); - q.push(e1); - if let Some(existing) = q.iter_mut().find(|e| e.key == key && e.region_id == 7) { - existing.recorded_source = e2.recorded_source; - existing.resolve_at_drain = false; - existing.parked = false; - } else { - q.push(e2); - } - assert_eq!(q.len(), 1); - assert_eq!(q[0].recorded_source, 1); - } + // ── split keys and transfer-debt refusal ──────────────────────────── #[test] fn split_key_prefers_the_peer_median_and_stays_inside_the_range() { @@ -4384,47 +3648,6 @@ mod tests { ); } - #[tokio::test] - #[serial_test::serial(chdb)] - async fn reconciliation_queue_is_inert_after_metadata_only_splits() { - let harness = ReconcileTestHarness::new().await; - let key = MeasurementKey::new("db", "autogen", "cpu"); - let old = now_unix() - RECONCILE_MIN_AGE_SECS - 10; - harness - .shard_map - .apply_op(ShardMapOp::BootstrapMeasurement { - key: key.clone(), - region: flagged_test_region(1, old), - }) - .await - .unwrap(); - - for _ in 0..=RECONCILE_PARK_AFTER_TICKS { - harness.scheduler.tick_once_for_test().await.unwrap(); - } - - let q = harness.scheduler.reconciliation_queue.lock().unwrap(); - assert!( - q.is_empty(), - "legacy reconciliation drain is disabled; flagged children are not enqueued" - ); - } - - #[tokio::test] - #[serial_test::serial(chdb)] - async fn rollup_destinations_are_excluded_from_reconciliation() { - let harness = ReconcileTestHarness::new().await; - let key = MeasurementKey::new("db", "autogen", "rollup_dest"); - harness.scheduler.seed_rollup_dest(&key).await; - - harness - .scheduler - .enqueue_reconciliation(pending_entry(&key, 3, now_unix())); - - let q = harness.scheduler.reconciliation_queue.lock().unwrap(); - assert!(q.is_empty(), "SummingMergeTree spaces must never enqueue"); - } - /// Placement stages rows and `apply_transfer_push` is not idempotent, so a /// placement that stages and then fails its proposal re-delivers on the /// next tick. `SummingMergeTree` destinations would sum the redelivery. @@ -4446,31 +3669,6 @@ mod tests { ); } - #[tokio::test] - #[serial_test::serial(chdb)] - async fn rebuild_skips_flagged_rollup_destinations() { - let harness = ReconcileTestHarness::new().await; - let key = MeasurementKey::new("db", "autogen", "rollup_dest"); - harness.scheduler.seed_rollup_dest(&key).await; - harness - .shard_map - .apply_op(ShardMapOp::BootstrapMeasurement { - key: key.clone(), - region: flagged_test_region(1, now_unix()), - }) - .await - .unwrap(); - - let map = harness.shard_map.snapshot().await.unwrap(); - harness.scheduler.rebuild_queue_from_map(&map); - - let q = harness.scheduler.reconciliation_queue.lock().unwrap(); - assert!( - q.is_empty(), - "flagged rollup dest must not enter the transfer queue on rebuild" - ); - } - #[tokio::test] #[serial_test::serial(chdb)] async fn split_and_merge_are_refused_while_transfer_outstanding() { @@ -4699,8 +3897,8 @@ mod tests { use crate::adapters::cluster::replication_log::ReplicationLog; use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; use crate::adapters::wal::rocksdb_wal::RocksDbWal; - use crate::application::cluster::bootstrap::ClusterBootstrap; use crate::application::materialized_view_service::MaterializedViewService; + use crate::composition::bootstrap::ClusterBootstrap; use crate::ports::points_sink::PointsSinkPort; let dir = tempfile::tempdir().unwrap(); @@ -4823,7 +4021,6 @@ mod tests { metadata, wal, None, - Some(sink), 1, sharding, 10_000, @@ -5243,8 +4440,8 @@ mod tests { use crate::adapters::chdb::session::SharedSession; use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; use crate::adapters::wal::rocksdb_wal::RocksDbWal; - use crate::application::cluster::bootstrap::ClusterBootstrap; use crate::application::materialized_view_service::MaterializedViewService; + use crate::composition::bootstrap::ClusterBootstrap; use crate::ports::points_sink::PointsSinkPort; let dir = tempfile::tempdir().unwrap(); @@ -5308,7 +4505,6 @@ mod tests { metadata, wal, None, - Some(sink), 99, sharding, 0, diff --git a/hyperbytedb/src/application/sharded_mv_backfill.rs b/hyperbytedb/src/application/sharded_mv_backfill.rs index d01a6c4..11f2593 100644 --- a/hyperbytedb/src/application/sharded_mv_backfill.rs +++ b/hyperbytedb/src/application/sharded_mv_backfill.rs @@ -12,7 +12,6 @@ use crate::error::HyperbytedbError; use crate::ports::metadata::MetadataPort; use crate::ports::points_sink::PointsSinkPort; use crate::ports::query::QueryPort; -use crate::ports::sharding::ShardMapPort; /// Borrowed measurement identity within a database and retention policy. #[derive(Clone, Copy)] diff --git a/hyperbytedb/src/bootstrap.rs b/hyperbytedb/src/bootstrap.rs index 727363d..fd7eb91 100644 --- a/hyperbytedb/src/bootstrap.rs +++ b/hyperbytedb/src/bootstrap.rs @@ -9,7 +9,6 @@ use crate::adapters::metadata::rocksdb_meta::RocksDbMetadata; use crate::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use crate::adapters::wal::batching_wal::BatchingWal; use crate::adapters::wal::rocksdb_wal::{RocksDbWal, RocksDbWalOptions}; -use crate::application::cluster::bootstrap::ClusterBootstrap; use crate::application::cluster::drain::DrainService; use crate::application::disk_monitor::{self, DiskMonitorPaths}; use crate::application::flush_service::FlushServiceImpl; @@ -22,6 +21,7 @@ use crate::application::runtime::RegionWriteStats; use crate::application::runtime_settings as rs; use crate::application::shard_routing::ShardRoutingContext; use crate::application::statement_summary::StatementSummary; +use crate::composition::bootstrap::ClusterBootstrap; use crate::config::HyperbytedbConfig; use crate::domain::sharding::ShardLocationCache; use crate::ports::metadata::MetadataPort; diff --git a/hyperbytedb/src/application/cluster/bootstrap.rs b/hyperbytedb/src/composition/bootstrap.rs similarity index 100% rename from hyperbytedb/src/application/cluster/bootstrap.rs rename to hyperbytedb/src/composition/bootstrap.rs diff --git a/hyperbytedb/src/application/cluster/leader_monitor.rs b/hyperbytedb/src/composition/leader_monitor.rs similarity index 100% rename from hyperbytedb/src/application/cluster/leader_monitor.rs rename to hyperbytedb/src/composition/leader_monitor.rs diff --git a/hyperbytedb/src/composition/mod.rs b/hyperbytedb/src/composition/mod.rs new file mode 100644 index 0000000..a2f9aee --- /dev/null +++ b/hyperbytedb/src/composition/mod.rs @@ -0,0 +1,7 @@ +//! Composition: builds the concrete adapters and wires them into the +//! application. Outside the hexagon's layers, like `bootstrap.rs` and +//! `server.rs`, so it may name any of them. + +pub mod bootstrap; +pub mod leader_monitor; +pub mod raft_formation; diff --git a/hyperbytedb/src/application/cluster/raft_formation.rs b/hyperbytedb/src/composition/raft_formation.rs similarity index 100% rename from hyperbytedb/src/application/cluster/raft_formation.rs rename to hyperbytedb/src/composition/raft_formation.rs diff --git a/hyperbytedb/src/config.rs b/hyperbytedb/src/config.rs index 84c4214..6a959c1 100644 --- a/hyperbytedb/src/config.rs +++ b/hyperbytedb/src/config.rs @@ -102,9 +102,9 @@ pub struct ShardingConfig { /// (replica-set healing). Disable to manage replica sets manually. #[serde(default = "default_peer_heal_enabled")] pub peer_heal_enabled: bool, - /// Propose `ClearVerified` shard-map ops once split-transfer movement - /// verifies. Disable on mixed-version clusters: nodes running builds - /// older than the `ClearVerified` op cannot decode it from the Raft log. + /// No effect: it gated `ClearVerified` proposals from the split-transfer + /// reconciliation, which has been removed. Still accepted so existing + /// config files load. #[serde(default = "default_transfer_clear_proposals_enabled")] pub transfer_clear_proposals_enabled: bool, /// Propose `AddPeer` shard-map ops to place a joiner as a region replica. @@ -1606,3 +1606,21 @@ mod admission_tests { assert_eq!(resolve_max_inflight_writes(0, 0), INFLIGHT_WRITES_PER_CPU); } } + +#[cfg(test)] +mod legacy_config_tests { + /// The split-transfer reconciliation that read this key is gone, but + /// existing config files still set it and must keep loading. + #[test] + fn old_config_with_transfer_clear_proposals_still_loads() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + std::fs::write( + &path, + "[sharding]\ntransfer_clear_proposals_enabled = false\n", + ) + .unwrap(); + let cfg = super::HyperbytedbConfig::load(Some(path.to_str().unwrap())).unwrap(); + assert!(!cfg.sharding.transfer_clear_proposals_enabled); + } +} diff --git a/hyperbytedb/src/domain/chdb_naming.rs b/hyperbytedb/src/domain/chdb_naming.rs index dab0c43..08f81c0 100644 --- a/hyperbytedb/src/domain/chdb_naming.rs +++ b/hyperbytedb/src/domain/chdb_naming.rs @@ -23,12 +23,6 @@ use crate::error::HyperbytedbError; pub struct SanitizedIdent(String); impl SanitizedIdent { - /// Sanitize arbitrary input into a valid ClickHouse identifier. - #[must_use] - pub fn sanitize(input: &str) -> Self { - Self(sanitise_ident(input)) - } - #[must_use] pub fn as_str(&self) -> &str { &self.0 @@ -113,6 +107,37 @@ pub fn unquoted_table_name(db: &str, rp: &str, measurement: &str) -> SanitizedId )) } +/// Append `s` to `out` escaped for the inside of a ClickHouse string +/// literal: `\` and `'` are backslash-escaped, `\n` `\r` `\t` are written +/// as escape sequences and other control characters as `\xNN`. +pub fn push_string_literal_body(out: &mut String, s: &str) { + use std::fmt::Write as _; + for ch in s.chars() { + match ch { + '\\' => out.push_str("\\\\"), + '\'' => out.push_str("\\'"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + c if (c as u32) < 0x20 => { + let _ = write!(out, "\\x{:02x}", c as u32); + } + c => out.push(c), + } + } +} + +/// `s` as a single-quoted ClickHouse string literal; see +/// [`push_string_literal_body`]. +#[must_use] +pub fn string_literal(s: &str) -> String { + let mut out = String::with_capacity(s.len() + 2); + out.push('\''); + push_string_literal_body(&mut out, s); + out.push('\''); + out +} + /// Quote an identifier with backticks, escaping embedded backticks. /// Used for table names, column names, and database references in /// generated SQL. @@ -231,6 +256,16 @@ mod tests { ); } + #[test] + fn string_literal_escapes_quotes_backslashes_and_controls() { + assert_eq!(string_literal("a'b\\c"), "'a\\'b\\\\c'"); + assert_eq!(string_literal("x\ty\n"), "'x\\ty\\n'"); + assert_eq!(string_literal("\u{1}"), "'\\x01'"); + let mut body = String::new(); + push_string_literal_body(&mut body, "it's"); + assert_eq!(body, "it\\'s"); + } + #[test] fn quote_backticks_escapes_embedded_backticks() { assert_eq!(quote_backticks("a`b"), "`a``b`"); diff --git a/hyperbytedb/src/domain/cluster/mod.rs b/hyperbytedb/src/domain/cluster/mod.rs index ff26cdc..d293ab1 100644 --- a/hyperbytedb/src/domain/cluster/mod.rs +++ b/hyperbytedb/src/domain/cluster/mod.rs @@ -1,5 +1,6 @@ pub mod forward; pub mod membership; +pub mod raft_request; pub mod replication_wire; pub mod sync; pub mod types; diff --git a/hyperbytedb/src/domain/cluster/raft_request.rs b/hyperbytedb/src/domain/cluster/raft_request.rs new file mode 100644 index 0000000..eb24447 --- /dev/null +++ b/hyperbytedb/src/domain/cluster/raft_request.rs @@ -0,0 +1,88 @@ +//! Requests that go through Raft consensus and their responses. Their JSON +//! is the Raft log's wire format: keep it stable across versions. + +use serde::{Deserialize, Serialize}; + +use crate::domain::cluster::membership::NodeState; +use crate::domain::cluster::types::MutationRequest; +use crate::domain::sharding::ShardMapOp; + +/// Application-level request that goes through Raft consensus. +/// Only cluster coordination and schema DDL use Raft -- high-throughput +/// time-series writes stay on the existing fan-out data plane. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub enum ClusterRequest { + SetNodeState { node_id: u64, state: NodeState }, + SchemaMutation(Box), + ShardMapMutation(Box), +} + +/// Response returned after a ClusterRequest is committed and applied. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ClusterResponse { + pub ok: bool, + pub message: Option, +} + +impl ClusterResponse { + pub fn success() -> Self { + Self { + ok: true, + message: None, + } + } + + pub fn error(msg: impl Into) -> Self { + Self { + ok: false, + message: Some(msg.into()), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::domain::cluster::membership::NodeState; + use crate::domain::cluster::types::MutationRequest; + use crate::domain::sharding::ShardMapOp; + + /// Raft log entries written by older nodes must still decode: the JSON + /// of every variant is pinned to what it was before the types moved + /// out of the Raft adapter. + #[test] + fn cluster_request_json_is_unchanged() { + let cases = [ + ( + ClusterRequest::SetNodeState { + node_id: 3, + state: NodeState::Active, + }, + r#"{"SetNodeState":{"node_id":3,"state":"Active"}}"#, + ), + ( + ClusterRequest::SchemaMutation(Box::new(MutationRequest::DropDatabase( + "db".into(), + ))), + r#"{"SchemaMutation":{"DropDatabase":"db"}}"#, + ), + ( + ClusterRequest::ShardMapMutation(Box::new(ShardMapOp::DropSpace { + db: "db".into(), + rp: Some("autogen".into()), + measurement: None, + })), + r#"{"ShardMapMutation":{"DropSpace":{"db":"db","rp":"autogen","measurement":null}}}"#, + ), + ]; + for (req, json) in cases { + assert_eq!(serde_json::to_string(&req).unwrap(), json); + let back: ClusterRequest = serde_json::from_str(json).unwrap(); + assert_eq!(serde_json::to_string(&back).unwrap(), json); + } + assert_eq!( + serde_json::to_string(&ClusterResponse::error("x")).unwrap(), + r#"{"ok":false,"message":"x"}"# + ); + } +} diff --git a/hyperbytedb/src/domain/columnar_batch.rs b/hyperbytedb/src/domain/columnar_batch.rs new file mode 100644 index 0000000..49984b9 --- /dev/null +++ b/hyperbytedb/src/domain/columnar_batch.rs @@ -0,0 +1,37 @@ +//! The columnar MessagePack write batch (format v1): one measurement, one +//! float field, shared tags. A wire format the ingest path decodes and the +//! chDB sink writes, so it lives in the domain. + +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; + +/// Wire map for columnar msgpack v1 (MessagePack map, not a row array). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ColumnarMsgpackBatch { + pub measurement: String, + #[serde(default)] + pub tags: BTreeMap, + pub field: String, + pub values: Vec, + pub timestamps: Option>, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn batch_round_trips_through_msgpack() { + let batch = ColumnarMsgpackBatch { + measurement: "cpu".into(), + tags: [("host".to_string(), "a".to_string())].into(), + field: "v".into(), + values: vec![1.0, 2.0], + timestamps: Some(vec![1, 2]), + }; + let bytes = rmp_serde::to_vec_named(&batch).unwrap(); + let back: ColumnarMsgpackBatch = rmp_serde::from_slice(&bytes).unwrap(); + assert_eq!(back.values, vec![1.0, 2.0]); + assert_eq!(back.tags["host"], "a"); + } +} diff --git a/hyperbytedb/src/domain/field_type.rs b/hyperbytedb/src/domain/field_type.rs index 13a06c0..3b11a4d 100644 --- a/hyperbytedb/src/domain/field_type.rs +++ b/hyperbytedb/src/domain/field_type.rs @@ -51,12 +51,6 @@ pub fn merge_field_type_map( out } -/// Widen a single incoming discriminant into `stored`, returning the new value. -#[must_use] -pub fn merge_field_disc(stored: u8, incoming: u8) -> u8 { - widen_field_disc(stored, incoming).unwrap_or(stored) -} - #[cfg(test)] mod tests { use super::*; diff --git a/hyperbytedb/src/domain/measurement.rs b/hyperbytedb/src/domain/measurement.rs index cf9e94e..0df89fc 100644 --- a/hyperbytedb/src/domain/measurement.rs +++ b/hyperbytedb/src/domain/measurement.rs @@ -27,6 +27,15 @@ pub struct MeasurementMeta { } impl MeasurementMeta { + /// Whether a raw write to this measurement in `rp` is refused: it is a + /// materialized-view destination there, which only its view fills. + /// `materialized_rp: None` (entries from before it was recorded) refuses + /// every retention policy. + #[must_use] + pub fn refuses_raw_writes(&self, rp: &str) -> bool { + self.materialized && self.materialized_rp.as_deref().is_none_or(|mr| mr == rp) + } + pub fn field_types_as_tuples(&self) -> Vec<(String, u8)> { self.field_types .iter() @@ -34,3 +43,28 @@ impl MeasurementMeta { .collect() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn refuses_raw_writes_only_for_its_own_rp() { + let plain = MeasurementMeta::default(); + assert!(!plain.refuses_raw_writes("autogen")); + let dest = MeasurementMeta { + materialized: true, + materialized_rp: Some("rollup".into()), + ..Default::default() + }; + assert!(dest.refuses_raw_writes("rollup")); + assert!(!dest.refuses_raw_writes("autogen")); + let legacy = MeasurementMeta { + materialized: true, + materialized_rp: None, + ..Default::default() + }; + assert!(legacy.refuses_raw_writes("autogen")); + assert!(legacy.refuses_raw_writes("rollup")); + } +} diff --git a/hyperbytedb/src/domain/mod.rs b/hyperbytedb/src/domain/mod.rs index 6a9c8a5..99c715b 100644 --- a/hyperbytedb/src/domain/mod.rs +++ b/hyperbytedb/src/domain/mod.rs @@ -2,6 +2,7 @@ pub mod arrow_coalesce; pub mod chdb_naming; pub mod cluster; pub mod column_mapping; +pub mod columnar_batch; pub mod continuous_query; pub mod cq_schedule; pub mod database; @@ -10,6 +11,7 @@ pub mod materialized_view; pub mod measurement; pub mod point; pub mod point_coalesce; +pub mod point_codec; pub mod prepared_wal; pub mod query_result; pub mod rollup; diff --git a/hyperbytedb/src/domain/point.rs b/hyperbytedb/src/domain/point.rs index e52dd0f..5459211 100644 --- a/hyperbytedb/src/domain/point.rs +++ b/hyperbytedb/src/domain/point.rs @@ -11,16 +11,6 @@ pub enum FieldValue { } impl FieldValue { - pub fn type_name(&self) -> &'static str { - match self { - FieldValue::Float(_) => "float", - FieldValue::Integer(_) => "integer", - FieldValue::UInteger(_) => "unsigned", - FieldValue::String(_) => "string", - FieldValue::Boolean(_) => "boolean", - } - } - pub fn type_discriminant(&self) -> u8 { match self { FieldValue::Float(_) => 0, diff --git a/hyperbytedb/src/domain/point_codec.rs b/hyperbytedb/src/domain/point_codec.rs new file mode 100644 index 0000000..d10cf4d --- /dev/null +++ b/hyperbytedb/src/domain/point_codec.rs @@ -0,0 +1,40 @@ +//! The binary point frame used node to node: `magic | u32 LE length | +//! bincode(Vec)`. A wire format, so it lives with the point model; +//! the WAL batcher and the peer client use it as well as ingestion. + +/// Start of a binary points frame. Line protocol never starts with NUL, so a +/// body is binary exactly when it starts with this. +pub const POINTS_FRAME_MAGIC: &[u8; 4] = b"\0HBP"; + +/// Whether `body` is binary points frames rather than line protocol. +#[must_use] +pub fn is_binary_points(body: &[u8]) -> bool { + body.starts_with(POINTS_FRAME_MAGIC) +} + +/// Starting buffer size per point for one-pass bincode encoding: a point with +/// a few short tags and one field. Only a hint; the buffer grows as needed. +pub const POINT_ENCODED_SIZE_HINT: usize = 96; + +/// Append `value`'s bincode encoding to `out` in one pass. Same bytes as +/// `bincode::serialize`, without its separate sizing pass. +pub fn encode_bincode_into( + out: &mut Vec, + value: &T, +) -> Result<(), bincode::Error> { + bincode::serialize_into(out, value) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn frames_are_recognised_by_their_magic() { + assert!(is_binary_points(b"\0HBP\0\0\0\0")); + assert!(!is_binary_points(b"cpu v=1 1")); + let mut out = Vec::new(); + encode_bincode_into(&mut out, &vec![1u32, 2]).unwrap(); + assert_eq!(out, bincode::serialize(&vec![1u32, 2]).unwrap()); + } +} diff --git a/hyperbytedb/src/domain/sharding/location_cache.rs b/hyperbytedb/src/domain/sharding/location_cache.rs index d360b6a..39d22d3 100644 --- a/hyperbytedb/src/domain/sharding/location_cache.rs +++ b/hyperbytedb/src/domain/sharding/location_cache.rs @@ -169,11 +169,6 @@ impl ShardLocationCache { } } - pub fn invalidate_all(&self) { - let mut inner = self.write_inner(); - inner.by_measurement.clear(); - } - pub fn invalidate_measurement(&self, key: &MeasurementKey) { let mut inner = self.write_inner(); inner diff --git a/hyperbytedb/src/domain/sharding/ops.rs b/hyperbytedb/src/domain/sharding/ops.rs index 3cae7c5..fc082fb 100644 --- a/hyperbytedb/src/domain/sharding/ops.rs +++ b/hyperbytedb/src/domain/sharding/ops.rs @@ -342,10 +342,10 @@ pub fn apply_shard_map_op( "split regions must meet at split_key".into(), )); } - // Apply-time normalization (durable reconciliation intent): flag - // every child whose primary differs from the parent's — that - // child's historical rows must be re-pushed and verified before - // the debt clears. `transfer_first_seen` propagates from the + // Apply-time normalization: flag every child whose primary differs + // from the parent's (a metadata-only split never does). Nothing + // clears the flag any more: the reconciliation that re-pushed + // and verified flagged children was removed. `transfer_first_seen` propagates from the // parent so re-splitting a flagged region cannot reset the age // gate. Predicates are deterministic from (op, prior map), so all // replicas normalize identically. diff --git a/hyperbytedb/src/domain/user.rs b/hyperbytedb/src/domain/user.rs index 2c4f861..5772628 100644 --- a/hyperbytedb/src/domain/user.rs +++ b/hyperbytedb/src/domain/user.rs @@ -66,8 +66,34 @@ impl StoredUser { } } +/// A salted argon2 hash of `password`, as stored for users. +/// +/// # Errors +/// `Internal` if hashing fails. +pub fn hash_password(password: &str) -> Result { + use argon2::Argon2; + use argon2::password_hash::{PasswordHasher, SaltString}; + let salt = SaltString::generate(&mut argon2::password_hash::rand_core::OsRng); + let hash = Argon2::default() + .hash_password(password.as_bytes(), &salt) + .map_err(|e| { + crate::error::HyperbytedbError::Internal(format!("password hash failed: {e}").into()) + })?; + Ok(hash.to_string()) +} + #[cfg(test)] mod tests { + + #[test] + fn hash_password_is_a_salted_argon2_hash() { + let a = super::hash_password("secret").unwrap(); + let b = super::hash_password("secret").unwrap(); + assert!(a.starts_with("$argon2"), "{a}"); + assert_ne!(a, b, "each hash has its own salt"); + assert!(!a.contains("secret")); + } + use super::DatabasePrivilege::{self, All, Read, Write}; #[test] diff --git a/hyperbytedb/src/lib.rs b/hyperbytedb/src/lib.rs index 7bbdb77..0884423 100644 --- a/hyperbytedb/src/lib.rs +++ b/hyperbytedb/src/lib.rs @@ -4,6 +4,7 @@ pub mod adapters; pub mod application; pub mod bootstrap; +pub mod composition; pub mod config; pub mod domain; pub mod error; diff --git a/hyperbytedb/src/ports/leadership.rs b/hyperbytedb/src/ports/leadership.rs new file mode 100644 index 0000000..51988b2 --- /dev/null +++ b/hyperbytedb/src/ports/leadership.rs @@ -0,0 +1,40 @@ +//! Who leads the cluster, for services that only act on the leader. + +/// The cluster's current leader, as this node sees it. +pub trait LeadershipPort: Send + Sync { + /// The leader's node id, if one is known. + fn current_leader(&self) -> Option; + + /// Whether `node_id` is the leader. + fn is_leader(&self, node_id: u64) -> bool { + self.current_leader() == Some(node_id) + } +} + +/// Whether a leader-only job (retention, continuous queries) may run on +/// `node_id`. With no consensus (`None`, a single node) it always may. +#[must_use] +pub fn may_run(leadership: Option<&dyn LeadershipPort>, node_id: u64) -> bool { + leadership.is_none_or(|l| l.is_leader(node_id)) +} + +#[cfg(test)] +mod tests { + use super::*; + + struct LeaderIs(u64); + impl LeadershipPort for LeaderIs { + fn current_leader(&self) -> Option { + Some(self.0) + } + } + + #[test] + fn retention_and_cq_gate_on_leadership() { + let stub = LeaderIs(2); + assert!(!may_run(Some(&stub), 1)); + assert!(may_run(Some(&stub), 2)); + // Single node: no consensus, always runs. + assert!(may_run(None, 1)); + } +} diff --git a/hyperbytedb/src/ports/mod.rs b/hyperbytedb/src/ports/mod.rs index b18f8e8..b9e6208 100644 --- a/hyperbytedb/src/ports/mod.rs +++ b/hyperbytedb/src/ports/mod.rs @@ -2,10 +2,12 @@ pub mod auth; pub mod flush; pub mod forward_queue; pub mod ingestion; +pub mod leadership; pub mod metadata; pub mod peer_http; pub mod points_sink; pub mod query; pub mod replication; +pub mod replication_acks; pub mod sharding; pub mod wal; diff --git a/hyperbytedb/src/ports/points_sink.rs b/hyperbytedb/src/ports/points_sink.rs index bf979c3..30b1ae6 100644 --- a/hyperbytedb/src/ports/points_sink.rs +++ b/hyperbytedb/src/ports/points_sink.rs @@ -60,7 +60,7 @@ pub trait PointsSinkPort: Send + Sync { _db: &str, _rp: &str, _origin_node_id: u64, - _wire: &crate::application::columnar_msgpack::ColumnarMsgpackBatch, + _wire: &crate::domain::columnar_batch::ColumnarMsgpackBatch, _precision: Option<&str>, ) -> Result { Err(HyperbytedbError::Internal( diff --git a/hyperbytedb/src/ports/replication_acks.rs b/hyperbytedb/src/ports/replication_acks.rs new file mode 100644 index 0000000..18c6edb --- /dev/null +++ b/hyperbytedb/src/ports/replication_acks.rs @@ -0,0 +1,37 @@ +//! Peers' replication acknowledgements, as the flush and drain paths read +//! them: how far each peer has acknowledged this node's WAL and mutation log. + +use crate::error::HyperbytedbError; + +/// What the flush and drain paths read from the replication log. +pub trait ReplicationAcks: Send + Sync { + /// How far `peer_id` has acknowledged this node's WAL. + fn get_wal_ack(&self, peer_id: u64) -> Result; + /// The lowest and highest WAL acknowledgement among `peer_ids`. + fn min_max_wal_ack_for_peers(&self, peer_ids: &[u64]) -> Result<(u64, u64), HyperbytedbError>; + /// How far `peer_id` has acknowledged this node's mutation log. + fn get_mutation_ack(&self, peer_id: u64) -> Result; + /// The sequence of the last mutation this node logged. + fn last_mutation_seq(&self) -> u64; + /// The lowest mutation acknowledgement across peers, if any. + fn min_mutation_ack(&self) -> Result, HyperbytedbError>; + /// Drop logged mutations before `seq`. + fn truncate_mutations_before(&self, seq: u64) -> Result<(), HyperbytedbError>; +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::adapters::cluster::replication_log::ReplicationLog; + use std::sync::Arc; + + #[test] + fn replication_log_answers_through_the_port() { + let dir = tempfile::tempdir().unwrap(); + let log: Arc = + Arc::new(ReplicationLog::open(dir.path().join("repl")).unwrap()); + assert_eq!(log.last_mutation_seq(), 0); + assert_eq!(log.get_wal_ack(7).unwrap(), 0); + assert_eq!(log.min_mutation_ack().unwrap(), None); + } +} diff --git a/hyperbytedb/src/ports/sharding.rs b/hyperbytedb/src/ports/sharding.rs index dc11bd0..b6555de 100644 --- a/hyperbytedb/src/ports/sharding.rs +++ b/hyperbytedb/src/ports/sharding.rs @@ -93,5 +93,3 @@ impl ShardMapPort for DisabledShardMap { Ok(true) } } - -pub type SharedShardMapPort = Arc; diff --git a/hyperbytedb/src/server.rs b/hyperbytedb/src/server.rs index 9c936c7..ed81894 100644 --- a/hyperbytedb/src/server.rs +++ b/hyperbytedb/src/server.rs @@ -8,12 +8,12 @@ use std::time::Duration; use crate::adapters::http::router::{build_control_router, build_router}; use crate::application::cluster::heartbeat; use crate::application::cluster::hinted_handoff; -use crate::application::cluster::leader_monitor; -use crate::application::cluster::raft_formation; use crate::application::continuous_query_service::ContinuousQueryService; use crate::application::disk_monitor; use crate::application::retention_service::RetentionService; use crate::bootstrap::build_services; +use crate::composition::leader_monitor; +use crate::composition::raft_formation; use crate::config::{HyperbytedbConfig, RetentionConfig}; use crate::ports::metadata::MetadataPort; use crate::ports::points_sink::PointsSinkPort; @@ -234,7 +234,6 @@ async fn serve_inner( app_state.metadata.clone(), app_state.wal.clone(), Some(app_state.query_port.clone()), - Some(app_state.points_sink.clone()), config.cluster.node_id, config.sharding.clone(), config.server.max_points_per_request, @@ -310,7 +309,7 @@ async fn serve_inner( pc, m, map, - raft, + Arc::new(raft), meta, query_port, region_write_stats, @@ -340,7 +339,10 @@ async fn serve_inner( let cq_service = Arc::new(ContinuousQueryService::new( app_state.metadata.clone(), app_state.query.clone(), - app_state.raft.clone(), + app_state + .raft + .clone() + .map(|r| Arc::new(r) as Arc), config.cluster.node_id, )); let rx = service_shutdown_rx.clone(); @@ -366,7 +368,10 @@ async fn serve_inner( RetentionService::new( app_state.metadata.clone(), app_state.query_port.clone(), - app_state.raft.clone(), + app_state + .raft + .clone() + .map(|r| Arc::new(r) as Arc), config.cluster.node_id, ) .with_sharding(config.sharding.enabled, app_state.shard_map.clone()), diff --git a/hyperbytedb/src/timeseriesql/to_clickhouse/coalesce.rs b/hyperbytedb/src/timeseriesql/to_clickhouse/coalesce.rs index 13e0d30..ed96ea4 100644 --- a/hyperbytedb/src/timeseriesql/to_clickhouse/coalesce.rs +++ b/hyperbytedb/src/timeseriesql/to_clickhouse/coalesce.rs @@ -21,15 +21,6 @@ pub(super) fn build_coalesced_fact_view_filtered( build_coalesced_fact_view_impl(fact_table, mapping, false, filter) } -/// Like [`build_coalesced_fact_view`], but preserves `ingest_seq` / `origin_node_id` for -/// downstream aggregates (materialized view source dedup). -pub fn build_coalesced_fact_view_with_row_meta( - fact_table: &QuotedTableName, - mapping: &ColumnMapping, -) -> String { - build_coalesced_fact_view_impl(fact_table, mapping, true, None) -} - pub(super) fn build_coalesced_fact_view_impl( fact_table: &QuotedTableName, mapping: &ColumnMapping, diff --git a/hyperbytedb/src/timeseriesql/to_clickhouse/conditions.rs b/hyperbytedb/src/timeseriesql/to_clickhouse/conditions.rs index 3357bd8..b5b72ae 100644 --- a/hyperbytedb/src/timeseriesql/to_clickhouse/conditions.rs +++ b/hyperbytedb/src/timeseriesql/to_clickhouse/conditions.rs @@ -401,7 +401,7 @@ pub(super) fn quote_phys_identifier(name: &str) -> String { } pub(super) fn quote_string(s: &str) -> String { - format!("'{}'", s.replace('\\', "\\\\").replace('\'', "\\'")) + crate::domain::chdb_naming::string_literal(s) } pub(super) fn format_float(f: f64) -> String { diff --git a/hyperbytedb/src/timeseriesql/to_clickhouse/mod.rs b/hyperbytedb/src/timeseriesql/to_clickhouse/mod.rs index 46bd164..bf1c09f 100644 --- a/hyperbytedb/src/timeseriesql/to_clickhouse/mod.rs +++ b/hyperbytedb/src/timeseriesql/to_clickhouse/mod.rs @@ -18,7 +18,7 @@ pub use rename::rename_time_bucket_alias; pub use selectors::{BUCKET_COL, MultiSelector, SelectorKind, TIME_NS_COL, multi_selector}; pub use time_bounds::extract_time_bounds; -pub use coalesce::{build_coalesced_fact_view, build_coalesced_fact_view_with_row_meta}; +pub use coalesce::build_coalesced_fact_view; pub use conditions::translate_condition; pub use materialized_view::{ build_create_fact_materialized_view, build_create_series_materialized_view, diff --git a/hyperbytedb/tests/common/sharding_cluster.rs b/hyperbytedb/tests/common/sharding_cluster.rs index e927137..f59a042 100644 --- a/hyperbytedb/tests/common/sharding_cluster.rs +++ b/hyperbytedb/tests/common/sharding_cluster.rs @@ -276,7 +276,6 @@ pub async fn start_sharded_node( metadata.clone(), wal.clone(), Some(chdb_adapter.clone() as Arc), - Some(sink.clone()), node_id, opts.sharding.clone(), 0, diff --git a/hyperbytedb/tests/compat/query_tests.rs b/hyperbytedb/tests/compat/query_tests.rs index 90be06c..3dd12b4 100644 --- a/hyperbytedb/tests/compat/query_tests.rs +++ b/hyperbytedb/tests/compat/query_tests.rs @@ -1463,3 +1463,41 @@ async fn where_clause_with_thousands_of_or_terms() { ); assert!(!hyperbytedb::adapters::chdb::engine_failed()); } + +/// A tag value with a control character must match in WHERE once the +/// ClickHouse string escapers are one: the native writer escaped `\t` while +/// the query translator wrote it raw. +#[tokio::test] +#[serial(chdb)] +async fn tag_value_with_a_tab_round_trips() { + let ctx = TestContext::new().unwrap(); + ctx.metadata.create_database("testdb").await.unwrap(); + // Line protocol refuses a tab in a tag value; msgpack carries it. + #[derive(serde::Serialize)] + struct Point { + measurement: &'static str, + tags: std::collections::BTreeMap<&'static str, &'static str>, + fields: std::collections::BTreeMap<&'static str, hyperbytedb::domain::point::FieldValue>, + timestamp: Option, + } + let body = rmp_serde::to_vec_named(&vec![Point { + measurement: "cpu", + tags: [("host", "a\tb")].into(), + fields: [("v", hyperbytedb::domain::point::FieldValue::Float(1.0))].into(), + timestamp: Some(1_000_000_000), + }]) + .unwrap(); + use hyperbytedb::ports::ingestion::{IngestionPort, WritePayloadFormat}; + ctx.ingestion + .ingest("testdb", None, None, &body, WritePayloadFormat::Msgpack) + .await + .unwrap(); + ctx.flush_service.flush().await.unwrap(); + let resp = ctx + .query("testdb", "SELECT v FROM cpu WHERE host = 'a\tb'") + .await + .unwrap(); + let stmt = &resp.results[0]; + assert!(stmt.error.is_none(), "{:?}", stmt.error); + assert_eq!(stmt.series.as_ref().map_or(0, |s| s[0].values.len()), 1); +} diff --git a/hyperbytedb/tests/sharding_failover_integration.rs b/hyperbytedb/tests/sharding_failover_integration.rs index 9267dc6..c64e65c 100644 --- a/hyperbytedb/tests/sharding_failover_integration.rs +++ b/hyperbytedb/tests/sharding_failover_integration.rs @@ -10,9 +10,9 @@ use hyperbytedb::adapters::chdb::session::SharedSession; use hyperbytedb::adapters::metadata::rocksdb_meta::RocksDbMetadata; use hyperbytedb::adapters::sharding::rocksdb_shard_map::RocksDbShardMap; use hyperbytedb::adapters::wal::rocksdb_wal::RocksDbWal; -use hyperbytedb::application::cluster::bootstrap::ClusterBootstrap; use hyperbytedb::application::materialized_view_service::MaterializedViewService; use hyperbytedb::application::shard_scheduler::ShardScheduler; +use hyperbytedb::composition::bootstrap::ClusterBootstrap; use hyperbytedb::config::ClusterConfig; use hyperbytedb::domain::cluster::membership::NodeState; use hyperbytedb::domain::sharding::{ @@ -122,7 +122,6 @@ async fn primary_failover_updates_shard_map_after_unhealthy_timeout() { metadata, wal, None, - Some(sink), 1, sharding, 0,