Improve WAL mapper stability under I/O pressure#110
Conversation
zablotchi
commented
Jan 8, 2026
zablotchi
commented
- Add WalCleanupThread for background file deletion and Arc dropping
- Increase INITIAL_MAPS_BUFFER from 2 to 10
- Increase get_writeable_map timeout from 10s to 60s
- Update tests to use max_maps >= 16 to accommodate increased buffer
- Add WalCleanupThread for background file deletion and Arc dropping - Increase INITIAL_MAPS_BUFFER from 2 to 10 - Increase get_writeable_map timeout from 10s to 60s - Update tests to use max_maps >= 16 to accommodate increased buffer
andll
left a comment
andll
left a comment
There was a problem hiding this comment.
This PR actually have quite a bit of changes that simply rename variables/functions etc. This makes PR hard to review, because for every line you need to figure out if this is just renaming or some other change. Is it possible to rollback all the naming changes? If we think some names are confusing we can rename in a separate PR
tidehunter/src/wal/mod.rs
Outdated
| const MAX_ATTEMPTS: usize = 60 * 1000; | ||
| for _ in 0..MAX_ATTEMPTS { | ||
| let Some(map) = self.wal.get_map(map) else { | ||
| let Some(map) = self.wal.get_map(map_id) else { |
There was a problem hiding this comment.
This change just renames variable, lets avoid changes like that
| DropMaps(Arc<WalMaps>), | ||
| } | ||
|
|
||
| pub(crate) struct WalCleanupThread { |
There was a problem hiding this comment.
If you look at wal mapper and others, the naming convention is usually this:
- Struct that hold controls and used externally is called
WalMapper - Struct that hold data needed for thread is called
WalMapperThread
Since we use this pattern universally can you name structs in this file in similar manner (e.g. WalCleanup and WalCleanupThread)
|
|
||
| impl WalCleanupWorker { | ||
| pub fn run(self) { | ||
| while let Ok(msg) = self.receiver.recv() { |
There was a problem hiding this comment.
Can you add thread utilization metric here similar to other threads like mapper etc?
| std::fs::remove_file(&path).expect("Failed to remove wal file"); | ||
| } | ||
| WalCleanupMessage::DropMaps(maps) => { | ||
| drop(maps); // Explicit drop; munmap happens here in background |
There was a problem hiding this comment.
Maybe remove 'in background' since its not clear what it means
andll
force-pushed
the
pr/wal-mapper-stability
branch
from
d0eaa4e to
1acceb6
Compare
2 participants
