WIP: block/repair.rs: Initial implementation of scrub checkpointing #582

jpds · 2023-06-08T13:26:02Z

jpds commented

2023-06-08 13:26:02 +00:00

Fixes: #518.

jpds added 1 commit 2023-06-08 13:26:03 +00:00

continuous-integration/drone/pr Build is passing Details

41d4817eeb block/repair.rs: Initial implementation of scrub checkpointing.

jpds commented

2023-06-08 13:27:22 +00:00

@lx I have tunnel-vision from looking at this code for too long - can you please take a look and tell me what would be the best way to hook the last_hash in next() to the last_path ?

@lx I have tunnel-vision from looking at this code for too long - can you please take a look and tell me what would be the best way to hook the `last_hash` in `next()` to the `last_path` ?

lx commented

2023-06-09 09:12:12 +00:00

Here is my suggestion:

keep the next() function as before, with zero argument
add to the BlockStoreIterator a function skip_until(hash: Hash) that reads the root directory, skips until the first subdirectory relevant to the given hash, and then does the same for sub-subdirectories

OR maybe there's a simpler way of doing it by adding a field start_at: Hash in the BlockStoreIterator itself, and then at each time a subdirectory is read, pos is initialized not to zero but to the first position that is relevant given the hash we want to start at (on the line *last_path = ReadingDir::Read { subpaths, pos: 0 };)
in ScrubWorker::new, put something like this:

let work = match persisted.last_hash_checked {
    None => ScrubWorkerState::Finished,
    Some(hash) => {
        let mut it = BlockStoreIterator::new(...);
        it.skip_until(hash).await?;
        ScrubWorkerState::Running(it)
    }
};
Self {
    work,
    ...
}

(this will require making the new function async and making it return a Result)

Here is my suggestion: - keep the `next()` function as before, with zero argument - add to the `BlockStoreIterator` a function `skip_until(hash: Hash)` that reads the root directory, skips until the first subdirectory relevant to the given hash, and then does the same for sub-subdirectories OR maybe there's a simpler way of doing it by adding a field `start_at: Hash` in the BlockStoreIterator itself, and then at each time a subdirectory is read, `pos` is initialized not to zero but to the first position that is relevant given the hash we want to start at (on the line `*last_path = ReadingDir::Read { subpaths, pos: 0 };`) - in `ScrubWorker::new`, put something like this: ```rust let work = match persisted.last_hash_checked { None => ScrubWorkerState::Finished, Some(hash) => { let mut it = BlockStoreIterator::new(...); it.skip_until(hash).await?; ScrubWorkerState::Running(it) } }; Self { work, ... } ``` (this will require making the `new` function async and making it return a Result)

lx reviewed 2023-06-09 09:13:28 +00:00

src/block/repair.rs

					
				@ -205,3 +205,3 @@

				}

				pub use v082::*;

				mod v083 {

lx commented

2023-06-09 09:13:28 +00:00

BTW you do not need to add this whole module with a new migration. Since the value of last_hash_checked if it was not present before is its Default::default() value (None), you can just add it to the struct in v082 with the attribute #[serde(default)]

BTW you do not need to add this whole module with a new migration. Since the value of `last_hash_checked` if it was not present before is its `Default::default()` value (`None`), you can just add it to the struct in `v082` with the attribute `#[serde(default)]`

lx commented

2023-09-04 14:12:06 +00:00

I will implement this as part of #625, as I am making some changes to the block store iterator for multi-hdd support which incidentally make it much easier to implement checkpointing.

lx closed this pull request

2023-09-04 14:12:06 +00:00

lx referenced this pull request

2023-09-04 14:12:46 +00:00

multi-hdd support (fix #218) #625

continuous-integration/drone/pr Build is passing

Required

Details