Skip to content

Commit 6ca9f86

Browse files
committed
fix(codex+coderabbit): dn_redis is a key-shape protocol, not wire-protocol emulation
Four review findings on PR #455 walked back, all real: Codex P2 1 - Redis Cluster is not plug-and-play. The walk_to_root / mget_dns operations use MGET, but the current key layout (ada:dn:{hex}) has no shared hash tag. Redis Cluster requires multi-key ops to belong to the same hash slot. So external Redis CLUSTER is NOT a valid backend without re-keying with hash tags. Added explicit caveat. Codex P2 2 - dn_redis is NOT wire-protocol emulation. The module exports Rust String keys + a RedisCommand enum + RedisPipeline. There is no RESP encoder, no parser, no listener, no command executor. Existing Redis clients cannot communicate with it. The FalkorDB / KuzuDB precedent does NOT apply (those projects implement the actual RESP wire protocol; dn_redis does not). Reframed the doc to "key-shape protocol + Rust command type model" - which is what the module actually is. Codex P2 3 - There is no dn_redis::Backend trait to implement. The "Recommended consumer pattern" first step cited a nonexistent API. Rewrote the section to show what consumer code actually has to provide: a struct + execute function that takes RedisCommand and runs it against the chosen backend. The example is illustrative (not a shipped trait). CodeRabbit minor - Same critique class. The speculative API wording is gone. Net effect: the doc now accurately describes what dn_redis IS (key-shape + command type model) and what adopters have to provide (an executor). Two valid backend categories enumerated honestly: (A) standalone Redis with optional hash-tag re-keying for clustered deployments; (B) in-binary executor over Lance via DataFusion. File renamed: DN_REDIS_EMULATION.md -> DN_REDIS_KEY_SHAPE_PROTOCOL.md to match the corrected framing. Provenance: codex P2 + coderabbit minor on PR #455 commit aac8121.
1 parent aac8121 commit 6ca9f86

2 files changed

Lines changed: 92 additions & 61 deletions

File tree

docs/DN_REDIS_EMULATION.md

