Fix nasty rebuild bug where partiy would be reconstructed incorrectly
when both data and parity was missing. The fix is to just call decode when reconstructing parity, since it will have to do extra work anyway when data is missing. We did a little extra work in ISA-L to do better, but can save that for later, since 99% of the time decode will perform just fine.
This commit is contained in:
parent
c10ec8a255
commit
0d1e4136ae
|
@ -137,7 +137,7 @@ static int jerasure_rs_cauchy_reconstruct(void *desc, char **data, char **parity
|
|||
int *missing_idxs, int destination_idx, int blocksize)
|
||||
{
|
||||
int k, m, w; /* erasure code paramters */
|
||||
int ret = 1; /* return code */
|
||||
int ret = 0; /* return code */
|
||||
int *decoding_row = NULL; /* decoding matrix row for decode */
|
||||
int *erased = NULL; /* k+m length list of erased frag ids */
|
||||
int *dm_ids = NULL; /* k length list of fragment ids */
|
||||
|
@ -149,29 +149,46 @@ static int jerasure_rs_cauchy_reconstruct(void *desc, char **data, char **parity
|
|||
m = jerasure_desc->m;
|
||||
w = jerasure_desc->w;
|
||||
|
||||
dm_ids = (int *) alloc_zeroed_buffer(sizeof(int) * k);
|
||||
decoding_matrix = (int *) alloc_zeroed_buffer(sizeof(int *) * k * k * w * w);
|
||||
erased = jerasure_desc->jerasure_erasures_to_erased(k, m, missing_idxs);
|
||||
if (NULL == decoding_matrix || NULL == dm_ids || NULL == erased) {
|
||||
goto out;
|
||||
}
|
||||
if (destination_idx < k) {
|
||||
dm_ids = (int *) alloc_zeroed_buffer(sizeof(int) * k);
|
||||
decoding_matrix = (int *) alloc_zeroed_buffer(sizeof(int *) * k * k * w * w);
|
||||
erased = jerasure_desc->jerasure_erasures_to_erased(k, m, missing_idxs);
|
||||
if (NULL == decoding_matrix || NULL == dm_ids || NULL == erased) {
|
||||
goto out;
|
||||
}
|
||||
|
||||
ret = jerasure_desc->jerasure_make_decoding_bitmatrix(k, m, w,
|
||||
ret = jerasure_desc->jerasure_make_decoding_bitmatrix(k, m, w,
|
||||
jerasure_desc->bitmatrix,
|
||||
erased, decoding_matrix, dm_ids);
|
||||
if (destination_idx < k) {
|
||||
decoding_row = decoding_matrix + (destination_idx * k * w * w);
|
||||
} else {
|
||||
decoding_row = jerasure_desc->bitmatrix + ((destination_idx - k) * k * w * w);
|
||||
}
|
||||
if (ret == 0) {
|
||||
decoding_row = decoding_matrix + (destination_idx * k * w * w);
|
||||
|
||||
if (ret == 0) {
|
||||
jerasure_desc->jerasure_bitmatrix_dotprod(jerasure_desc->k, jerasure_desc->w,
|
||||
jerasure_desc->jerasure_bitmatrix_dotprod(jerasure_desc->k, jerasure_desc->w,
|
||||
decoding_row, dm_ids, destination_idx,
|
||||
data, parity, blocksize,
|
||||
PYECC_CAUCHY_PACKETSIZE);
|
||||
data, parity, blocksize, PYECC_CAUCHY_PACKETSIZE);
|
||||
} else {
|
||||
/*
|
||||
* ToDo (KMG) I know this is not needed, but keeping to prevent future
|
||||
* memory leaks, as this function will be better optimized for decoding
|
||||
* missing parity
|
||||
*/
|
||||
goto out;
|
||||
}
|
||||
} else {
|
||||
goto out;
|
||||
/*
|
||||
* If it is parity we are reconstructing, then just call decode.
|
||||
* ToDo (KMG): We can do better than this, but this should perform just
|
||||
* fine for most cases. We can adjust the decoding matrix like we
|
||||
* did with ISA-L.
|
||||
*/
|
||||
jerasure_desc->jerasure_bitmatrix_decode(k, m, w,
|
||||
jerasure_desc->bitmatrix,
|
||||
0,
|
||||
missing_idxs,
|
||||
data,
|
||||
parity,
|
||||
blocksize,
|
||||
PYECC_CAUCHY_PACKETSIZE);
|
||||
}
|
||||
|
||||
out:
|
||||
|
|
|
@ -108,7 +108,7 @@ static int jerasure_rs_vand_decode(void *desc, char **data, char **parity,
|
|||
static int jerasure_rs_vand_reconstruct(void *desc, char **data, char **parity,
|
||||
int *missing_idxs, int destination_idx, int blocksize)
|
||||
{
|
||||
int ret = 1; /* return code */
|
||||
int ret = 0; /* return code */
|
||||
int *decoding_row; /* decoding matrix row for decode */
|
||||
int *erased = NULL; /* k+m length list of erased frag ids */
|
||||
int *dm_ids = NULL; /* k length list of frag ids */
|
||||
|
@ -117,35 +117,45 @@ static int jerasure_rs_vand_reconstruct(void *desc, char **data, char **parity,
|
|||
struct jerasure_rs_vand_descriptor *jerasure_desc =
|
||||
(struct jerasure_rs_vand_descriptor*) desc;
|
||||
|
||||
dm_ids = (int *) alloc_zeroed_buffer(sizeof(int) * jerasure_desc->k);
|
||||
decoding_matrix = (int *)
|
||||
alloc_zeroed_buffer(sizeof(int*) * jerasure_desc->k * jerasure_desc->k);
|
||||
if (NULL == decoding_matrix || NULL == dm_ids) {
|
||||
goto out;
|
||||
}
|
||||
|
||||
erased = jerasure_desc->jerasure_erasures_to_erased(jerasure_desc->k,
|
||||
jerasure_desc->m, missing_idxs);
|
||||
if (NULL == erased) {
|
||||
goto out;
|
||||
}
|
||||
|
||||
ret = jerasure_desc->jerasure_make_decoding_matrix(jerasure_desc->k,
|
||||
jerasure_desc->m, jerasure_desc->w, jerasure_desc->matrix,
|
||||
erased, decoding_matrix, dm_ids);
|
||||
if (destination_idx < jerasure_desc->k) {
|
||||
dm_ids = (int *) alloc_zeroed_buffer(sizeof(int) * jerasure_desc->k);
|
||||
decoding_matrix = (int *)
|
||||
alloc_zeroed_buffer(sizeof(int*) * jerasure_desc->k * jerasure_desc->k);
|
||||
erased = jerasure_desc->jerasure_erasures_to_erased(jerasure_desc->k,
|
||||
jerasure_desc->m, missing_idxs);
|
||||
if (NULL == decoding_matrix || NULL == dm_ids || NULL == erased) {
|
||||
goto out;
|
||||
}
|
||||
|
||||
ret = jerasure_desc->jerasure_make_decoding_matrix(jerasure_desc->k,
|
||||
jerasure_desc->m, jerasure_desc->w, jerasure_desc->matrix,
|
||||
erased, decoding_matrix, dm_ids);
|
||||
|
||||
decoding_row = decoding_matrix + (destination_idx * jerasure_desc->k);
|
||||
} else {
|
||||
decoding_row = jerasure_desc->matrix +
|
||||
((destination_idx - jerasure_desc->k) * jerasure_desc->k);
|
||||
}
|
||||
|
||||
if (ret == 0) {
|
||||
jerasure_desc->jerasure_matrix_dotprod(jerasure_desc->k,
|
||||
jerasure_desc->w, decoding_row, dm_ids, destination_idx,
|
||||
data, parity, blocksize);
|
||||
if (ret == 0) {
|
||||
jerasure_desc->jerasure_matrix_dotprod(jerasure_desc->k,
|
||||
jerasure_desc->w, decoding_row, dm_ids, destination_idx,
|
||||
data, parity, blocksize);
|
||||
} else {
|
||||
/*
|
||||
* ToDo (KMG) I know this is not needed, but keeping to prevent future
|
||||
* memory leaks, as this function will be better optimized for decoding
|
||||
* missing parity
|
||||
*/
|
||||
goto out;
|
||||
}
|
||||
} else {
|
||||
goto out;
|
||||
/*
|
||||
* If it is parity we are reconstructing, then just call decode.
|
||||
* ToDo (KMG): We can do better than this, but this should perform just
|
||||
* fine for most cases. We can adjust the decoding matrix like we
|
||||
* did with ISA-L.
|
||||
*/
|
||||
jerasure_desc->jerasure_matrix_decode(jerasure_desc->k,
|
||||
jerasure_desc->m, jerasure_desc->w,
|
||||
jerasure_desc->matrix, 1, missing_idxs, data, parity, blocksize);
|
||||
goto parity_reconstr_out;
|
||||
}
|
||||
|
||||
out:
|
||||
|
@ -153,6 +163,7 @@ out:
|
|||
free(decoding_matrix);
|
||||
free(dm_ids);
|
||||
|
||||
parity_reconstr_out:
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
|
@ -768,6 +768,17 @@ static void encode_decode_test_impl(const ec_backend_id_t be_id,
|
|||
free(avail_frags);
|
||||
}
|
||||
|
||||
/**
|
||||
* Note: this test will attempt to reconstruct a single fragment when
|
||||
* one or more other fragments are missing (specified by skip).
|
||||
*
|
||||
* For example, if skip is [0, 0, 0, 1, 0, 0] and we are reconstructing
|
||||
* fragment 5, then it will test the reconstruction of fragment 5 when 3
|
||||
* and 5 are assumed unavailable.
|
||||
*
|
||||
* We only mark at most 2 as unavailable, as we cannot guarantee every situation
|
||||
* will be able to habndle 3 failures.
|
||||
*/
|
||||
static void reconstruct_test_impl(const ec_backend_id_t be_id,
|
||||
struct ec_args *args,
|
||||
int *skip)
|
||||
|
@ -796,15 +807,24 @@ static void reconstruct_test_impl(const ec_backend_id_t be_id,
|
|||
rc = liberasurecode_encode(desc, orig_data, orig_data_size,
|
||||
&encoded_data, &encoded_parity, &encoded_fragment_len);
|
||||
assert(rc == 0);
|
||||
num_avail_frags = create_frags_array(&avail_frags, encoded_data,
|
||||
encoded_parity, args, skip);
|
||||
out = malloc(encoded_fragment_len);
|
||||
assert(out != NULL);
|
||||
for (i = 0; i < num_fragments; i++) {
|
||||
if (skip[i] == 0) {
|
||||
continue;
|
||||
}
|
||||
char *cmp = NULL;
|
||||
// If the current fragment was not chosen as fragments to skip,
|
||||
// remove it and the chosen fragments to skip from the available list
|
||||
// and reset its state
|
||||
if (skip[i] == 0) {
|
||||
skip[i] = 1;
|
||||
num_avail_frags = create_frags_array(&avail_frags, encoded_data,
|
||||
encoded_parity, args, skip);
|
||||
skip[i] = 0;
|
||||
// Do not reset the skip state if the fragment was chosen as a fragment
|
||||
// to skip for this invocation of the test
|
||||
} else {
|
||||
num_avail_frags = create_frags_array(&avail_frags, encoded_data,
|
||||
encoded_parity, args, skip);
|
||||
}
|
||||
if (i < args->k) {
|
||||
cmp = encoded_data[i];
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue