feat: introduce num wal files to keep #25801
Conversation
influxdb3/src/commands/serve.rs
Outdated
| /// Number of wal files to keep behind, wal flush does not clear the wal files immediately | ||
| /// instead they are only deleted when num wal files count exceed this keep behind size | ||
| #[clap( | ||
| long = "num-wal-files-to-keep", |
There was a problem hiding this comment.
should be renamed to snapshotted-wal-files-to-keep. It's not the actual total number of WAL files, it's the number that have been snapshotted. So if you have 200 files that haven't been snapshotted and this setting is 50, you'd have 250 total files at that point.
influxdb3_wal/src/object_store.rs
Outdated
| use tokio::sync::Mutex; | ||
| use tokio::sync::{oneshot, OwnedSemaphorePermit, Semaphore}; | ||
|
|
||
| // struct WalObjectStoreArgs { |
influxdb3_wal/src/object_store.rs
Outdated
| @@ -25,6 +36,11 @@ pub struct WalObjectStore { | |||
| added_file_notifiers: parking_lot::Mutex<Vec<Arc<dyn WalFileNotifier>>>, | |||
| /// Buffered wal ops go in here along with the state to track when to snapshot | |||
| flush_buffer: Mutex<FlushBuffer>, | |||
| /// oldest, last and latest wal file nums are used to keep track of what wal files | |||
| /// to remove from the OS | |||
| num_wal_files_to_keep: u64, | |||
There was a problem hiding this comment.
same, rename this so it indicates that it's the number of snapshotted wal files to keep.
| all_wal_file_paths | ||
| .iter() | ||
| .filter(|path| { | ||
| if let Some(last_wal_sequence_number) = last_wal_sequence_number { |
There was a problem hiding this comment.
One thing I'm not sure of looking through this, does the persisted snapshot have the last known wal file number saved, or is it the wal file number it's snapshotting to? It should have both, but the one we care about here is the one it's snapshotting to. Can you verify?
There was a problem hiding this comment.
The persisted snapshot file carries the wal file number that came in the current wal contents, that's why @hiltontj had to use >= instead of >. SnapshotDetails holds the last_wal_file_sequence_number (which is based on wal period we're snapshotting). If we used the one coming in SnapshotDetails we can then say load files only > this wal file number
There was a problem hiding this comment.
Ok, so we need to update so that we're also keeping a record of what WAL file we're snapshotting to. That way we initialize with the correct one (i.e. not the latest WAL file written). In the case of a forced snapshot, those numbers will be the same, but in the case of normal snapshots, the snapshot number will be < latest wal number.
There was a problem hiding this comment.
addressed it in commit - good spot, thanks.
| } | ||
|
|
||
| if path_count == paths.len() { | ||
| break; |
There was a problem hiding this comment.
should we sort before this so that paths always returns sorted?
There was a problem hiding this comment.
This is from existing code, I thought it's doing the right already - yes I can sort it.
influxdb3_wal/src/object_store.rs
Outdated
| #[derive(Debug)] | ||
| struct WalFileRemover { | ||
| oldest_wal_file: Option<WalFileSequenceNumber>, | ||
| last_wal_sequence_number: Option<WalFileSequenceNumber>, |
There was a problem hiding this comment.
rename to last_snapshotted_wal_sequence_number for clarity. The latest wal file number will always be >= this one.
This commit allows a configurable number of wal files to be left behind in OS. This is necessary as enterprise replicas rely on these files. closes: #25788
praveen-influx
force-pushed
the
praveen/delete-wal-background
branch
2 times, most recently
from
80a61d5 to
9096eee
Compare
praveen-influx
force-pushed
the
praveen/delete-wal-background
branch
from
9096eee to
5e76230
Compare
praveen-influx
force-pushed
the
praveen/delete-wal-background
branch
from
2f6f29a to
a03f3fb
Compare
This commit allows a configurable number of wal files to be left behind in OS. This is necessary as enterprise replicas rely on these files.
closes: #25788