Lines changed: 0 additions & 61 deletions
This file was deleted.
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
# dn_redis is a key-shape protocol + Rust command type model — not a Redis service dep
2+
3+
## TL;DR
4+
5+
`lance-graph-cognitive::container_bs::dn_redis` is a **key-shape protocol definition + a Rust-side command type model** for HHTL-keyed hot lookups. It is NOT:
6+
7+
- A network-protocol implementation (no RESP encoder/parser, no listener, no command executor)
8+
- A drop-in replacement that lets existing Redis clients talk to lance-graph
9+
- A wire-protocol emulator (the FalkorDB / KùzuDB precedent does NOT apply here — those projects implement the Redis RESP protocol; dn_redis does not)
10+
11+
What dn_redis IS:
12+
13+
- A **Rust API** that exports key-construction helpers (`dn_key`, `spine_key`, `walk_to_root_keys`, `children_pattern`, `subtree_pattern`) producing `String` keys like `ada:dn:{hex}` and `ada:spine:{hex}`
14+
- A **command type model** (`RedisCommand` enum + `RedisPipeline` struct) that adopters can populate to describe the operations they need to execute
15+
- A **serde layer** for `CogRecord` payloads (`cog_record_to_bytes`, `cog_record_from_bytes`)
16+
17+
## What is missing — and what adopters have to provide
18+
19+
The module exports the SHAPE of the operations; adopters provide the EXECUTOR. There is no `Backend` trait, no listener, no `connect()` function, no `execute()` method on `RedisPipeline` — those are what consumer code must implement.
20+
21+
To use dn_redis, a consumer must:
22+
23+
1. Decide on a backend (see below)
24+
2. Write an executor that takes `RedisPipeline` (or individual `RedisCommand` values) and runs them against the chosen backend
25+
3. Mount the executor at the call sites where the consumer's cognitive substrate needs hot-key lookups
26+
27+
This doc proposes formalizing two valid backend categories so adopters do not have to discover them by reverse-engineering the call sites.
28+
29+
## Two valid backend categories
30+
31+
### Backend A: standalone Redis (or Redis with hash-tag re-keying)
32+
33+
A consumer can run a Redis server and pipe `RedisCommand`s to it via any Redis client (`redis-rs`, `fred`, etc.). The consumer writes the executor: receive `RedisCommand`, translate to the client's API, return results.
34+
35+
**Important caveat (per codex P2 review on PR #455):** the documented `walk_to_root` operation uses `MGET ada:dn:{ancestor1} ada:dn:{ancestor2} ...`. In a **Redis Cluster** deployment, `MGET` requires all keys to belong to the same hash slot — which means they must share a `{hash_tag}` substring per the [Redis Cluster specification](https://redis.io/docs/latest/operate/oss_and_stack/reference/cluster-spec/). The current key layout `ada:dn:{hex}` does NOT include a hash tag, so cluster `MGET` will return `CROSSSLOT Keys in request don't hash to the same slot` errors.
36+
37+
For Redis Cluster to be a valid backend, EITHER:
38+
- (i) The key layout must be re-shaped to include a shared hash tag for keys that are co-queried (e.g. `ada:dn:{root_basin}:0102...` so all descendants of a basin hash to one slot)
39+
- (ii) The consumer must split multi-key operations into per-key calls (defeats the pipeline)
40+
- (iii) The consumer runs standalone Redis (not Cluster)
41+
42+
This caveat is the practical reason most consumers should treat dn_redis's key shape as "designed for standalone Redis OR for an in-binary executor", not for a clustered Redis deployment without re-shaping.
43+
44+
### Backend B: in-binary executor over Lance via DataFusion
45+
46+
A consumer can implement an executor that takes the same `RedisCommand` shape and runs it against the local Lance dataset via DataFusion queries. The result is Redis-protocol-shaped responses (via the Rust types — not wire-protocol bytes) emitted from the consumer's own data, with **no Redis service required**.
47+
48+
What the consumer writes (per codex P2 #3 — there is no shipped trait to implement; this IS new consumer code):
49+
50+
```rust
51+
// Consumer code, NOT lance-graph code
52+
struct LanceBackend { /* ... */ }
53+
54+
impl LanceBackend {
55+
fn execute(&self, cmd: RedisCommand) -> Result<RedisValue, Error> {
56+
match cmd {
57+
RedisCommand::Get(key) => {
58+
let dn = self.parse_dn_from_key(&key)?;
59+
let row = self.lance.read_by_dn(dn).await?;
60+
Ok(RedisValue::Bulk(cog_record_to_bytes(&row)))
61+
}
62+
RedisCommand::Mget(keys) => { /* batch lookup over Lance */ }
63+
RedisCommand::Keys(pattern) => { /* DataFusion scan filtered by prefix */ }
64+
// ... etc per the enum's variants
65+
}
66+
}
67+
}
68+
```
69+
70+
This is "Redis-shape over Lance" — the consumer projects Lance results through the command-result type. There is no protocol parsing because no wire protocol is involved; everything is in-process Rust types.
71+
72+
## What this doc does NOT claim
73+
74+
- **It does not claim FalkorDB or KùzuDB use dn_redis.** Those are independent products that implement the actual Redis RESP wire protocol; dn_redis does not implement RESP. The earlier version of this doc cited them as a precedent for "talk Redis without being Redis" — that framing was wrong. The honest framing is more constrained: dn_redis provides the key-shape and command-type contract that a consumer can EITHER pipe to a real Redis (standalone) OR execute in-binary against their own data.
75+
- **It does not document a network protocol.** There is no listener, parser, or executor shipped by lance-graph for dn_redis. Calling it "wire-protocol emulation" (as a prior version of this doc did) misleads consumers about what is implemented vs what they must implement themselves.
76+
- **It does not claim Redis Cluster is plug-and-play.** Per the caveat above, the current key shape is incompatible with cluster `MGET` slot routing.
77+
78+
## Why this matters
79+
80+
For single-binary deployments (e.g. `AdaWorldAPI/bardioc/substrate-b`) the natural backend is option (B): an in-binary executor over Lance. This means "zero application-level boundaries within substrate-b" (per the PR #452 / #454 append-only-Raft doc) survives the addition of HHTL-keyed hot lookups precisely because dn_redis is a TYPE MODEL, not a service dependency.
81+
82+
For consumers who already operate a standalone Redis (with no clustering or with cluster + re-keyed hash tags), option (A) is straightforward: write the executor that translates `RedisCommand` to a Redis client's API.
83+
84+
The earlier version of this doc framed dn_redis as wire-protocol emulation; codex review on PR #455 caught the inaccuracy. The corrected framing is more constrained but more honest: dn_redis is the SHAPE, the consumer provides the EXECUTION.
85+
86+
## Cross-references
87+
88+
- `lance-graph-cognitive::container_bs::dn_redis` — the key-shape protocol + command type model
89+
- `lab-vs-canonical-surface.md` — the canonical-vs-lab discipline that frames adapters
90+
- Companion docs `APPEND_ONLY_RAFT_DOVETAIL.md` + `CLUSTER_ASYMMETRY.md` (PR #452 / #453 / #454 — merged)
91+
- `AdaWorldAPI/bardioc` PR #15 conversation thread (where this doc + the corrections originated)
92+
- [Redis Cluster specification](https://redis.io/docs/latest/operate/oss_and_stack/reference/cluster-spec/) for the hash-slot constraint

0 commit comments

Comments
 (0)