From 45d18c1e9d5d162eaf56ce122d53299d5299e8a9 Mon Sep 17 00:00:00 2001 From: Di Wang Date: Thu, 1 Apr 2021 21:57:58 +0000 Subject: [PATCH] DAOS-7136 object: merged recxs during punch migration Merged punched recxs under same akey to avoid the failure of punched recxs migration. Signed-off-by: Di Wang --- src/object/srv_obj_migrate.c | 293 +++++++++++++------------- src/tests/suite/daos_rebuild_simple.c | 41 ++++ 2 files changed, 193 insertions(+), 141 deletions(-) diff --git a/src/object/srv_obj_migrate.c b/src/object/srv_obj_migrate.c index e7c07a9b611..ae5de6fc7b4 100644 --- a/src/object/srv_obj_migrate.c +++ b/src/object/srv_obj_migrate.c @@ -1490,16 +1490,152 @@ migrate_one_ult(void *arg) } } +/* If src_iod is NULL, it will try to merge the recxs inside dst_iod */ static int -migrate_one_iod_merge_recx(daos_unit_oid_t oid, daos_iod_t *dst_iod, - daos_iod_t *src_iod); +migrate_merge_iod_recx(daos_iod_t *dst_iod, daos_iod_t *src_iod) +{ + struct obj_auxi_list_recx *recx; + struct obj_auxi_list_recx *tmp; + daos_recx_t *recxs; + d_list_t merge_list; + int nr_recxs = 0; + int i; + int rc = 0; + + D_INIT_LIST_HEAD(&merge_list); + if (src_iod != NULL) { + recxs = src_iod->iod_recxs; + for (i = 0; i < src_iod->iod_nr; i++) { + D_DEBUG(DB_REBUILD, "src merge "DF_U64"/"DF_U64"\n", + recxs[i].rx_idx, recxs[i].rx_nr); + rc = merge_recx(&merge_list, recxs[i].rx_idx, + recxs[i].rx_nr); + if (rc) + D_GOTO(out, rc); + } + } + + D_ASSERT(dst_iod != NULL); + recxs = dst_iod->iod_recxs; + for (i = 0; i < dst_iod->iod_nr; i++) { + D_DEBUG(DB_REBUILD, "dst merge "DF_U64"/"DF_U64"\n", + recxs[i].rx_idx, recxs[i].rx_nr); + rc = merge_recx(&merge_list, recxs[i].rx_idx, recxs[i].rx_nr); + if (rc) + D_GOTO(out, rc); + } + + d_list_for_each_entry(recx, &merge_list, recx_list) + nr_recxs++; + + if (nr_recxs > dst_iod->iod_nr) { + D_ALLOC_ARRAY(recxs, nr_recxs); + if (recxs == NULL) + D_GOTO(out, rc = -DER_NOMEM); + } else { + recxs = dst_iod->iod_recxs; + } + + i = 0; + d_list_for_each_entry_safe(recx, tmp, &merge_list, recx_list) { + recxs[i++] = recx->recx; + + D_DEBUG(DB_REBUILD, "merge recx "DF_U64"/"DF_U64"\n", + recx->recx.rx_idx, recx->recx.rx_nr); + d_list_del(&recx->recx_list); + D_FREE(recx); + } + + if (dst_iod->iod_recxs != recxs) + D_FREE(dst_iod->iod_recxs); + + dst_iod->iod_recxs = recxs; + dst_iod->iod_nr = i; +out: + d_list_for_each_entry_safe(recx, tmp, &merge_list, recx_list) { + d_list_del(&recx->recx_list); + D_FREE(recx); + } + return rc; +} + +static int +migrate_iod_sgl_add(daos_iod_t *iods, uint32_t *iods_num, daos_iod_t *new_iod, + d_sg_list_t *sgls, d_sg_list_t *new_sgl) +{ + daos_iod_t *dst_iod; + int rc = 0; + int i; + + for (i = 0; i < *iods_num; i++) { + if (daos_iov_cmp(&iods[i].iod_name, &new_iod->iod_name)) + break; + } + + /* None duplicate iods, let's create new one */ + if (i == *iods_num) { + rc = daos_iod_copy(&iods[i], new_iod); + if (rc) + return rc; + + if (new_sgl) { + rc = daos_sgl_alloc_copy_data(&sgls[i], new_sgl); + if (rc) { + daos_iov_free(&iods[i].iod_name); + return rc; + } + } + + if (new_iod->iod_type == DAOS_IOD_SINGLE) { + iods[i].iod_recxs = NULL; + } else { + /** + * recx in iod has been reused, i.e. it will be + * freed with mrone, so let's set iod_recxs in + * iod to be NULL to avoid it is being freed + * with iod afterwards. + */ + new_iod->iod_recxs = NULL; + } + D_DEBUG(DB_REBUILD, "add new akey "DF_KEY" at %d\n", + DP_KEY(&new_iod->iod_name), i); + (*iods_num)++; + return rc; + } + + /* Try to merge the iods to the existent IOD */ + dst_iod = &iods[i]; + if (dst_iod->iod_size != new_iod->iod_size || + dst_iod->iod_type != new_iod->iod_type) { + D_ERROR(DF_KEY" dst_iod size "DF_U64" != "DF_U64 + " dst_iod type %d != %d\n", + DP_KEY(&new_iod->iod_name), dst_iod->iod_size, + new_iod->iod_size, dst_iod->iod_type, + new_iod->iod_type); + D_GOTO(out, rc); + } + + rc = migrate_merge_iod_recx(dst_iod, new_iod); + if (rc) + D_GOTO(out, rc); + + if (new_sgl) { + rc = daos_sgl_merge(&sgls[i], new_sgl); + if (rc) + D_GOTO(out, rc); + } + + D_DEBUG(DB_REBUILD, "Merge akey "DF_KEY" to %d\n", + DP_KEY(&new_iod->iod_name), i); +out: + return rc; +} static int rw_iod_pack(struct migrate_one *mrone, daos_iod_t *iod, d_sg_list_t *sgl) { uint64_t total_size = 0; int rec_cnt = 0; - bool free_recxs = true; int i; int rc; @@ -1512,58 +1648,13 @@ rw_iod_pack(struct migrate_one *mrone, daos_iod_t *iod, d_sg_list_t *sgl) return -DER_NOMEM; } - for (i = 0; i < mrone->mo_iod_num; i++) { - if (daos_iov_cmp(&mrone->mo_iods[i].iod_name, - &iod->iod_name)) - break; - } - - if (i < mrone->mo_iod_num) { - daos_iod_t *dst_iod = &mrone->mo_iods[i]; - - /*merge the iods to the existent IOD */ - D_DEBUG(DB_REBUILD, "Merge akey "DF_KEY" to %d\n", - DP_KEY(&iod->iod_name), i); - if (dst_iod->iod_size != iod->iod_size || - dst_iod->iod_type != iod->iod_type) { - D_ERROR(DF_KEY" dst_iod size "DF_U64" != "DF_U64 - " dst_iod type %d != %d\n", - DP_KEY(&iod->iod_name), dst_iod->iod_size, - iod->iod_size, dst_iod->iod_type, - iod->iod_type); - D_GOTO(out, rc = -DER_INVAL); - } - - rc = migrate_one_iod_merge_recx(mrone->mo_oid, dst_iod, iod); - if (rc) - D_GOTO(out, rc); - - if (sgl) { - rc = daos_sgl_merge(&mrone->mo_sgls[i], sgl); - if (rc) - D_GOTO(out, rc); - } - } else { - rc = daos_iod_copy(&mrone->mo_iods[i], iod); - if (rc) - D_GOTO(out, rc); - - free_recxs = false; - mrone->mo_iod_num++; - if (sgl) { - rc = daos_sgl_alloc_copy_data(&mrone->mo_sgls[i], sgl); - if (rc) - D_GOTO(out, rc); - } - } - if (iod->iod_type == DAOS_IOD_SINGLE) { - mrone->mo_iods[i].iod_recxs = NULL; rec_cnt = 1; total_size = iod->iod_size; + D_DEBUG(DB_REBUILD, "single recx "DF_U64"\n", total_size); } else { for (i = 0; i < iod->iod_nr; i++) { - D_DEBUG(DB_REBUILD, "recx "DF_U64"/"DF_U64"\n", + D_DEBUG(DB_REBUILD, "new recx "DF_U64"/"DF_U64"\n", iod->iod_recxs[i].rx_idx, iod->iod_recxs[i].rx_nr); rec_cnt += iod->iod_recxs[i].rx_nr; @@ -1571,13 +1662,10 @@ rw_iod_pack(struct migrate_one *mrone, daos_iod_t *iod, d_sg_list_t *sgl) } } - /** - * If free_recxs is true, then the caller needs to free iod_recxs later, - * so do not set iod_recxs to NULL here, otherwise this mrone will - * reuse recxs. - */ - if (!free_recxs) - iod->iod_recxs = NULL; + rc = migrate_iod_sgl_add(mrone->mo_iods, &mrone->mo_iod_num, iod, + mrone->mo_sgls, sgl); + if (rc != 0) + D_GOTO(out, rc); D_DEBUG(DB_REBUILD, "idx %d akey "DF_KEY" nr %d size "DF_U64" type %d rec %d total " @@ -1605,9 +1693,10 @@ punch_iod_pack(struct migrate_one *mrone, daos_iod_t *iod, daos_epoch_t eph) return -DER_NOMEM; } - rc = daos_iod_copy(&mrone->mo_punch_iods[idx], iod); - if (rc) - return rc; + rc = migrate_iod_sgl_add(mrone->mo_punch_iods, &mrone->mo_punch_iod_num, + iod, NULL, NULL); + if (rc != 0) + D_GOTO(out, rc); D_DEBUG(DB_TRACE, "idx %d akey "DF_KEY" nr %d size "DF_U64" type %d\n", @@ -1616,83 +1705,7 @@ punch_iod_pack(struct migrate_one *mrone, daos_iod_t *iod, daos_epoch_t eph) if (mrone->mo_rec_punch_eph < eph) mrone->mo_rec_punch_eph = eph; - - mrone->mo_punch_iod_num++; - iod->iod_recxs = NULL; - return 0; -} - -static int -migrate_one_iod_merge_recx(daos_unit_oid_t oid, daos_iod_t *dst_iod, - daos_iod_t *src_iod) -{ - struct obj_auxi_list_recx *recx; - struct obj_auxi_list_recx *tmp; - struct daos_oclass_attr *oca; - daos_recx_t *recxs; - d_list_t merge_list; - int nr_recxs = 0; - int i; - int rc = 0; - - oca = daos_oclass_attr_find(oid.id_pub); - if (oca == NULL) - return -DER_NONEXIST; - - D_INIT_LIST_HEAD(&merge_list); - if (src_iod != NULL) { - recxs = src_iod->iod_recxs; - for (i = 0; i < src_iod->iod_nr; i++) { - D_DEBUG(DB_REBUILD, "src merge "DF_U64"/"DF_U64"\n", - recxs[i].rx_idx, recxs[i].rx_nr); - rc = merge_recx(&merge_list, recxs[i].rx_idx, - recxs[i].rx_nr); - if (rc) - D_GOTO(out, rc); - } - } - - D_ASSERT(dst_iod != NULL); - recxs = dst_iod->iod_recxs; - for (i = 0; i < dst_iod->iod_nr; i++) { - D_DEBUG(DB_REBUILD, "dst merge "DF_U64"/"DF_U64"\n", - recxs[i].rx_idx, recxs[i].rx_nr); - rc = merge_recx(&merge_list, recxs[i].rx_idx, recxs[i].rx_nr); - if (rc) - D_GOTO(out, rc); - } - - d_list_for_each_entry(recx, &merge_list, recx_list) - nr_recxs++; - - if (nr_recxs > dst_iod->iod_nr) { - D_ALLOC_ARRAY(recxs, nr_recxs); - if (recxs == NULL) - D_GOTO(out, rc = -DER_NOMEM); - } else { - recxs = dst_iod->iod_recxs; - } - - i = 0; - d_list_for_each_entry_safe(recx, tmp, &merge_list, recx_list) { - recxs[i++] = recx->recx; - - D_DEBUG(DB_REBUILD, "merge recx "DF_U64"/"DF_U64"\n", - recx->recx.rx_idx, recx->recx.rx_nr); - d_list_del(&recx->recx_list); - D_FREE(recx); - } - - if (dst_iod->iod_recxs != recxs) - D_FREE(dst_iod->iod_recxs); - - dst_iod->iod_recxs = recxs; - dst_iod->iod_nr = i; out: - d_list_for_each_entry_safe(recx, tmp, &merge_list, recx_list) { - d_list_del(&recx->recx_list); - D_FREE(recx); - } return rc; } @@ -1721,9 +1734,8 @@ migrate_one_merge(struct migrate_one *mo, struct dss_enum_unpack_io *io) &io->ui_iods[i].iod_name)) continue; if (mo->mo_iods[j].iod_type == DAOS_IOD_ARRAY) { - rc = migrate_one_iod_merge_recx(io->ui_oid, - &mo->mo_iods[j], - &io->ui_iods[i]); + rc = migrate_merge_iod_recx(&mo->mo_iods[j], + &io->ui_iods[i]); if (rc) D_GOTO(out, rc); @@ -1909,8 +1921,7 @@ migrate_enum_unpack_cb(struct dss_enum_unpack_io *io, void *data) * some duplicate recxs due to replication/parity * space. let's remove them. */ - rc = migrate_one_iod_merge_recx(io->ui_oid, - iod, NULL); + rc = migrate_merge_iod_recx(iod, NULL); if (rc) return rc; diff --git a/src/tests/suite/daos_rebuild_simple.c b/src/tests/suite/daos_rebuild_simple.c index a3e717d7be4..616ab32e457 100644 --- a/src/tests/suite/daos_rebuild_simple.c +++ b/src/tests/suite/daos_rebuild_simple.c @@ -964,6 +964,45 @@ rebuild_full_shards(void **state) reintegrate_single_pool_target(arg, 3, -1); } +static void +rebuild_punch_recs(void **state) +{ + test_arg_t *arg = *state; + daos_obj_id_t oid; + struct ioreq req; + daos_recx_t recx; + char buffer[1001] = { 0 }; + int i; + int rc; + + if (!test_runable(arg, 4)) + return; + + oid = daos_test_oid_gen(arg->coh, arg->obj_class, 0, 0, arg->myrank); + oid = dts_oid_set_rank(oid, ranks_to_kill[0]); + ioreq_init(&req, arg->coh, oid, DAOS_IOD_ARRAY, arg); + + memset(buffer, 'a', 1000); + recx.rx_idx = 0; + recx.rx_nr = 1000; + insert_recxs("d_key", "a_key", 1, DAOS_TX_NONE, &recx, 1, buffer, + 1000, &req); + + for (i = 0; i < 5; i++) { + /* punch string */ + recx.rx_idx = i * 100; + recx.rx_nr = 50; + punch_recxs("d_key", "a_key", &recx, 1, DAOS_TX_NONE, &req); + } + ioreq_fini(&req); + + rebuild_single_pool_target(arg, ranks_to_kill[0], -1, false); + + rc = daos_obj_verify(arg->coh, oid, DAOS_EPOCH_MAX); + if (rc != 0) + assert_rc_equal(rc, -DER_NOSYS); +} + /** create a new pool/container for each test */ static const struct CMUnitTest rebuild_tests[] = { {"REBUILD1: rebuild small rec multiple dkeys", @@ -998,6 +1037,8 @@ static const struct CMUnitTest rebuild_tests[] = { rebuild_large_snap, rebuild_small_sub_setup, test_teardown}, {"REBUILD16: rebuild with full stripe", rebuild_full_shards, rebuild_small_pool_n4_setup, test_teardown}, + {"REBUILD17: rebuild with punch recxs", + rebuild_punch_recs, rebuild_small_sub_setup, test_teardown}, }; int