Skip to content

feat(tier-2): Lane A — SWAPDB, MOVE, COPY DB n, CLUSTER REPLICAS/SLAVES, COUNT-FAILURE-REPORTS #100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TinDang97 merged 8 commits into from
May 24, 2026
Merged

feat(tier-2): Lane A — SWAPDB, MOVE, COPY DB n, CLUSTER REPLICAS/SLAVES, COUNT-FAILURE-REPORTS #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c381b31
feat(command): implement SWAPDB across shards (Tier 2 / T2.1)
TinDang97 May 24, 2026
4958dc9
feat(command): implement MOVE key db (Tier 2 / T2.2)
TinDang97 May 24, 2026
bbc6117
feat(command): implement COPY ... DB n option (Tier 2 / T2.3)
TinDang97 May 24, 2026
f538589
feat(cluster): implement CLUSTER REPLICAS / SLAVES (Tier 2 / T2.4)
TinDang97 May 24, 2026
ebd240a
feat(cluster): implement CLUSTER COUNT-FAILURE-REPORTS (Tier 2 / T2.5)
TinDang97 May 24, 2026
608e2d1
perf(handler): collapse duplicate is_write gate on MOVE/COPY hot path
TinDang97 May 24, 2026
e429b2b
refactor(storage): split segment.rs into segment/ directory module
TinDang97 May 24, 2026
3267648
fix(tier-2): address CodeRabbit review findings on PR #100
TinDang97 May 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,51 @@ and **Change Data Capture (CDC)**, built additively on top of the existing
per-shard WAL v3 + dual-root manifest. No changes to the KV hot path, MVCC,
page format, or transaction layer.

### Added — Tier 2 Lane A (PR #100)

- **T2.1** `c381b31` — `SWAPDB` cross-shard atomic swap via `ShardMessage::SwapDb`;
WAL-durable; BGREWRITEAOF concurrency guard; restart-replay test.
- **T2.2** `4958dc9` — `MOVE key db` with `with_two_dbs_locked` (lower-index-first
lock ordering); WAL-durable; intercept in all four handler paths.
- **T2.3** `bbc6117` — `COPY ... DB n` cross-database; reuses `with_two_dbs_locked`;
WAL-durable.
- **T2.4** `f538589` — `CLUSTER REPLICAS` / `CLUSTER SLAVES`; shared
`format_node_line(node, self_node_id)` helper extracted from `CLUSTER NODES`.
- **T2.5** `ebd240a` — `CLUSTER COUNT-FAILURE-REPORTS`; counts non-stale
`pfail_reports`; exposes `DEFAULT_NODE_TIMEOUT_MS` as `pub(crate)`.

### Fixed

- **PERF** `608e2d1` — collapse duplicate `is_write` PHF gate on MOVE/COPY hot
path; restores s=1 SET p=1 throughput (−9.5 % → +0.9 % vs. merge base).
- **CR** _(this PR)_ — SWAPDB now runs **after** the ACL gate in `handler_monoio`,
closing a runtime-specific authorization bypass.
- **CR** _(this PR)_ — `with_two_dbs_locked` and `ShardDatabases::swap_dbs` now
hard-assert non-equal indices in release builds, preventing same-index
self-deadlock.
- **CR** _(this PR)_ — `SWAPDB` strict arity (exactly two args) across all three
handlers; rejects `SWAPDB 0 1 extra` with the canonical wrong-arity error.
- **CR** _(this PR)_ — `DashTable::Segment::insert_or_update_at` now sets
`has_non_home_keys = true` whenever the chosen free slot is in a non-home
group; fixes a latent miss where `find()` could not locate a fallback-placed
key on subsequent lookups.
- **CR** _(this PR)_ — Local `MOVE` / `COPY` AOF append is gated on
`Frame::Integer(1)` (success) rather than `!Error`, matching the
`handler_single` behavior and suppressing no-op `:0` log entries.
- **CR** _(this PR)_ — `MOVE`, `COPY ... DB n`, and `SWAPDB` are rejected with
`ERR_TXN_CROSS_SHARD` while an `active_cross_txn` is in flight; previously the
intercepts bypassed undo/intents bookkeeping and escaped `TXN.ABORT` rollback.
- **CR** _(this PR)_ — `spsc_handler` `MOVE`/`COPY` arms call
`refresh_now_from_cache` on both source and destination DBs before
`move_core`/`copy_core`; fixes expired-key visibility skew on the local-write
path.

### Refactor

- `e429b2b` — `src/storage/dashtable/segment.rs` (1587 LOC) split into
`segment/{mod,find,insert,ops}.rs`; mechanical refactor, zero semantic change,
brings all files under the 1500-LOC limit ahead of future hot-path additions.

### Added — Point-in-Time Recovery (PITR)

- **P0** `ac3aa92` — `WalWriterV3::new()` now scans existing `.wal` segments on
Expand Down
38 changes: 38 additions & 0 deletions scripts/test-consistency.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,44 @@ both SET edge:touch "val"
assert_both "TOUCH" TOUCH edge:touch
assert_both "TOUCH missing" TOUCH edge:nomiss

