diff --git a/HISTORY b/HISTORY index 01b3a984..ba9c14ce 100644 --- a/HISTORY +++ b/HISTORY @@ -24,6 +24,13 @@ SnapRAID HISTORY By scaling to 512-bit ZMM registers, the implementation now processes 64 bytes per iteration, effectively doubling the data width of the previous AVX2 routines. + * Improved the chances of a successful recovery after an interrupted + sync. This is achieved by removing the sync optimization of not + recomputing the parity if the input data matches the expected hash. + While only the specific sync immediately following an interruption + will be slower, this change preserves hash data in the content file + to ensure a better fix during disk failures and allows the --import + option to remain functional even after an incomplete sync. 14.6 2026/06 ============ diff --git a/cmdline/elem.h b/cmdline/elem.h index 97404f7a..8c1d72cf 100644 --- a/cmdline/elem.h +++ b/cmdline/elem.h @@ -75,8 +75,8 @@ struct snapraid_filter { * The block has both the hash and the parity computed. * This is the normal state of a saved block. * - * The block hash field IS set. - * The parity for this disk is updated. + * The block hash is the hash of the CURRENT data. + * The parity for this block IS valid. */ #define BLOCK_STATE_BLK 1 @@ -85,42 +85,33 @@ struct snapraid_filter { * This happens when a new block overwrites a just removed block, or an empty * space. * - * The block hash field MAY be set and it represents the hash of the OLD data. - * The hash may be also INVALID or ZERO. + * The block hash is the hash of the OLD data. Possible values are: + * - ZERO -> The OLD block was empty + * - INVALID -> The OLD block has an unknown hash + * - Otherwise -> The OLD block has this hash * - * If the OLD block was empty, the hash is set to the special ZERO value. - * If the OLD block was lost, the hash is set to the special INVALID value. + * The parity for this block IS NOT valid, because it was computed with the old + * data referenced by the hash. * - * The parity for this disk is not updated, but it contains the old data referenced by the hash. - * - * If the state is read from an incomplete sync, we don't really know if the hash is referring to the - * data used to compute the parity, because the sync process was interrupted at an unknown point, - * and the parity may or may not be updated. - * - * For this reason we clear all such hashes when reading the state from an incomplete sync before - * starting a new sync, because sync is affected by such hashes, as sync updates the parity, only - * if the new data read for CHG blocks has a mismatching hash. - * Clearing is done setting the ::clear_past_hash flag before reading the state. - * No clearing is done in other commands, as check and fix are instead able to work with unsynced - * hashes, and scrub ignores CHG/DELETED blocks. + * If the state is read from an incomplete sync, we don't really know if the + * hash is referring to the data used to compute the parity, because the sync + * process was interrupted at an unknown point, and the parity may or may not + * be updated. */ #define BLOCK_STATE_CHG 2 /** * The block is new and hashed. - * This happens when a new block overwrites a just removed block, or an empty space. + * This happens when a new but hashed block overwrites a just removed block, + * or an empty space. + * + * The block hash is the hash of the NEW data. + * The parity for this block IS NOT valid. * * Note that when the file copy heuristic is enabled, the REP blocks may be set * using this heuristic, meaning that the hash may be wrong. - * - * For this reason, when the ::force_nocopy flag is enabled in sync, we convert all the REP blocks + * For this reason, when the ::force_nocopy flag is enabled, we convert all the REP blocks * to CHG, invalidating the stored hash. - * Clearing is done setting the ::clear_past_hash flag before reading the state. - * No clearing is done in other commands, as they don't stop the process like in sync - * when there is a false silent error. - * - * The block hash field IS set, and it represents the hash of the new data. - * The parity for this disk is not updated. */ #define BLOCK_STATE_REP 3 @@ -128,20 +119,17 @@ struct snapraid_filter { * This block is a deleted one. * This happens when a file is deleted. * - * The block hash field IS set, and it represents the hash of the previous data, - * but only if it's different by all 0. - * The parity for this disk is not updated, but it contains the old data referenced by the hash. + * The block hash is the hash of the OLD data. Possible values are: + * - INVALID -> The OLD block has an unknown hash + * - Otherwise -> The OLD block has this hash * - * If the state is read from an incomplete sync, we don't really know if the hash is referring to the - * data used to compute the parity, because the sync process was interrupted at an unknown point, - * and the parity may or may not be updated. + * The parity for this block IS NOT valid, because it was computed with the old + * data referenced by the hash. * - * As now the sync process is not affected by DELETED hash, so clearing won't be really needed, - * but considering that we have to do it for CHG blocks, we do it also for DELETED ones, - * clearing all the past hashes. - * Clearing is done setting the ::clear_past_hash flag before reading the state. - * No clearing is done in other commands, as check and fix are instead able to work with unsynced - * hashes, and scrub ignores CHG/DELETED blocks. + * If the state is read from an incomplete sync, we don't really know if the + * hash is referring to the data used to compute the parity, because the sync + * process was interrupted at an unknown point, and the parity may or may not + * be updated. */ #define BLOCK_STATE_DELETED 4 @@ -728,19 +716,6 @@ static inline int block_has_updated_hash(const struct snapraid_block* block) return state == BLOCK_STATE_BLK || state == BLOCK_STATE_REP; } -/** - * Check if the specified block has a past hash, - * i.e. the hash of the data that it's now overwritten or lost. - * - * Note that EMPTY / BLK / REP return 0. - */ -static inline int block_has_past_hash(const struct snapraid_block* block) -{ - unsigned state = block_state_get(block); - - return state == BLOCK_STATE_CHG || state == BLOCK_STATE_DELETED; -} - /** * Check if the specified block is part of a file. * diff --git a/cmdline/scan.c b/cmdline/scan.c index aea5f83d..32ea55e6 100644 --- a/cmdline/scan.c +++ b/cmdline/scan.c @@ -271,23 +271,25 @@ static void scan_file_allocate(struct snapraid_scan* scan, struct snapraid_file* /* otherwise it's a DELETED one */ assert(over_state == BLOCK_STATE_DELETED); + /* + * In this cases we don't know if the old state is still the one + * stored inside the parity, because after an aborted sync, the parity + * may be or may be not have been updated with the data that it's now + * deleted. + * + * We anyway keep the hash as the new sync will not assume it correct, + * but the check/fix may be using it. + * + * For example: + * - One file is deleted + * - Sync aborted after updating the parity to the new state, + * but without saving the content file representing this new state. + * - Another file is added again (exactly here) + * with the hash of DELETED block not representing the real parity state + */ + /* copy the past hash of the block */ memcpy(block->hash, over_block->hash, BLOCK_HASH_SIZE); - - /* if we have not already cleared the past hash */ - if (!state->clear_past_hash) { - /* in this case we don't know if the old state is still the one */ - /* stored inside the parity, because after an aborted sync, the parity */ - /* may be or may be not have been updated with the new data */ - /* Then we reset the hash to a bogus value */ - /* For example: */ - /* - One file is deleted */ - /* - Sync aborted after, updating the parity to the new state, */ - /* but without saving the content file representing this new state. */ - /* - Another file is added again (exactly here) */ - /* with the hash of DELETED block not representing the real parity state */ - hash_invalid_set(block->hash); - } } } @@ -314,7 +316,6 @@ static void scan_file_allocate(struct snapraid_scan* scan, struct snapraid_file* */ static void scan_file_deallocate(struct snapraid_scan* scan, struct snapraid_file* file) { - struct snapraid_state* state = scan->state; struct snapraid_disk* disk = scan->disk; block_off_t i; @@ -348,21 +349,23 @@ static void scan_file_deallocate(struct snapraid_scan* scan, struct snapraid_fil /* we keep the hash making it an "old" hash, because the parity is still containing data for it */ break; case BLOCK_STATE_CHG : - /* if we have not already cleared the past hash */ - if (!state->clear_past_hash) { - /* in these cases we don't know if the old state is still the one */ - /* stored inside the parity, because after an aborted sync, the parity */ - /* may be or may be not have been updated with the data that it's now */ - /* deleted. Then we reset the hash to a bogus value. */ - /* For example: */ - /* - One file is added */ - /* - Sync aborted after updating the parity to the new state, */ - /* but without saving the content file representing this new state. */ - /* - File is now deleted after the aborted sync */ - /* - Sync again, deleting the blocks (exactly here) */ - /* with the hash of CHG block not representing the real parity state */ - hash_invalid_set(block->hash); - } + /* + * In this cases we don't know if the old state is still the one + * stored inside the parity, because after an aborted sync, the parity + * may be or may be not have been updated with the data that it's now + * deleted. + * + * We anyway keep the hash as the new sync will not assume it correct, + * but the check/fix may be using it. + * + * For example: + * - One file is added + * - Sync aborted after updating the parity to the new state, + * but without saving the content file representing this new state. + * - File is now deleted after the aborted sync + * - Sync again, deleting the blocks (exactly here) + * with the hash of CHG block not representing the real parity state + */ break; case BLOCK_STATE_REP : /* we just don't know the old hash, and then we set it to invalid */ diff --git a/cmdline/snapraid.c b/cmdline/snapraid.c index 1e125867..10b221b5 100644 --- a/cmdline/snapraid.c +++ b/cmdline/snapraid.c @@ -1796,35 +1796,6 @@ int snapraid_main(int argc, char* argv[]) if (ret > 0) exit(EXIT_SYNC_NEEDED); } else if (operation == OPERATION_SYNC) { - - /* in the next state read ensures to clear all the past hashes in case */ - /* we are reading from an incomplete sync */ - /* The indeterminate hash are only for CHG/DELETED blocks for which we don't */ - /* know if the previous interrupted sync was able to update or not the parity. */ - /* The sync process instead needs to trust this information because it's used */ - /* to avoid to recompute the parity if all the input are equals as before. */ - - /* In these cases we don't know if the old state is still the one */ - /* stored inside the parity, because after an aborted sync, the parity */ - /* may be or may be not have been updated with the data that may be now */ - /* deleted. Then we reset the hash to a bogus value. */ - - /* An example for CHG blocks is: */ - /* - One file is added creating a CHG block with ZERO state */ - /* - Sync aborted after updating the parity to the new state, */ - /* but without saving the content file representing this new BLK state. */ - /* - File is now deleted after the aborted sync */ - /* - Sync again, deleting the blocks over the CHG ones */ - /* with the hash of CHG blocks not representing the real parity state */ - - /* An example for DELETED blocks is: */ - /* - One file is deleted creating DELETED blocks */ - /* - Sync aborted after, updating the parity to the new state, */ - /* but without saving the content file representing this new EMPTY state. */ - /* - Another file is added again over the DELETE ones */ - /* with the hash of DELETED blocks not representing the real parity state */ - state.clear_past_hash = 1; - state_read(&state); /* mark the files that have to be reallocated */ diff --git a/cmdline/state.c b/cmdline/state.c index f4662d2e..1a8175bd 100644 --- a/cmdline/state.c +++ b/cmdline/state.c @@ -165,7 +165,6 @@ void state_init(struct snapraid_state* state) state->pool_device = 0; state->lockfile[0] = 0; state->level = 1; /* default is the lowest protection */ - state->clear_past_hash = 0; state->no_conf = 0; for (i = 0; i < SMART_IGNORE_MAX; ++i) state->smartignore[i] = 0; @@ -2350,26 +2349,13 @@ static void state_read_content(struct snapraid_state* state, const char* path, S hash_zero_set(block->hash); } - /* if the block contains a hash of past data */ - /* and we are clearing such indeterminate hashes */ - if (state->clear_past_hash - && block_has_past_hash(block) - ) { - /* set the hash value to INVALID */ - hash_invalid_set(block->hash); - } - /* if we are disabling the copy optimization */ /* we want also to clear any already previously stored information */ /* in other sync commands */ - /* note that this is required only in sync, and we detect */ - /* this using the clear_past_hash flag */ - if (state->clear_past_hash - && state->opt.force_nocopy - && block_state_get(block) == BLOCK_STATE_REP - ) { + if (state->opt.force_nocopy && block_state_get(block) == BLOCK_STATE_REP) { /* set the hash value to INVALID */ hash_invalid_set(block->hash); + /* convert from REP to CHG block */ block_state_set(block, BLOCK_STATE_CHG); } @@ -2568,12 +2554,6 @@ static void state_read_content(struct snapraid_state* state, const char* path, S /* LCOV_EXCL_STOP */ } - /* if we are clearing indeterminate hashes */ - if (state->clear_past_hash) { - /* set the hash value to INVALID */ - hash_invalid_set(block->hash); - } - /* insert the block in the block array */ fs_allocate(disk, v_pos, deleted, v_idx); @@ -4380,6 +4360,9 @@ void state_read(struct snapraid_state* state) /* LCOV_EXCL_STOP */ } + if (state->unsynced_blocks) + msg_progress("The latest sync was interrupted!\n"); + /* update the mapping */ state_map(state); diff --git a/cmdline/state.h b/cmdline/state.h index 84e8ca93..33e3728d 100644 --- a/cmdline/state.h +++ b/cmdline/state.h @@ -182,8 +182,6 @@ struct snapraid_state { */ uint64_t tick_last; - int clear_past_hash; /**< Clear all the hash from CHG and DELETED blocks when reading the state from an incomplete sync. */ - time_t progress_whole_start; /**< Initial start of the whole process. */ time_t progress_interruption; /**< Time of the start of the progress interruption. */ time_t progress_wasted; /**< Time wasted in interruptions. */ diff --git a/cmdline/sync.c b/cmdline/sync.c index 46722919..1916bac5 100644 --- a/cmdline/sync.c +++ b/cmdline/sync.c @@ -652,10 +652,6 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari unsigned waiting_mac; bit_vect_t* block_enabled; - /* the sync process assumes that all the hashes are correct */ - /* including the ones from CHG and DELETED blocks */ - assert(state->clear_past_hash != 0); - /* get the present time */ now = time(0); @@ -770,13 +766,16 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari /* if we have to use the old hash */ rehash = info_get_rehash(info); - /* if the parity requires to be updated */ - /* It could happens that all the blocks are EMPTY/BLK and CHG but with the hash */ - /* still matching because the specific CHG block was not modified. */ - /* In such case, we can avoid to update parity, because it would be the same as before */ - /* Note that CHG/DELETED blocks already present in the content file loaded */ - /* have the hash cleared (::clear_past_hash flag), and then they won't never match the hash. */ - /* We are treating only CHG blocks created at runtime. */ + /* + * If the parity requires to be updated + * + * It could happens that all the blocks are EMPTY/BLK and CHG but with the hash + * still matching because the specific CHG block was not modified. + * In such case, we can avoid to update parity, because it would be the same as before + * + * Note that if there is any CHG/DELETED blocks already present in the content + * file loaded, meaning that there are unsynced_blocks, this optimization is disabled + */ parity_needs_to_be_updated = state->opt.force_full || state->opt.force_parity_update; /* if the parity is going to be updated */ @@ -960,15 +959,46 @@ static int state_sync_process(struct snapraid_state* state, struct snapraid_pari /* have "parity_needs_to_be_updated" already at 1 */ assert(block_state_get(block) == BLOCK_STATE_CHG); - /* if the hash represents the data unequivocally */ - if (hash_is_unique(block->hash)) { + /* + * When a sync is interrupted, the state of the parity is unknown becase + * we don't know exactly where the process stopped. + * + * This means that the hash information of the OLD blocks stored in the + * content file for CHG/DELETED blocks may be correct or not. + * + * The sync process uses the hash of CHG blocks to decide if the parity has to be + * recomputed, avoiding the recomputation if the input data is the same as before. + * But in case of an interrupted sync we cannot trust this data, so we + * disable this optimization if there are unsynced blocks. + * + * Note that CHG blocks may be from reading the content file, or from + * scanning the disk for really changed file, like with a different timestamp. + * + * An example for CHG blocks is: + * - One file is added creating a CHG block with ZERO state + * - Sync aborted after updating the parity to the new state, + * but without saving the content file representing this new BLK state. + * - File is now deleted after the aborted sync + * - Sync again, deleting the blocks over the CHG ones + * with the hash of CHG blocks not representing the real parity state + * + * An example for DELETED blocks is: + * - One file is deleted creating DELETED blocks + * - Sync aborted after, updating the parity to the new state, + * but without saving the content file representing this new EMPTY state. + * - Another file is added again over the DELETE ones + * with the hash of DELETED blocks not representing the real parity state + */ + + /* if the previous sync was completed and the hash represents the data unequivocally */ + if (state->unsynced_blocks == 0 && hash_is_unique(block->hash)) { /* check if the hash is changed */ if (memcmp(hash, block->hash, BLOCK_HASH_SIZE) != 0) { /* the block is different, and we must update parity */ parity_needs_to_be_updated = 1; } } else { - /* if the hash is already invalid, we update parity */ + /* if we don't know the hash, always update parity */ parity_needs_to_be_updated = 1; } }