Repository navigation
Conversation
…y log entry. feat #66: Refactor the Errors to distinguish between protocol logic errors and system-level errors. * feat #43: When a Raft leader is elected, it should first send an empty log entry. 1. implement verify_leadership_in_new_term 2. refactor verify_leadership_in_new_term and enforce_quorum_consensus * feat #66: Refactor the Errors to distinguish between protocol logic errors and system-level errors.
…r-friendly StorageEngine and StateMachine * feat #59: Refactor state_machine_commit_listener into a separate function to improve visibility across all newly spawned threads. * feat #79: Add snapshot feature * fix #90: Refactor: Decouple Client Command Protocol from Raft Internal Log Payload Type * feat #89: Add auto-discovery support for new learner nodes * feat #101: Revised learner join process with promotion/fail semantics * feat #45: Implemented first/last index for term to resolve replication conflicts * fix #106: Retry leadership noop confirmation until timeout * feat #102: Automatic node removal * feat #109: Enable RPC connection cache * fix #107: 1. Add back rpc connection params 2. Change parking lot sync lock to Tokio async lock in RaftMembership 3. optimize MembershipGuard for performance. Remove read locker. * feat #110: 1. add new events SnapshotCreated and LogPurgeCompleted 2. decouple Leader snapshot create to several events 3. refactor raft log index generation from pre_allocate_raft_logs_next_index to pre_allocate_id_range 4. optimize LocalLogBatch structure - based on performance analysis * feat #119: Make the StorageEngine trait more developer-friendly * feat #120: Make the StateMachine trait more developer-friendly * fix #121: Election: higher term vote request should be updated immediately and reset voted_for * feat #122: Refactor compress and decompress snapshot logic from StateMachine to StateMachineHandler * fix #123: Replace global tracing log with trace_test crate * feat #125: Make rocksdb adaptor as feature. Move sled implementation as example
…tart when path is already locked by another node
* refs #138: optimize to use long lived peer tasks for append entries request only * refs #138: try to fix race issue - Multiple threads can pass the initial check and all create new appenders * feat #138: 1. introduce crossbeam 2. optimize buffered_raft_log by creating a flush worker pool with configurable number of workers to handle persistence operations * feat #138: optimize flush_workers as configure * feat #138: optimize channel_capacity into configure * feat #139: work on a reliable monitoring metric for resource utilization * feat #140: Optimize proto bytes fields to use bytes::Bytes * feat #140: switch Into<Bytes> to AsRef<[u8]> * fix #141: Optimize RocksDB write path and Raft log loop for lower latency * feat 142: Add Read Consistency Policy Support and Lease-Based Read Optimization * feat #142: update v0.1.4 bench report * feat #142: add client doc guide * feat #143: Refactor gRPC compression configuration for Raft transport (performance optimization) - step1: disable server side client rpc compress response * feat #143: Refactor gRPC compression configuration for Raft transport (performance optimization) * feat #143: add performance diagram * feat #138: ready to publish v0.1.4 * feat #138: add ut * fix #146: buffered_raft_log.rs Code Reviews * fix #146: update bench report * fix #145: Bug: Undefined Behavior from Incorrect Lifetime Conversion in Mmap Zero-Copy Path * fix #148: fix rocksdb storage engine flush bug * fix #147: fix file state machine bug
Update CHANGELOG.md, README.md, and crate docs for v0.2.0 release: - Rewrite CHANGELOG v0.2.0 section focusing on developer value - Add v0.2.0 highlights to README.md Features section - Update installation examples with feature flags - Improve FAQ section with production-readiness statement - Add "What's New in v0.2.0" section to d-engine/src/lib.rs Changes emphasize product-to-market messaging: workspace structure, TTL/Lease, Watch API, EmbeddedEngine, LocalKvClient
…audit - Add RUSTSEC-2025-0134 to deny.toml ignore list - rustls-pemfile is indirect dep via tonic 0.12 - Will upgrade to tonic 0.14+ in future release - Update Cargo.lock
- Add check_cluster_is_ready() after node restart in failover_test - Root cause: client connected before node fully initialized - Connection refused error due to missing readiness check - Update overview.md Quick Start examples - Add EmbeddedEngine::with_rocksdb() as recommended approach - Fix FileStateMachine::new() async call (add .await) - Clarify API usage: EmbeddedEngine (simple) vs NodeBuilder (flexible)
Problem: Single node cannot exit with Ctrl+C during startup when peers are unavailable. Process hangs with "Connection refused" errors during cluster ready check and connection warmup phases. Solution: 1. Add shutdown_signal (watch::Receiver) to Node struct 2. Use tokio::select! in run() to monitor shutdown during: - check_cluster_is_ready() - pre_warm_connections() 3. Implement Drop for GrpcTransport to abort peer appender tasks 4. Add abort_all_tasks() method for immediate task termination Impact: - Node responds to SIGINT immediately during startup - No connection retry spam after Ctrl+C - Clean shutdown in both single-node and cluster modes
Problem: 1. lib.rs contained broken .html links (e.g., raft-role.html) - Actual paths use underscores and index.html (raft_role/index.html) 2. All workspace crates generated docs, including internal ones - d-engine-core and d-engine-proto are implementation details 3. External references in quick-start-5min.md broke in generated docs 4. Missing error-handling.md caused broken references Solution: 1. Fixed all .html links in lib.rs to match rustdoc output format - Changed hyphenated names to underscore_names/index.html 2. Updated Makefile docs target to exclude internal crates - Added --exclude d-engine-core --exclude d-engine-proto - publish = false ensures they won't be released to crates.io 3. Removed external product-design references - Replaced with inline content in quick-start-5min.md 4. Created error-handling.md with error categories and retry strategies - Updated go-client.md to reference new doc instead of GitHub links Files Changed: - d-engine-docs/src/lib.rs: Fixed 12+ .html link paths - d-engine-docs/src/docs/client_guide/error-handling.md: New file - d-engine-docs/src/docs/client_guide/mod.rs: Added error_handling module - d-engine-docs/src/docs/client_guide/go-client.md: Updated error reference - d-engine-docs/src/docs/client_guide/service-discovery-pattern.md: Fixed scale-to-cluster link - d-engine-docs/src/docs/server_guide/watch-feature.md: Removed broken config/performance links - d-engine-docs/src/docs/quick-start-5min.md: Removed external reference - Makefile: Updated docs target to exclude internal crates Impact: - All documentation links now work correctly - Cleaner workspace docs (only public crates: client, server, docs) - Better user experience with complete error handling guide - No broken external dependencies in generated documentation
## Feature Simplification - Remove `full` feature (confusing, no real use case) - Add `default = ["server", "rocksdb"]` for zero-config start - Move `KvClient` trait export to `client` feature (from `full`) ## Documentation Enhancements - README: Add "When to Use" section explaining 3 integration modes - Embedded Mode (Rust): Zero-overhead, single binary - Standalone Mode (Go/Python/Java): Language-agnostic gRPC - Custom Storage: Advanced use cases (Sled, memory-only) - Add use-cases.md: Detailed scenarios for control plane, DNS, orchestration - Include objective performance claims (51% vs etcd, with disclaimers) ## Developer Experience - Simplify dependency declaration: `d-engine = "0.2"` (no features needed) - Examples updated to use default feature (7 examples simplified) - Add ❌ "Don't use" guidance to prevent common mistakes ## Testing Infrastructure - Fix `make test-examples`: Now properly verifies all examples compile - Add `test-examples` to `make test-all` (pre-release checklist)
Changes: - Remove commented legacy package config from workspace Cargo.toml - Update README benchmark reference to v0.2.0 (dengine_comparison_v0.2.0.png) - Upgrade rustfmt edition from 2021 to 2024 - Clean up outdated v0.2.0 benchmark reports (report_20251205/09/10.md) - Add final v0.2.0 benchmark report and comparison chart - Sync version references across all example Cargo.toml files
- Fix typo: rename backgroup → background in snapshot transfer - Remove obsolete storage_test.rs - Update client error handling and KV operations - Fix network module exports - Update candidate state and state machine handler - Fix single-node cluster examples documentation - Update benchmark workflow configuration
Cache cluster metadata in LeaderState to eliminate 3 async calls per write.
Performance: +2-3% throughput, -13-21% p99 latency vs v0.2.0
Changes:
- Add ClusterMetadata { is_single_node, total_voters } cache
- Initialize on leader election, update on membership changes
- Pass to replication handler to avoid repeated queries
- Fix 6 test failures due to missing metadata initialization
…ale cache Bug: update_cluster_metadata() only updated total_voters, missing is_single_node. Caused test_embedded_node_rejoin to hang intermittently. Fix: - Update BOTH is_single_node and total_voters in update_cluster_metadata() - Call update_cluster_metadata() after learner promotion - Add unit test to catch this bug
Race condition: gRPC servers start async, election RPCs timeout before ready. Solution: Increase test election timeout 100ms→2000ms, retries 3→5. Only affects tests/common/mod.rs test configuration.
…tations - Add Project Status section to README (pre-1.0 + compatibility promise) - Simplify MIGRATION_GUIDE (remove NodeBuilder API, keep WAL format only) - Streamline CHANGELOG v0.2.0 highlights (code examples + concise features) - Update benchmark reports with actual test data (Dec 13, 2025) - Restructure performance report (TL;DR first, detailed results collapsed) Focus: Lower cognitive load for new users while maintaining transparency
…ments This PR consolidates architecture review improvements and critical bug fixes for v0.2.0 release. ## 🎯 Critical Fixes - **#212**: Fix learner promotion stuck (voter count + role transition bugs) - **#218**: Fix leader next_index initialization for new learners - **#222**: Return NOT_LEADER with leader metadata for client redirection - **#209**: Fix node restart wait_ready() timeout (leader notification race) ## ✨ Feature Additions - **#213**: Implement READ_ONLY Learner nodes for permanent analytics - **#219**: Pre-generate Go protobuf code for zero-config experience ## 🔧 Refactoring & Performance - **#223**: Optimize check_learner_progress() lock contention - Extract 5 helper methods (SRP), add 11 unit tests - **#211**: Remove Arc::get_mut anti-pattern from lease injection - **#210**: Simplify watch architecture (tokio::broadcast, 90% code sharing) - **#217**: Refactor Node::run() with strategy pattern - **#209**: Consolidate Raft unit tests (migrate 27 tests from server to core) ## 📚 Documentation - Restructure quick-start docs (embedded + standalone examples) - Add integration-modes.md and use-cases.md - Delete 911 lines of internal architecture docs (20/80 principle) - Fix Go client example with pre-generated protobufs ## 🧪 Testing & Validation - 14+ new unit tests across components - Fix flaky tests and timing issues (#204, #209) - Optimize test suite with nextest - All 430 core + 305 server + 292 integration tests passing ## 📊 Performance - Benchmarks stable (±3% variance) - CI optimized: removed benchmark compile check (saves 2-3 min/run) --- **Closes:** #209, #210, #211, #212, #213, #217, #218, #219, #222, #223 **Migration Notes:** - Config: `raft.watch.enabled` removed (breaking) - API: `StateMachine::start()` changed to async (breaking) - NodeStatus enum refactored: PROMOTABLE/READ_ONLY/ACTIVE (non-breaking) **Files Changed:** 100+ files, ~5000 insertions, ~1500 deletions
…ion for v0.2.0 ## Overview Prepare d-engine v0.2.0 for multi-crate publishing with comprehensive documentation, improved developer experience, and production-ready examples. ## Key Changes ### 📚 Documentation Restructuring (#225) **Adopted OpenRaft documentation pattern:** - Moved docs from standalone crate to `d-engine/src/docs/` (preserves git history) - Unified documentation structure with role-based guides - All crates now have clear positioning and usage guidelines **Multi-crate README strategy:** - Added READMEs for all workspace crates with consistent structure - Clear guidance on when to use each crate vs. main `d-engine` package - Architecture diagrams showing crate relationships - Verified all crates ready for publishing (`cargo publish --dry-run`) **Link fixes after refactoring:** - Updated 14 files with cross-crate doc links to docs.rs format - Fixed intra-doc links in `d-engine/src/docs/mod.rs` - Added tested Go/Python code generation examples in `d-engine-proto` ### 🔧 Developer Experience Improvements (#226) **Reduced startup noise:** - Eliminated misleading connection errors during cluster startup - Downgraded expected connection failures from `error!` to `debug!` level - Added startup banner with config path and node prefixes `[Node1]` `[Node2]` `[Node3]` - Improved log readability with emoji indicators **Examples cleanup:** - Removed duplicate `rocksdb-cluster` example (superseded by `three-nodes-cluster`) - Fixed `quick-start-embedded` release mode config error (added `CONFIG_PATH`) - Updated all examples to match current API (`EmbeddedEngine`, `StandaloneServer`) - Clarified service-discovery examples focus on **Watch API** demonstration ### 🐛 Bug Fixes **Compilation fixes:** - Fixed `Result` type conflicts in `quick-start-embedded` (use `std::result::Result`) - Updated API imports in `sled-cluster` after module reorganization - Fixed clippy `uninlined_format_args` warning in `three-nodes-cluster` **Updated references:** - All `rocksdb-cluster` references point to `three-nodes-cluster` - Updated `include_str!` paths in `d-engine-core` after docs relocation ## Testing - ✅ All examples compile and pass clippy checks - ✅ All crates ready for publishing (dry-run verified) - ✅ Proto generation commands tested (Go/Python/Java) - ✅ No performance regressions (±5% variance within normal range) ## Files Changed - **8** crate lib.rs/README.md files - **6** example README files - **3** example source files (compilation fixes) - **5** network/logging files (startup experience) - **1** docs reorganization (mod.rs/overview.md) --- **Ready for review and merge to `develop`** 🚀
📝 WalkthroughWalkthroughThis PR prepares v0.2.0: updates workspace and release metadata, consolidates docs into the main crate (removing the d-engine-docs crate), adjusts workspace dependencies and publish flags, restructures the d-engine public re-exports and prelude, expands client/core public surfaces, removes the rocksdb-cluster example, and updates many READMEs and example usages. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR prepares d-engine for v0.2.0 multi-crate publishing by reorganizing documentation and updating configuration. The main changes consolidate documentation from d-engine-docs into d-engine/src/docs, update crate metadata for publishing, remove the rocksdb-cluster example, and reorganize imports in the sled-cluster examples.
Key changes:
- Documentation consolidated from separate crate into main d-engine crate
- Publishing enabled for d-engine-server, d-engine-client, d-engine-core, d-engine-proto
- rocksdb-cluster example removed (files deleted)
- Documentation links updated to point to docs.rs URLs
- Logging verbosity reduced for startup connection failures
Reviewed changes
Copilot reviewed 49 out of 67 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| d-engine/src/lib.rs | Simplified re-exports and added docs module |
| d-engine/src/docs/* | New documentation structure with guides |
| d-engine-*/Cargo.toml | Publishing enabled, descriptions updated |
| d-engine-*/README.md | New READMEs for all crates |
| examples/rocksdb-cluster/* | Entire example deleted |
| examples/*/README.md | Documentation links updated to docs.rs |
| examples/sled-cluster/src/* | Import paths reorganized |
| d-engine-server/src/*.rs | Logging levels reduced to debug |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CHANGELOG.md (1)
7-91: Remove or create the missingexamples/quick-start/example.The CHANGELOG references
examples/quick-start/as a new example for "5-minute single-node setup" (line 13), but this directory does not exist in the repository. The other three examples (single-node-expansion,service-discovery-embedded,service-discovery-standalone) and the migration guide exist as documented. Either create the quick-start example or remove its reference from the changelog.
🧹 Nitpick comments (5)
examples/three-nodes-cluster/src/main.rs (1)
93-96: Consider dynamic banner width for long config paths.The banner uses fixed-width formatting (
{config_path:<28}) which will cause misalignment if the config path exceeds 28 characters. Sinceconfig_pathcan be set via theCONFIG_PATHenvironment variable, users might provide longer paths.🔎 Proposed fix
Option 1: Dynamic box width based on path length:
- println!("╔════════════════════════════════════════╗"); - println!("║ d-engine Node Starting... ║"); - println!("║ Config: {config_path:<28} ║"); - println!("╚════════════════════════════════════════╝"); + let box_width = config_path.len().max(28) + 12; + let inner_width = box_width - 4; + println!("╔{}╗", "═".repeat(box_width)); + println!("║ d-engine Node Starting...{:width$}║", "", width = inner_width - 26); + println!("║ Config: {config_path:<width$} ║", width = inner_width - 9); + println!("╚{}╝", "═".repeat(box_width));Option 2: Truncate long paths with ellipsis:
+ let display_path = if config_path.len() > 28 { + format!("{}...", &config_path[..25]) + } else { + config_path.clone() + }; println!("╔════════════════════════════════════════╗"); println!("║ d-engine Node Starting... ║"); - println!("║ Config: {config_path:<28} ║"); + println!("║ Config: {display_path:<28} ║"); println!("╚════════════════════════════════════════╝");d-engine-proto/README.md (1)
38-38: Optional: Consider using proper headings instead of bold text.Static analysis detected that Lines 38 ("Step 1: Get the proto files") and 47 ("Step 2: Generate client code") use emphasis (bold) instead of proper markdown headings. While this is purely stylistic, using proper headings (
###or####) would improve document structure and accessibility.🔎 Proposed markdown structure improvement
-**Step 1: Get the proto files** +### Step 1: Get the proto files -**Step 2: Generate client code** +### Step 2: Generate client codeAlso applies to: 47-47
d-engine-client/src/lib.rs (2)
79-87: Glob re-exports may cause API instability.Using
pub use module::*for multiple modules risks unintended public API surface expansion and potential name collisions. Consider using explicit re-exports for the specific types you want to expose publicly, similar to the curatedprotocolandcluster_typesmodules below.🔎 Example of explicit re-exports
-pub use builder::*; -pub use cluster::*; -pub use config::*; -pub use error::*; -pub use grpc_kv_client::*; -pub use kv_client::*; -pub use kv_error::*; -pub use pool::*; -pub use utils::*; +pub use builder::ClientBuilder; +pub use cluster::ClusterClient; +pub use config::ClientConfig; +pub use error::ClientApiError; +pub use grpc_kv_client::GrpcKvClient; +pub use kv_client::KvClient; +pub use kv_error::KvError; +pub use pool::ConnectionPool; +// Only export utils that are part of the public API
152-158: Consider restrictingClientInnervisibility.
ClientInnerappears to be an implementation detail holding internal pool, config, and endpoint state. Exposing it aspub structallows external code to depend on these internals.🔎 Proposed fix
-#[derive(Clone)] -pub struct ClientInner { +#[derive(Clone)] +pub(crate) struct ClientInner { pool: ConnectionPool, client_id: u32, config: ClientConfig, endpoints: Vec<String>, }examples/quick-start-embedded/README.md (1)
125-140: Consider simplifying the byte conversion pattern in examples.The example uses
.as_bytes().to_vec()for string-to-bytes conversion. While correct, if the API acceptsimpl Into<Vec<u8>>, you could show a cleaner pattern. This is consistent with the code but could be more idiomatic.💡 Alternative if API supports it
// If put/get accept Into<Vec<u8>> client.put("user:1:name".into(), "alice".into()).await?; // or with explicit Vec<u8> client.put(b"user:1:name".to_vec(), b"alice".to_vec()).await?;However, verify the current pattern matches the actual API requirements.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockexamples/rocksdb-cluster/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
CHANGELOG.mdCargo.tomlMakefileREADME.mdbenches/d-engine-bench/README.mdbenches/d-engine-bench/reports/v0.2.0/report_v0.2.0_final.mdd-engine-client/Cargo.tomld-engine-client/README.mdd-engine-client/src/lib.rsd-engine-core/Cargo.tomld-engine-core/README.mdd-engine-core/src/lib.rsd-engine-core/src/storage/state_machine.rsd-engine-core/src/storage/storage_engine.rsd-engine-docs/Cargo.tomld-engine-docs/src/lib.rsd-engine-proto/Cargo.tomld-engine-proto/README.mdd-engine-proto/src/lib.rsd-engine-server/Cargo.tomld-engine-server/README.mdd-engine-server/src/lib.rsd-engine-server/src/membership/raft_membership.rsd-engine-server/src/network/health_checker.rsd-engine/Cargo.tomld-engine/src/docs/client_guide/error-handling.mdd-engine/src/docs/client_guide/mod.rsd-engine/src/docs/client_guide/read-consistency.mdd-engine/src/docs/examples/mod.rsd-engine/src/docs/examples/single-node-expansion.mdd-engine/src/docs/examples/three-nodes-cluster.mdd-engine/src/docs/integration-modes.mdd-engine/src/docs/mod.rsd-engine/src/docs/overview.mdd-engine/src/docs/performance/mod.rsd-engine/src/docs/performance/throughput-optimization-guide.mdd-engine/src/docs/quick-start-5min.mdd-engine/src/docs/quick-start-standalone.mdd-engine/src/docs/server_guide/consistency-tuning.mdd-engine/src/docs/server_guide/customize-state-machine.mdd-engine/src/docs/server_guide/customize-storage-engine.mdd-engine/src/docs/server_guide/mod.rsd-engine/src/docs/server_guide/watch-feature.mdd-engine/src/docs/use-cases.mdd-engine/src/lib.rsexamples/README.mdexamples/client_usage/README.mdexamples/quick-start-embedded/Makefileexamples/quick-start-embedded/README.mdexamples/quick-start-embedded/src/main.rsexamples/quick-start-standalone/README.mdexamples/rocksdb-cluster/.gitignoreexamples/rocksdb-cluster/Cargo.tomlexamples/rocksdb-cluster/Makefileexamples/rocksdb-cluster/config/n1.tomlexamples/rocksdb-cluster/config/n2.tomlexamples/rocksdb-cluster/config/n3.tomlexamples/rocksdb-cluster/src/main.rsexamples/service-discovery-embedded/README.mdexamples/service-discovery-standalone/README.mdexamples/single-node-expansion/README.mdexamples/sled-cluster/src/sled_engine_test.rsexamples/sled-cluster/src/sled_state_machine.rsexamples/three-nodes-cluster/Makefileexamples/three-nodes-cluster/src/main.rs
💤 Files with no reviewable changes (10)
- README.md
- examples/rocksdb-cluster/config/n3.toml
- examples/rocksdb-cluster/config/n2.toml
- examples/rocksdb-cluster/src/main.rs
- examples/rocksdb-cluster/Cargo.toml
- d-engine-docs/Cargo.toml
- examples/rocksdb-cluster/.gitignore
- examples/rocksdb-cluster/config/n1.toml
- examples/rocksdb-cluster/Makefile
- d-engine-docs/src/lib.rs
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
d-engine-proto/README.md
38-38: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
47-47: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: audit
- GitHub Check: Lint and Format Check
- GitHub Check: Analyze (rust)
- GitHub Check: audit
- GitHub Check: Lint and Format Check
🔇 Additional comments (47)
examples/client_usage/README.md (1)
1-31: LGTM!The removal of the node-join command is appropriate for a client usage example, which should focus on client operations rather than cluster management. The documentation is clear, features are well-listed, and the usage examples are consistent and practical. The example is ready for v0.2.0.
d-engine-server/src/network/health_checker.rs (1)
59-64: LGTM! Appropriate log level adjustment for startup-time failures.The change from error-level to debug-level logging correctly reflects that connection failures are expected when peers are still initializing. The explanatory comment makes the intent clear, and this reduces unhelpful noise in startup logs.
d-engine-server/src/membership/raft_membership.rs (2)
569-571: LGTM! Appropriate log level adjustment for pre-warming failures.Downgrading from warn to debug is correct here since connection pre-warming failures are expected during cluster startup when not all peers are ready yet. The comment clarifies the reasoning.
602-604: LGTM! Consistent log level adjustment for startup-time connection failures.This change mirrors the adjustment in
health_checker.rsand correctly treats startup-time connection failures as debug-worthy rather than warning-worthy. The explanatory comment makes the intent clear.examples/single-node-expansion/README.md (1)
178-178: The documentation URL is correctly structured and will resolve properly when published. The hyphen-to-underscore conversion (single-node-expansion.md→single_node_expansionmodule) follows standard Rust documentation conventions and is properly declared in the module hierarchy viapub mod single_node_expansion.benches/d-engine-bench/README.md (1)
15-15: Version update is correct.The dependency version in the example Cargo.toml snippet has been appropriately updated from "0.1.2" to "0.2.0" to align with the v0.2.0 release.
d-engine/src/docs/mod.rs (2)
12-31: Documentation module organization is well-structured.The module includes both inline documentation (via
include_str!macros for markdown files) and submodules for more complex guides. The use of fully qualifiedcrate::docs::*paths in the documentation links (lines 8-10) is idiomatic and preferred over relative paths.
8-10: All referenced documentation modules exist and are properly exported.All three modules (
client_guide,performance,server_guide) are correctly set up as subdirectories with mod.rs files at d-engine/src/docs/{module}/mod.rs, and the docs module is properly exported from lib.rs.examples/three-nodes-cluster/Makefile (1)
25-32: Excellent UX improvement for multi-node debugging.The addition of rocket emojis and per-node output prefixing (
[Node1],[Node2],[Node3]) viasedwill significantly improve log readability when running the cluster. The2>&1redirect ensures both stdout and stderr are captured and properly labeled.This pairs well with the startup banner added in
main.rsto provide clear visibility into which node is producing which output.Also applies to: 35-41, 44-50
d-engine-proto/src/lib.rs (1)
1-60: Documentation improvements are accurate and well-structured.All referenced paths and proto files have been verified:
- ✅
examples/quick-start-standaloneexample exists with proper structure- ✅ All proto files (
common.proto,error.proto,client/client_api.proto) present at referenced locations- ✅ Documentation organization by use case and language-specific instructions will improve developer experience
Changes are approved.
d-engine-core/src/lib.rs (1)
1-55: Excellent documentation strategy for an internal crate.The crate-level documentation effectively discourages direct usage while providing clear guidance for contributors. The
⚠️ warning, comparison examples, and reference to d-engine-server as a reference integration are particularly helpful.One minor suggestion: Consider adding a brief note about API stability guarantees (e.g., "Breaking changes may occur between 0.x versions") to set clear expectations alongside the "unstable before v1.0" statement on Line 10.
d-engine-server/src/lib.rs (1)
1-92: Documentation improvements significantly enhance usability.The restructured documentation clearly differentiates between embedded and standalone modes, provides concrete usage examples, and includes helpful guidance on when to use this crate versus the d-engine umbrella crate. The "When to use" sections (lines 5-21) are particularly effective for guiding users to the right API level.
Makefile (1)
38-38: Clean workspace simplification with consistent documentation path updates.The removal of
d-engine-docsfrom workspace members and the corresponding updates to documentation generation targets correctly reflect the consolidation of documentation into the maind-enginecrate. All path references have been consistently updated fromd_engine_docstod_engine.Also applies to: 385-398
examples/service-discovery-standalone/README.md (1)
3-77: Documentation restructuring effectively highlights Watch API patterns.The updated README successfully reframes the example to emphasize the Watch API as the primary learning objective, with service discovery as a practical application. The "What you'll learn" section (lines 5-9) and the Primary/Secondary concept organization (lines 66-77) make the educational goals immediately clear.
examples/service-discovery-embedded/README.md (1)
3-9: LGTM!The documentation updates effectively communicate the Watch API capabilities in embedded mode. The new sections clearly explain the latency benefits and architecture patterns.
Also applies to: 56-68, 72-74
d-engine-client/Cargo.toml (1)
6-6: LGTM!The updated description accurately reflects the crate's purpose, and enabling
publish = trueis appropriate for the v0.2.0 multi-crate release.Also applies to: 12-12
d-engine/src/lib.rs (2)
31-32: Module structure verified correctly.The
pub mod docs;declaration is properly supported byd-engine/src/docs/mod.rs, and the referenceddocs/overview.mdfile exists at the expected path.
4-10: No name collision risk between server and client feature-gated re-exports.The client crate exports ClientApiError and KvClientError (specific names), not generic Error or Result types. Both server and client features can be enabled simultaneously without naming conflicts—the server provides Error/Result from d_engine_core while the client provides only specific error types (ClientApiError, KvClientError). The prelude module already demonstrates the safe pattern by explicitly selecting types per feature.
Likely an incorrect or invalid review comment.
d-engine-core/src/storage/state_machine.rs (1)
1-1: Documentation path correctly updated for consolidated docs.The path change from
d-engine-docsto thed-enginecrate aligns with the PR's documentation restructuring. The markdown file exists at the new location with the correct relative path resolution.examples/quick-start-embedded/Makefile (1)
25-25: [Rewritten review comment]
[Exactly ONE classification tag]examples/sled-cluster/src/sled_state_machine.rs (1)
7-15: Import paths are correctly updated to match the module structure.All imports are valid and properly accessible:
d_engine::common::{Entry, LogId, entry_payload::Payload}— exported fromd-engine-server'spub mod commond_engine::client::{WriteCommand, write_command::{Delete, Insert, Operation}}— exported fromd-engine-server'spub mod clientd_engine::server_storage::SnapshotMetadata— exported fromd-engine-server'spub mod server_storageThe reorganized imports align with the consolidated re-export structure in
d-engine.d-engine-core/src/storage/storage_engine.rs (1)
1-1: The documentation file path has been correctly updated.The include path references the consolidated documentation in the d-engine crate at
d-engine/src/docs/server_guide/customize-storage-engine.md, and the file exists at the expected location. No remaining references to the old d-engine-docs path were found.d-engine/src/docs/server_guide/customize-state-machine.md (1)
209-209: Verify the example reference is correct.The documentation now references
examples/three-nodes-clusterinstead ofexamples/rocksdb-cluster. Ensure that:
- The
three-nodes-clusterexample exists in the repository- It demonstrates a custom StateMachine implementation as the documentation claims
- The old
rocksdb-clusterexample has been removed as expectedexamples/quick-start-standalone/README.md (1)
37-37: The docs.rs URL is correctly configured.The documentation module structure in d-engine matches the URL path. The markdown file exists at
d-engine/src/docs/quick-start-standalone.md, is properly declared aspub mod quick_start_standalonewithinclude_str!, and the docs module is publicly exposed inlib.rs. Once the crate is published to crates.io, docs.rs will generate the documented URL correctly.d-engine-server/Cargo.toml (1)
12-12: Crate is ready for publishing.The crate has all required publishing metadata: README.md (245 lines), comprehensive lib.rs documentation with usage guidance, proper Cargo.toml metadata (description, keywords, categories), and docs.rs configuration. The
publish = truechange is appropriate.d-engine-client/README.md (1)
1-43: Excellent README structure and clarity!The README effectively guides users toward the simpler
d-enginecrate with theclientfeature, while explaining the architectural rationale for contributors. The warning section is prominent and helpful.Verification confirms:
- The
clientfeature is correctly defined in d-engine's Cargo.toml- The referenced GitHub URL (https://github.com/deventlab/d-engine/blob/main/README.md) appropriately points to the main README
- The root README.md documents the client feature and v0.2 usage consistently
- Version constraints ("0.2") work correctly with workspace versioning
- Other workspace crates maintain similar README patterns
d-engine/src/docs/overview.md (3)
64-94: LGTM - Documentation index is well-organized.The role-based documentation organization (Client Developers, Server Operators, Examples & Performance) provides clear navigation paths for different user types. The dual MIT/Apache-2.0 licensing is clearly stated.
55-62: Documentation reference is accurate.The
examples/three-nodes-clusterdirectory exists with themake start-clustertarget properly defined.
7-24: All intra-doc links resolve correctly.Verification confirms that all referenced documentation modules are properly exported. Every link in overview.md (use_cases, quick_start_5min, quick_start_standalone, integration_modes, client_guide submodules, server_guide submodules, performance submodules, and examples submodules) has a corresponding pub mod declaration in the appropriate mod.rs file and is accessible via the crate::docs namespace.
d-engine-core/Cargo.toml (1)
6-6: Consider alignment between publish flag and "internal crate" messaging.The Cargo.toml enables publishing (
publish = true) and the description positions this as a building block ("for building custom Raft-based systems"), but the README (d-engine-core/README.md) warns users this is an "Internal Crate - Not Ready for Standalone Use" with an "unstable API before v1.0."This seems intentional to support advanced users building custom Raft systems while discouraging casual use, but verify this mixed messaging aligns with your release strategy. Consider if the description should include a stability caveat like "Pure Raft consensus algorithm (unstable API) - for building custom Raft-based systems."
Also applies to: 12-12
d-engine-core/README.md (3)
10-26: LGTM - Clear messaging to use d-engine instead.The warning section effectively communicates that d-engine-core is internal and directs users to the main d-engine crate. The side-by-side code examples (❌ Don't / ✅ Do) make the guidance immediately actionable.
28-46: LGTM - Helpful contributor context.The contributor section provides clear guidance on the core abstractions (StorageEngine, StateMachine, LogStore, MetaStore) and links to the reference integration in d-engine-server. This strikes the right balance for contributors who need to understand the internals.
62-63: These documentation links are valid and will resolve correctly.The
/latest/alias is fully supported by docs.rs, and the module structure (docs/server_guide/customize_storage_engineanddocs/server_guide/customize_state_machine) is correctly defined as public modules in the codebase. The URLs follow the proper docs.rs format and will be accessible after publishing.d-engine/Cargo.toml (1)
6-6: LGTM - Clear positioning as the main entry point.The updated description succinctly identifies d-engine as the "recommended entry point for most users," which aligns well with the guidance in d-engine-core's README directing users away from the core crate.
d-engine-proto/Cargo.toml (1)
6-6: LGTM - Proto crate appropriately positioned for multi-language clients.The updated description clearly communicates the purpose ("for building non-Rust d-engine clients") and setting
publish = trueis appropriate since proto definitions are meant to be consumed by external client implementations in various languages.Also applies to: 12-12
d-engine-server/README.md (5)
1-32: Excellent user guidance structure!The "When to use" vs "When NOT to use" sections effectively guide users to the appropriate crate for their needs. This reduces confusion and improves the developer experience.
95-125: Clear architecture visualization!The ASCII diagram effectively communicates the server's layered architecture and component relationships.
136-156: LGTM!The storage backend examples correctly demonstrate both file-based and RocksDB configurations with appropriate Arc wrapping for shared ownership.
46-61: The API examples in the README are accurate and match the current public API signatures.
EmbeddedEngine::start_with(config_path: &str) -> Result<Self>✓engine.wait_ready(timeout: Duration) -> Result<LeaderInfo>✓StandaloneServer::run(shutdown_rx: watch::Receiver<()>) -> Result<()>✓All method signatures are correct as shown in the code examples.
215-228: All referenced example directories exist in the repository.The example directories are correctly linked:
examples/single-node-expansion/✓examples/three-nodes-cluster/✓examples/quick-start-embedded/✓The GitHub links in the README are valid. Note that the docs.rs links are external references that cannot be verified in the sandbox environment, but they follow standard documentation hosting conventions.
examples/README.md (2)
69-79: Clear and concise run instructions!The commands are straightforward and correctly reference individual example READMEs for detailed instructions.
11-56: All referenced example directories exist.Verification confirms that all 8 example directories referenced in the README (lines 11-56) are present and properly aligned with the documentation:
- quick-start-embedded ✓
- quick-start-standalone ✓
- service-discovery-embedded ✓
- service-discovery-standalone ✓
- client_usage ✓
- three-nodes-cluster ✓
- single-node-expansion ✓
- sled-cluster ✓
Cargo.toml (2)
34-37: LGTM! Workspace dependencies correctly configured for publishing.Adding explicit
version = "0.2.0"to workspace dependencies enables crates.io publishing while maintaining local path resolution for development. This is the correct pattern for publishable workspace crates.
10-11: d-engine-docs crate fully removed and documentation properly consolidated.Verification confirms:
- d-engine-docs directory completely removed
- No d-engine-docs references remaining in codebase
- Documentation successfully consolidated into d-engine/src/docs/ (19 files across multiple guides)
- Root Cargo.toml correctly excludes "docs" directory
- d-engine-docs removed from workspace members list
- No broken imports or references
Migration complete.
examples/quick-start-embedded/README.md (2)
84-102: Clear API demonstration with correct method calls.The code snippets accurately show the EmbeddedEngine workflow and correctly distinguish between
put(strong consistency) andget_eventual(eventual consistency) operations.
1-80: Documentation accurately reflects code implementation.The README output examples, logging configuration, and feature descriptions match the EmbeddedEngine code exactly. The data directory path, node startup messages, and task demo all execute as documented. The docs.rs URL reference is valid and points to the existing
single_node_expansionmodule in the documentation structure.examples/quick-start-embedded/src/main.rs (1)
97-97: Verify the documentation URL path is correct.The URL references
https://docs.rs/d-engine/latest/d_engine/docs/examples/single_node_expansion/index.html. Ensure this documentation path exists in the published docs after the consolidation and that the link resolves correctly.
- Fix protoc working directory in d-engine-proto README (should be in d-engine-proto, not proto/ subdirectory) - Update Rust version requirement from 1.70+ to 1.85+ to match edition 2024 - Fix inconsistent imports in sled-cluster test (use d_engine_proto::client consistently across codebase)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
examples/sled-cluster/src/sled_engine_test.rs (1)
88-91: Consider increasing the timeout threshold for CI reliability.The 5-second timeout for 10,000 entries may be too strict for resource-constrained CI environments, potentially causing flaky test failures. Consider increasing to 10 seconds or making it configurable via an environment variable.
🔎 Proposed adjustment
assert!( - duration.as_millis() < 5000, - "Should persist 10k entries in <5s" + duration.as_millis() < 10000, + "Should persist 10k entries in <10s" );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
d-engine-proto/README.mdexamples/README.mdexamples/sled-cluster/src/sled_engine_test.rs
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
d-engine-proto/README.md
38-38: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
47-47: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Lint and Format Check
- GitHub Check: audit
- GitHub Check: Analyze (rust)
- GitHub Check: Lint and Format Check
- GitHub Check: audit
🔇 Additional comments (9)
d-engine-proto/README.md (2)
42-78: ✅ Working directory expectations are now clear and consistent.The past review comment about working directory inconsistency has been resolved. Line 44 correctly specifies
cd d-engine/d-engine-proto, and the subsequent protoc commands (lines 56–64) properly reference files with theproto/prefix relative to that directory. Line 75 adds a helpful clarifying note about working directory assumptions.
1-122: Comprehensive README with clear audience segmentation and language support transparency.The README effectively guides both Rust and non-Rust developers, clearly delineates when to use this crate vs. higher-level alternatives, provides tested examples (Go), and honestly documents language support status (Python pending integration). The restructured Quick Start section and clarified working directory instructions make proto generation accessible across multiple languages. Overall structure and content align well with the v0.2.0 publishing goals.
examples/README.md (3)
60-66: ✅ Rust version requirement correctly updated to match edition 2024.Line 62 now correctly specifies "Rust: 1.85+" to align with the codebase's use of
edition = "2024"(which requires Rust 1.85+), addressing the previous review comment. The other version requirements are also clearly specified.
1-3: Minor: Verify title casing convention is consistent with crate naming.Line 1 uses lowercase "d-engine" in the title. Confirm this matches the official project naming convention used elsewhere in primary crate documentation and Cargo.toml.
11-57: All example directories referenced in the README exist and are properly maintained.Verification confirms:
- All 8 referenced examples exist (quick-start-embedded, quick-start-standalone, service-discovery-embedded, service-discovery-standalone, client_usage, three-nodes-cluster, single-node-expansion, sled-cluster) ✓
- rocksdb-cluster was properly removed ✓
- No broken or stale references remain ✓
Minor observation: sled-cluster does not have a README.md file, unlike the other examples.
examples/sled-cluster/src/sled_engine_test.rs (4)
16-35: LGTM!The builder pattern with
TempDirand UUID-based unique paths is well-designed for test isolation and automatic cleanup.
37-65: LGTM!The basic test provides appropriate smoke test coverage for persist and retrieve operations.
96-118: LGTM!The helper function correctly constructs and encodes protobuf write commands for test entries. The use of
.expect()for encoding errors is appropriate in test code.
3-8: Imports correctly use proto-generated module structure.The imports properly reference
d_engine_proto::clientandd_engine_proto::commontypes. Both these direct imports and the equivalentd_engine::commonpattern used in sibling files are functionally correct, asd_engine::commonre-exports fromd_engine_proto::commonthrough thed_engine_servercrate.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Type
Related Issues
Checklist
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.