# ===========================================================================
# SWAPDB consistency
# ===========================================================================
log "=== SWAPDB ==="

# Seed: db0 has swapkey=hello, db1 is empty
both SELECT 0
both SET swapkey hello
both SELECT 1
both DEL swapkey

Comment on lines +548 to +553
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use explicit DB targeting when seeding SWAPDB state

both SELECT ... does not persist DB selection across later redis-cli invocations. On Line 552, DEL swapkey runs against default DB 0, which can invalidate the intended seed and make the swap check non-representative.

Suggested fix 
-# Seed: db0 has swapkey=hello, db1 is empty
-both SELECT 0
-both SET swapkey hello
-both SELECT 1
-both DEL swapkey
+# Seed: db0 has swapkey=hello, db1 is empty
+redis-cli -p "$PORT_REDIS" -n 0 SET swapkey hello >/dev/null 2>&1 || true
+redis-cli -p "$PORT_RUST"  -n 0 SET swapkey hello >/dev/null 2>&1 || true
+redis-cli -p "$PORT_REDIS" -n 1 DEL swapkey >/dev/null 2>&1 || true
+redis-cli -p "$PORT_RUST"  -n 1 DEL swapkey >/dev/null 2>&1 || true
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/test-consistency.sh` around lines 548 - 553, The seed step uses the
helper/command "both" but relies on a separate SELECT that won’t persist across
invocations, so the final "both DEL swapkey" runs against DB0 instead of DB1;
update the seed to target DB1 explicitly when deleting (i.e., remove the
separate SELECT 1 and invoke DEL with DB index specified for "both" so the DEL
runs against DB1 for key "swapkey") and ensure the commands around "both SET
swapkey hello" and the subsequent delete use explicit DB targeting.

# SWAPDB 0 1 — swaps databases 0 and 1
assert_both "SWAPDB 0 1" SWAPDB 0 1

# After swap: db0 should be empty (swapkey gone), db1 should have swapkey=hello
redis_after_swap=$(redis-cli -p "$PORT_REDIS" -n 1 GET swapkey 2>&1) || true
rust_after_swap=$(redis-cli -p "$PORT_RUST" -n 1 GET swapkey 2>&1) || true
assert_eq "SWAPDB: key moved to db1" "$redis_after_swap" "$rust_after_swap"

redis_db0_gone=$(redis-cli -p "$PORT_REDIS" -n 0 GET swapkey 2>&1) || true
rust_db0_gone=$(redis-cli -p "$PORT_RUST" -n 0 GET swapkey 2>&1) || true
assert_eq "SWAPDB: key absent from db0" "$redis_db0_gone" "$rust_db0_gone"

# Same-index SWAPDB is a no-op; must return OK (not error)
assert_both "SWAPDB 0 0 (same-index no-op)" SWAPDB 0 0

# Out-of-range indices must return ERR (not panic)
redis_oor=$(redis-cli -p "$PORT_REDIS" SWAPDB 0 9999 2>&1) || true
rust_oor=$(redis-cli -p "$PORT_RUST" SWAPDB 0 9999 2>&1) || true
if echo "$rust_oor" | grep -qi "ERR"; then
PASS=$((PASS + 1))
else
FAIL=$((FAIL + 1)); echo " FAIL: SWAPDB out-of-range should return ERR, got: $rust_oor"
fi
Comment on lines +570 to +576
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert out-of-range parity against Redis (and use redis_oor)

Line 570 captures redis_oor, but the check only validates moon output contains ERR. This misses direct parity verification and leaves redis_oor unused.

Suggested fix 
 redis_oor=$(redis-cli -p "$PORT_REDIS" SWAPDB 0 9999 2>&1) || true
 rust_oor=$(redis-cli -p "$PORT_RUST" SWAPDB 0 9999 2>&1) || true
-if echo "$rust_oor" | grep -qi "ERR"; then
-    PASS=$((PASS + 1))
-else
-    FAIL=$((FAIL + 1)); echo "  FAIL: SWAPDB out-of-range should return ERR, got: $rust_oor"
-fi
+assert_eq "SWAPDB out-of-range parity" "$redis_oor" "$rust_oor"
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 570-570: redis_oor appears unused. Verify use (or export if used externally).

(SC2034)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/test-consistency.sh` around lines 570 - 576, The test currently reads
redis_oor but only checks rust_oor for "ERR" so redis_oor is unused and parity
isn't asserted; update the conditional to verify both redis_oor and rust_oor
contain "ERR" (e.g., use grep -qi "ERR" on both $redis_oor and $rust_oor) and
only increment PASS when they both match, otherwise increment FAIL and echo a
message showing both $redis_oor and $rust_oor to highlight the discrepancy; keep
the existing variables (redis_oor, rust_oor, PASS, FAIL, SWAPDB) when
implementing this change.


# Swap back to restore state for remaining tests
both SWAPDB 0 1

# FLUSHDB (run last — clears all keys)
assert_both "FLUSHDB" FLUSHDB

Expand Down
Loading
Loading