Commit 5341a1b
perf(l1): replace synchronous disk I/O with async operations in snap sync (#6113)
## Motivation
During snap sync, multiple file system operations block the tokio
runtime:
- Directory creation (`std::fs::create_dir_all`)
- File reading (`std::fs::read`)
- Directory listing (`std::fs::read_dir`)
- Directory removal (`std::fs::remove_dir_all`)
These synchronous operations cause the async runtime to stall,
preventing network operations from proceeding while disk I/O completes.
This is particularly problematic during:
1. Snapshot file writing (account states, storage states)
2. Snapshot file reading during trie insertion
3. Cleanup after insertion completes
This PR addresses **roadmap item 1.6 (Async Disk I/O)** from the snap
sync improvement plan.
## Description
### Approach: Hybrid Async Strategy
We use a hybrid approach for async file operations:
| Operation | Method | Rationale |
|-----------|--------|-----------|
| `create_dir_all` | `tokio::fs` | Simple, no iterator complexity |
| `read` | `tokio::fs` | Simple, direct async support |
| `remove_dir_all` | `tokio::fs` | Simple, direct async support |
| `read_dir` | `spawn_blocking` | Returns iterator with complex
lifetimes; wrapping in spawn_blocking is cleaner than manual async
iteration |
### New Module: `snap/async_fs.rs`
Created a new module with async wrappers that provide consistent error
handling:
```rust
/// Ensures a directory exists, creating it if necessary.
pub async fn ensure_dir_exists(path: &Path) -> Result<(), SnapError>
/// Reads all file paths from a directory (sorted for deterministic ordering).
pub async fn read_dir_paths(dir: &Path) -> Result<Vec<PathBuf>, SnapError>
/// Reads the contents of a file asynchronously.
pub async fn read_file(path: &Path) -> Result<Vec<u8>, SnapError>
/// Removes a directory and all its contents asynchronously.
pub async fn remove_dir_all(path: &Path) -> Result<(), SnapError>
/// Writes data to a file asynchronously.
pub async fn write_file(path: &Path, contents: &[u8]) -> Result<(), SnapError>
/// Removes a directory if it exists, ignoring NotFound errors.
pub async fn remove_dir_all_if_exists(path: &Path) -> Result<(), SnapError>
```
All functions return `SnapError::FileSystem` with:
- Operation name (for debugging)
- Path (to identify which file/directory failed)
- Error kind (from `std::io::ErrorKind`)
### Changes to `snap/client.rs`
**Before (4 occurrences):**
```rust
if !std::fs::exists(account_state_snapshots_dir).map_err(|_| {
SnapError::SnapshotDir("State snapshots directory does not exist".to_string())
})? {
std::fs::create_dir_all(account_state_snapshots_dir).map_err(|_| {
SnapError::SnapshotDir("Failed to create state snapshots directory".to_string())
})?;
}
```
**After:**
```rust
async_fs::ensure_dir_exists(account_state_snapshots_dir).await?;
```
Locations updated:
1. `request_account_range()` - account state snapshot directory (loop)
2. `request_account_range()` - account state snapshot directory (final
flush)
3. `request_storage_ranges()` - storage snapshot directory (loop)
4. `request_storage_ranges()` - storage snapshot directory (final flush)
### Changes to `sync/snap_sync.rs`
#### Code Hashes Processing
**Before:**
```rust
std::fs::create_dir_all(&code_hashes_snapshot_dir).map_err(|_| SyncError::CorruptPath)?;
// ...
for entry in std::fs::read_dir(&code_hashes_dir)
.map_err(|_| SyncError::CodeHashesSnapshotsDirNotFound)?
{
let entry = entry.map_err(|_| SyncError::CorruptPath)?;
let snapshot_contents = std::fs::read(entry.path())
.map_err(|err| SyncError::SnapshotReadError(entry.path(), err))?;
// ...
}
std::fs::remove_dir_all(code_hashes_dir)
.map_err(|_| SyncError::CodeHashesSnapshotsDirNotFound)?;
```
**After:**
```rust
async_fs::ensure_dir_exists(&code_hashes_snapshot_dir).await?;
// ...
let code_hash_files = async_fs::read_dir_paths(&code_hashes_dir).await?;
for file_path in code_hash_files {
let snapshot_contents = async_fs::read_file(&file_path).await?;
// ...
}
async_fs::remove_dir_all(&code_hashes_dir).await?;
```
#### Account Insertion (non-rocksdb)
**Before:**
```rust
for entry in std::fs::read_dir(account_state_snapshots_dir)
.map_err(|_| SyncError::AccountStateSnapshotsDirNotFound)?
{
let entry = entry.map_err(|err| SyncError::SnapshotReadError(...))?;
let snapshot_path = entry.path();
let snapshot_contents = std::fs::read(&snapshot_path)
.map_err(|err| SyncError::SnapshotReadError(...))?;
// ...
}
std::fs::remove_dir_all(account_state_snapshots_dir)
.map_err(|_| SyncError::AccountStoragesSnapshotsDirNotFound)?;
```
**After:**
```rust
let snapshot_files = async_fs::read_dir_paths(account_state_snapshots_dir).await?;
for snapshot_path in snapshot_files {
let snapshot_contents = async_fs::read_file(&snapshot_path).await?;
// ...
}
async_fs::remove_dir_all(account_state_snapshots_dir).await?;
```
#### Storage Insertion (non-rocksdb)
Same pattern as account insertion - replaced `read_dir` + `read` +
`remove_dir_all` with async equivalents.
#### Account Insertion (rocksdb)
**Before:**
```rust
let file_paths: Vec<PathBuf> = std::fs::read_dir(account_state_snapshots_dir)
.map_err(|_| SyncError::AccountStateSnapshotsDirNotFound)?
.collect::<Result<Vec<_>, _>>()
.map_err(|_| SyncError::AccountStateSnapshotsDirNotFound)?
.into_iter()
.map(|res| res.path())
.collect();
// ...
std::fs::remove_dir_all(account_state_snapshots_dir)
.map_err(|_| SyncError::AccountStateSnapshotsDirNotFound)?;
std::fs::remove_dir_all(get_rocksdb_temp_accounts_dir(datadir))
.map_err(|e| SyncError::AccountTempDBDirNotFound(e.to_string()))?;
```
**After:**
```rust
let file_paths: Vec<PathBuf> = async_fs::read_dir_paths(account_state_snapshots_dir).await?;
// ...
async_fs::remove_dir_all(account_state_snapshots_dir).await?;
async_fs::remove_dir_all(&get_rocksdb_temp_accounts_dir(datadir)).await?;
```
#### Storage Insertion (rocksdb)
Same pattern - replaced sync fs operations with async equivalents.
### Summary of Changes
| File | Before | After | Change |
|------|--------|-------|--------|
| `snap/async_fs.rs` | - | 189 lines | New module |
| `snap/mod.rs` | - | +1 line | Module export |
| `snap/client.rs` | 42 lines | 4 lines | -38 lines (4 patterns
simplified) |
| `sync/snap_sync.rs` | 82 lines | 16 lines | -66 lines (16 calls
simplified) |
| `Cargo.toml` | - | +1 line | tempfile dev-dependency |
| `Cargo.lock` | - | +1 line | tempfile entry |
**Net change:** +223 lines, -93 lines = +130 lines (including 189 lines
of new module with tests)
## How to Test
### Unit Tests
Run the async_fs module tests:
```bash
cargo test -p ethrex-p2p async_fs
```
Expected output:
```
running 6 tests
test snap::async_fs::tests::test_remove_dir_all_if_exists ... ok
test snap::async_fs::tests::test_ensure_dir_exists_idempotent ... ok
test snap::async_fs::tests::test_remove_dir_all ... ok
test snap::async_fs::tests::test_ensure_dir_exists_creates_new ... ok
test snap::async_fs::tests::test_read_write_file ... ok
test snap::async_fs::tests::test_read_dir_paths ... ok
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 38 filtered out
```
### Compilation Tests
Verify both feature configurations compile:
```bash
# Without rocksdb (uses non-rocksdb insert_accounts/insert_storages)
cargo check -p ethrex-p2p
# With rocksdb (uses rocksdb-specific insert_accounts/insert_storages)
cargo check -p ethrex-p2p --features rocksdb
```
### Integration Testing
Run a snap sync on testnet to verify:
1. Account state snapshots are written and read correctly
2. Storage snapshots are written and read correctly
3. Code hash snapshots are written and read correctly
4. All cleanup operations complete successfully
5. No tokio runtime blocking warnings appear in logs
```bash
# Example: Run snap sync on Holesky
make start-holesky-metrics-docker
```
### Performance Verification
To verify async I/O is working:
1. Monitor CPU usage during snapshot write/read phases
2. Verify network operations continue during disk I/O
3. Check that `tokio::time::sleep` polling loops respond promptly
## Files Changed
| File | Description |
|------|-------------|
| `crates/networking/p2p/snap/async_fs.rs` | New async filesystem
helpers module |
| `crates/networking/p2p/snap/mod.rs` | Export async_fs module |
| `crates/networking/p2p/snap/client.rs` | Use async_fs for directory
creation |
| `crates/networking/p2p/sync/snap_sync.rs` | Use async_fs for all file
operations |
| `crates/networking/p2p/Cargo.toml` | Add tempfile dev-dependency |
| `Cargo.lock` | Updated lockfile |
## Checklist
- [x] Code compiles without rocksdb feature
- [x] Code compiles with rocksdb feature
- [x] All async_fs unit tests pass
- [x] No remaining `std::fs::` calls in snap sync code paths
- [x] Error handling preserved (SnapError::FileSystem)
- [x] Deterministic ordering maintained (read_dir_paths sorts results)
## Related
- Snap Sync Roadmap: Item 1.6 (Async Disk I/O)
- Parent branch: `refactor/snapsync-healing-unification`
---------
Co-authored-by: Esteban Dimitroff Hodi <esteban.dimitroff@lambdaclass.com>
Co-authored-by: ElFantasma <estebandh@gmail.com>1 parent fee57b1 commit 5341a1b
8 files changed
Lines changed: 205 additions & 161 deletions
File tree
- crates/networking/p2p
- snap
- sync
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
5 | 9 | | |
6 | 10 | | |
7 | 11 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
58 | 58 | | |
59 | 59 | | |
60 | 60 | | |
| 61 | + | |
61 | 62 | | |
62 | 63 | | |
63 | 64 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
| |||
160 | 160 | | |
161 | 161 | | |
162 | 162 | | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
| 163 | + | |
170 | 164 | | |
171 | 165 | | |
172 | 166 | | |
| |||
289 | 283 | | |
290 | 284 | | |
291 | 285 | | |
292 | | - | |
293 | | - | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
298 | | - | |
| 286 | + | |
299 | 287 | | |
300 | 288 | | |
301 | 289 | | |
| |||
603 | 591 | | |
604 | 592 | | |
605 | 593 | | |
606 | | - | |
607 | | - | |
608 | | - | |
609 | | - | |
610 | | - | |
611 | | - | |
612 | | - | |
613 | | - | |
614 | | - | |
| 594 | + | |
| 595 | + | |
615 | 596 | | |
616 | 597 | | |
617 | 598 | | |
| |||
996 | 977 | | |
997 | 978 | | |
998 | 979 | | |
999 | | - | |
1000 | | - | |
1001 | | - | |
1002 | | - | |
1003 | | - | |
1004 | | - | |
1005 | | - | |
| 980 | + | |
| 981 | + | |
1006 | 982 | | |
1007 | 983 | | |
1008 | 984 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
| 15 | + | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
| |||
0 commit comments