Skip to content

Commit 419d07b

Browse files
AlexStocksOmX
andauthored
refactor(storage): remove Engine abstraction (#365)
* docs(storage): plan concrete RocksDB ownership migration * fix(runtime): drain RocksDB owners during storage swap * fix(runtime): let paused monitors observe shutdown * refactor(storage): use concrete RocksDB ownership * fix(storage): own RocksDB shutdown lifecycle Wrap the concrete DB in a non-cloneable shutdown owner so Redis waits for active RocksDB background work before the final handle can be released from a callback thread. Expose only a borrowed DB accessor and cover the active compaction-filter drop race with a real RocksDB regression test. Tested: cargo +1.95-x86_64-pc-windows-msvc test --package storage --quiet Tested: 5x cargo +1.95-x86_64-pc-windows-msvc test --package storage --lib dropping_last_owner_waits_for_active_compaction_filter_before_reopen -- --test-threads=1 Tested: rustfmt +1.95-x86_64-pc-windows-msvc --edition 2024 --check src/storage/src/redis.rs src/storage/src/data_compaction_filter.rs src/storage/tests/redis_basic_test.rs src/storage/tests/redis_hash_test.rs src/storage/tests/redis_set_test.rs src/storage/tests/redis_zset_test.rs Tested: cargo +1.95-x86_64-pc-windows-msvc clippy --package storage --lib -- -D warnings -D clippy::unwrap_used Co-authored-by: OmX <omx@oh-my-codex.dev> * refactor(raft): store logs in concrete RocksDB handles * docs(storage): define the unique RocksDB shutdown owner * test(storage): harden RocksDB lifecycle coverage Signal from inside the unique owner Drop before background cancellation so the active-compaction regression cannot pass merely because the drop thread was not scheduled. Route the exported DB/CF macro through Redis::db() and exercise its expansion from an external integration crate. Tested: 10x cargo +1.95-x86_64-pc-windows-msvc test --package storage --lib dropping_last_owner_waits_for_active_compaction_filter_before_reopen -- --test-threads=1 Tested: cargo +1.95-x86_64-pc-windows-msvc test --package storage --lib --quiet Tested: cargo +1.95-x86_64-pc-windows-msvc test --package storage --quiet Tested: rustfmt +1.95-x86_64-pc-windows-msvc --edition 2024 --check src/storage/src/redis.rs src/storage/tests/redis_basic_test.rs Tested: cargo +1.95-x86_64-pc-windows-msvc clippy --package storage --lib -- -D warnings -D clippy::unwrap_used Tested: cargo +1.95-x86_64-pc-windows-msvc clippy --package storage --test redis_basic_test --no-deps -- -D warnings -D clippy::unwrap_used Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(raft): drain storage owners before snapshot restore * refactor(storage): remove the Engine abstraction crate * docs(raft): plan transactional snapshot installation * refactor(storage): stage snapshot restore before commit * fix(raft): gate every hot-swappable storage owner * fix(raft): persist snapshot install recovery state * fix(raft): make snapshot pauses cancellation-safe * docs: plan Python integration hardening * ci(test): run Python integration tests against Kiwi Start a dedicated temporary Kiwi instance for Ubuntu integration tests and make server availability a strict pytest gate in CI. Isolated runs flush the dedicated database around every test while ordinary local runs retain skip and prefix-cleanup behavior. Constraint: Keep lifecycle management in the test harness and do not modify product or test-case code. Tested: strict unavailable-server failure; ordinary local skip; missing binary, occupied port, and early server exit paths; bash syntax; staged diff check; isolated list sequence reports 2 passed. Not-tested: Full Python suite depends on the remaining Redis compatibility fixes in later plan tasks. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(resp): encode nulls for negotiated protocol * fix(resp): encode RESP2 null correctly * fix(storage): preserve binary string values * test(storage): cover multi-instance binary mget * fix(storage): support Redis key glob matching * fix(storage): make key glob matching binary safe * test: align Python integration tests with Redis semantics Correct MSET compatibility expectations and make concurrent observations use one MGET snapshot. Propagate worker failures through futures so pytest cannot silently pass thread assertion errors. Constraint: Python integration tests only; no Rust, fixture, or CI harness changes. Rejected: Sequential GET and LLEN/LRANGE comparisons because they observe different command boundaries. Confidence: high Scope-risk: Test expectations and concurrency orchestration only. Tested: py_compile; pytest --collect-only (55 tests); targeted old-binary probe (2 expected atomicity failures, corrected semantic tests pass). Not-tested: Full Python suite awaits the matching multi-instance storage fix and rebuilt Kiwi binary. Co-authored-by: OmX <omx@oh-my-codex.dev> * test: propagate Python worker failures Let ThreadPoolExecutor futures preserve worker tracebacks, remove the stress-test exception sink, and register exact test keys for unconditional cleanup after failures. Constraint: Follow-up touches only the four Python integration test files from the prior semantics commit. Rejected: Prefix-based cleanup and collected exception counters because they can hide worker failures or affect unrelated local keys. Confidence: high Scope-risk: Test orchestration and cleanup only. Tested: WSL venv py_compile; pytest --collect-only (55 tests); git diff --cached --check. Not-tested: Runtime suite awaits the concurrent Rust storage changes in the shared worktree. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(storage): match Redis glob class edge cases * fix(storage): serialize multi-instance multi-key commands * fix(storage): match Redis empty glob edge cases * test: harden multi-key concurrency coverage Observe DEL/MSET races with a synchronized MGET worker, namespace every touched Redis key, and bound concurrent and slow tests with pytest-timeout plus future deadlines. Constraint: Only the four Python integration test files are changed; existing Rust work remains separate. Rejected: Final-state-only atomicity checks, generic local Redis key names, and unbounded executor waits. Confidence: high Scope-risk: Test naming, isolation, synchronization, and timeout behavior only. Tested: WSL venv py_compile; pytest --collect-only (55 tests); combined Python diff check from 4b43a22^. Not-tested: Runtime Python suite awaits the final rebuilt Kiwi binary. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(storage): preserve scan allkeys matching * test: bound Python integration I/O Bound redis-py connect/read operations and make the DEL/MSET observer handshake wait for a completed mutation before sampling while holding the last mutator until that sample completes. Constraint: Only tests/python/conftest.py and the MSET concurrency test are changed; Rust work remains separate. Rejected: Permanently-set activity flags because they do not prove the sample happened before mutation completion. Confidence: high Scope-risk: Python test client timeouts and deterministic concurrency synchronization only. Tested: WSL venv py_compile; pytest --collect-only (55 tests); strict no-server mode exits non-zero without skipping; combined Python diff check. Not-tested: Runtime suite awaits the final rebuilt Kiwi binary. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(storage): make command visibility gate async * test: guarantee concurrent mutation observation Pause both mutators immediately after their first DEL or MSET until the observer validates a single MGET snapshot, then let them finish the remaining iterations before marking mutation completion. Constraint: Only tests/python/test_mset_concurrent.py changes; client timeout and Rust history remain untouched. Rejected: End-of-loop overlap gates because they do not force observation before subsequent mutations. Confidence: high Scope-risk: Deterministic test synchronization only. Tested: WSL venv py_compile; pytest --collect-only (55 tests); current and combined Python diff checks. Not-tested: Runtime suite awaits the final rebuilt Kiwi binary. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(storage): preserve list tail after initial lpush * fix(storage): preserve binary scan entries * fix(storage): make snapshot restore durable * perf(storage): seek binary hscan prefixes * fix(raft): preserve snapshot cleanup marker * refactor(cmd): centralize scan option parsing Share cursor, MATCH, and COUNT parsing across HSCAN, SSCAN, and ZSCAN while preserving binary patterns and existing error semantics. Align the KEYS test module with the project unwrap lint convention. Constraint: Keep storage scan APIs and command-visible behavior unchanged. Rejected: Changing MGET locking, snapshot pause handling, Windows durability dependencies, or unrelated review suggestions. Confidence: high Scope-risk: Command parser refactor and test lint annotation only. Tested: WSL Rust 1.95 cargo fmt --check; cargo test -p cmd (105 passed); cargo clippy -p cmd --all-features with warnings and unwrap denied. Not-tested: Full workspace test suite and remote CI; both will run after push. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix: harden snapshot coordination and glob matching Split snapshot state and publication coordination so apply resumes after checkpoint while snapshot publication remains serialized. Correct repeated-star matching for empty Redis keys and scan entries, with deterministic cancellation and error-path coverage. Constraint: Preserve fail-closed snapshot install marker semantics and existing public storage APIs. Rejected: A single long-held snapshot mutex, fixed-duration concurrency assertions, or public test hooks. Confidence: high Scope-risk: Raft snapshot locking and cancellation plus empty-byte glob matching in KEYS, HSCAN, and SSCAN. Tested: WSL Rust 1.95 workspace clippy with warnings and unwrap denied; cargo test -p raft (106 passed); cargo test -p storage glob_tests (10 passed); cargo test -p cmd (107 passed); Windows and WSL cargo fmt --check; git diff --check. Not-tested: Full workspace cargo test, Python integration suite, and remote CI. Co-authored-by: OmX <omx@oh-my-codex.dev> * fix(storage): restore Redis empty glob semantics Restore Redis-compatible initial-empty-string handling in the generic glob matcher while retaining the command-level exact single-star special case. Update KEYS, HSCAN, and SSCAN tests to distinguish '*' from repeated-star patterns. Constraint: Preserve the snapshot coordination fixes, public storage APIs, and all non-empty glob behavior. Rejected: Updating the existing storage integration tests to accept repeated-star matches, because Redis 8.4 source and a live Redis comparison show that behavior is incompatible. Confidence: high Scope-risk: Empty key, hash field, and set member filtering for KEYS, HSCAN, and SSCAN patterns only. Tested: Live Redis comparison for '*', '**'; WSL Rust 1.95 full workspace cargo test; workspace clippy with warnings and unwrap denied; six focused glob and command tests; cargo fmt --check; git diff --check. Not-tested: Local address sanitizer instrumentation and remote CI rerun. Co-authored-by: OmX <omx@oh-my-codex.dev> --------- Co-authored-by: OmX <omx@oh-my-codex.dev>
1 parent 9d94641 commit 419d07b

69 files changed

Lines changed: 7840 additions & 1908 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ jobs:
253253
run: make -C tests install-deps
254254

255255
- name: Run integration tests
256-
run: make -C tests test-python
256+
run: bash tests/run_python_integration.sh
257257

258258
sanitizers:
259259
name: Sanitizers (${{ matrix.sanitizer }})

CLAUDE.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,8 @@ Workspace members under `src/`:
6565
- `cmd/` — Redis command implementations. Each command implements the `Cmd` trait.
6666
- `executor/` — Async command executor / task pool.
6767
- `client/` — Per-connection client state (`argv`, `cmd_name`, `key`, reply buffer, authentication).
68-
- `storage/` — Multi-instance RocksDB storage, column families, TTL, key encoding, and log index for Raft.
69-
- `engine/``Engine` trait abstraction and RocksDB engine implementation.
70-
- `raft/` — OpenRaft integration, log store, state machine, snapshot archive, and gRPC services.
68+
- `storage/` — Multi-instance concrete RocksDB ownership, column families, TTL, key encoding, and log index for Raft.
69+
- `raft/` — OpenRaft integration, concrete RocksDB log-store ownership, state machine, snapshot archive, and gRPC services.
7170
- `conf/` — Configuration loading, validation, and sample-config generation.
7271
- `kstd/` — Utilities, including `LockMgr` for sharded key-level locking.
7372
- `common/runtime/` — Dual-runtime manager, async message channel between network and storage runtimes, and `StorageServer`.

Cargo.lock

Lines changed: 1 addition & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
[workspace]
22
resolver = "2"
33
members = [
4-
"src/engine",
54
"src/storage",
65
"src/kstd",
76
"src/common/macro",
@@ -70,9 +69,9 @@ uuid = { version = "1.23", features = ["v4", "serde"] }
7069
openraft = { version = "0.9", features = ["serde", "storage-v2"] }
7170
proptest = "1.11"
7271
arc-swap = "1.7"
72+
windows-sys = { version = "0.61.2", features = ["Win32_Storage_FileSystem"] }
7373

7474
## workspaces members
75-
engine = { path = "src/engine" }
7675
storage = { path = "src/storage" }
7776
kstd = { path = "src/kstd" }
7877
common-macro = { path = "src/common/macro" }

README.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,9 @@ src/server/ → Entry point (main.rs)
2424
src/net/ → TCP server, connection handling, cluster routing
2525
src/cmd/ → Command definitions: Cmd trait, CmdMeta, command table
2626
src/executor/ → Command executor: tokio async task pool
27-
src/storage/ → Multi-instance RocksDB, column families, TTL
28-
src/engine/ → Engine trait abstraction over RocksDB
27+
src/storage/ → Multi-instance RocksDB ownership, column families, TTL
2928
src/resp/ → RESP protocol: parser, encoder, RespData types
30-
src/raft/ → Raft consensus: OpenRaft integration, state machine, router
29+
src/raft/ → Raft consensus: OpenRaft integration, RocksDB log store, state machine, router
3130
src/conf/ → Configuration: loading, validation, RaftClusterConfig
3231
src/client/ → Client context: connection state, argv, reply buffer
3332
src/common/runtime/ → Runtime management: async channel between net & storage

README_CN.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,9 @@ src/server/ → 入口 (main.rs)
2424
src/net/ → TCP 服务、连接管理、集群路由
2525
src/cmd/ → 命令定义:Cmd trait、CmdMeta、命令表
2626
src/executor/ → 命令执行器:tokio 异步任务池
27-
src/storage/ → 多实例 RocksDB、列族、TTL
28-
src/engine/ → RocksDB 引擎 trait 抽象
27+
src/storage/ → 多实例 RocksDB 所有权、列族、TTL
2928
src/resp/ → RESP 协议:解析、编码、RespData 类型
30-
src/raft/ → Raft 共识:OpenRaft 集成、状态机、路由器
29+
src/raft/ → Raft 共识:OpenRaft 集成、RocksDB 日志存储、状态机、路由器
3130
src/conf/ → 配置:加载、校验、集群配置
3231
src/client/ → 客户端上下文:连接状态、参数、响应缓冲
3332
src/common/runtime/ → 运行时管理:网络与存储间的异步通道

0 commit comments

Comments
 (0)