Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR significantly improves the sync command by transitioning from a simple flat Merkle tree comparison to a recursive prefix-based synchronization approach. The new implementation uses tree traversal with configurable fanout and depth limits for more efficient synchronization of large datasets.
Key changes:
- Replaced flat Merkle tree diff with recursive prefix-based synchronization using a 64-character fanout
- Added support for remote HASH commands to compare subtree hashes before full reconciliation
- Introduced configurable max depth and leaf threshold parameters with sensible defaults
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| /// GET one key: "GET <key>\r\n" → "VALUE <plain>\r\n" or "NOT_FOUND\r\n" | ||
| async fn read_remote_value_plain(&self, addr: &str, key: &str) -> Result<Option<String>> { | ||
| /// GET key → "VALUE <val>" hoặc "NOT_FOUND" |
There was a problem hiding this comment.
The comment contains Vietnamese text 'hoặc' which should be 'or' in English to maintain consistency with the rest of the codebase.
| /// GET key → "VALUE <val>" hoặc "NOT_FOUND" | |
| /// GET key → "VALUE <val>" or "NOT_FOUND" |
| let (t, _map) = self.build_local_merkle_snapshot(prefix).await?; | ||
| Ok(match t.get_root_hash() { | ||
| Some(h) => to_hex(h), | ||
| None => "0".repeat(64), |
There was a problem hiding this comment.
Magic number 64 should be defined as a constant (e.g., const HASH_HEX_LENGTH: usize = 64) since it's used multiple times in the code for hash validation and generation.
| if hex.len() != 64 { | ||
| return Err(anyhow!("bad HASH hex length: {}", hex)); | ||
| } |
There was a problem hiding this comment.
This uses the same magic number 64 as in the previous comment. Both instances should reference a shared constant for hash hex length.
| fn sync_prefix_recursive<'a>( | ||
| &'a self, | ||
| addr: &'a str, | ||
| prefix: String, | ||
| depth: usize, | ||
| ) -> Pin<Box<dyn Future<Output = Result<()>> + Send + 'a>> { |
There was a problem hiding this comment.
The prefix parameter should be &str instead of String to avoid unnecessary allocations during recursive calls. The function can clone the prefix only when needed for the recursive call.
No description provided.