Skip to content

Commit

Permalink
DAOS-16876 vos: set cont parameter when deregister modification from …
Browse files Browse the repository at this point in the history
…DTX - b26

As long as the container is not destroyed, then anytime want to deregister
a modification from related active DTX entry (that is usually triggered for
vos discard or aggregation), the caller needs to offer container handle to
vos_dtx_deregister_record() for locating the DTX entry in active DTX table.
Otherwise, if the caller offers empty container handle, then it will cause
dangling reference in related DTX entry as to data corruption in subsequent
DTX commit or abort.

On the other hand, if the container will be destroyed, then all related DTX
entries for such container will be useless any more. We need to destroy DTX
table firstly to avoid generating dangling DTX references during destroying
the container.

Allow-unstable-test: true

Signed-off-by: Fan Yong <[email protected]>
  • Loading branch information
Nasf-Fan committed Dec 27, 2024
1 parent 776f123 commit 9ba1370
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 28 deletions.
12 changes: 3 additions & 9 deletions src/vos/tests/vts_aggregate.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,9 @@

static bool slow_test;

static void
static inline void
cleanup(void)
{
daos_fail_loc_set(DAOS_VOS_GC_CONT_NULL | DAOS_FAIL_ALWAYS);
gc_wait();
}

Expand Down Expand Up @@ -1315,24 +1314,19 @@ agg_punches_test(void **state, int record_type, bool discard)
}
}
}
/** cleanup() sets the flag to assert if there are items in container garbage collection
* heap which will always be the case for these punch tests. So let's run garbage
* collection before cleanup in this case.
*/
gc_wait();

cleanup();
}
static void
discard_14(void **state)
{
agg_punches_test(state, DAOS_IOD_SINGLE, true);
cleanup();
}

static void
discard_15(void **state)
{
agg_punches_test(state, DAOS_IOD_ARRAY, true);
cleanup();
}

static void
Expand Down
18 changes: 8 additions & 10 deletions src/vos/vos_dtx.c
Original file line number Diff line number Diff line change
Expand Up @@ -1557,7 +1557,6 @@ vos_dtx_deregister_record(struct umem_instance *umm, daos_handle_t coh,
uint32_t entry, daos_epoch_t epoch, umem_off_t record)
{
struct dtx_handle *dth = vos_dth_get(false);
struct vos_container *cont;
struct vos_dtx_act_ent *dae;
struct vos_dtx_act_ent_df *dae_df;
umem_off_t *rec_df;
Expand All @@ -1566,20 +1565,19 @@ vos_dtx_deregister_record(struct umem_instance *umm, daos_handle_t coh,
int rc;
int i;

/*
* If @coh is empty handle, then we are destroying the container. Under such case,
* both the in-DRAM and on-dish DTX entry have already been released or destroyed.
*/
if (daos_handle_is_inval(coh))
return 0;

if (!vos_dtx_is_normal_entry(entry))
return 0;

D_ASSERT(entry >= DTX_LID_RESERVED);

cont = vos_hdl2cont(coh);
/* If "cont" is NULL, then we are destroying the container.
* Under such case, the DTX entry in DRAM has been removed,
* The on-disk entry will be destroyed soon.
*/
if (cont == NULL)
return 0;

found = lrua_lookupx(cont->vc_dtx_array, entry - DTX_LID_RESERVED,
found = lrua_lookupx(vos_hdl2cont(coh)->vc_dtx_array, entry - DTX_LID_RESERVED,
epoch, &dae);
if (!found) {
D_WARN("Could not find active DTX record for lid=%d, epoch="
Expand Down
17 changes: 10 additions & 7 deletions src/vos/vos_gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,15 @@ gc_drain_cont(struct vos_gc *gc, struct vos_pool *pool, daos_handle_t coh,
int i;
int rc;

/*
* When we prepaer to drain the container, we do not need DTX entry any long.
* Then destroy DTX table firstly to avoid dangling DXT records during drain
* the container (that may yield).
*/
rc = vos_dtx_table_destroy(&pool->vp_umm, cont);
if (rc != 0)
return rc;

/** Move any leftover bags to the pool gc */
for (i = GC_AKEY; i < GC_CONT; i++) {
src_bin = &cont->cd_gc_bins[i];
Expand All @@ -305,13 +314,7 @@ gc_drain_cont(struct vos_gc *gc, struct vos_pool *pool, daos_handle_t coh,
static int
gc_free_cont(struct vos_gc *gc, struct vos_pool *pool, daos_handle_t coh, struct vos_gc_item *item)
{
int rc;

rc = vos_dtx_table_destroy(&pool->vp_umm, umem_off2ptr(&pool->vp_umm, item->it_addr));
if (rc == 0)
rc = umem_free(&pool->vp_umm, item->it_addr);

return rc;
return umem_free(&pool->vp_umm, item->it_addr);
}

static struct vos_gc gc_table[] = {
Expand Down
2 changes: 1 addition & 1 deletion src/vos/vos_obj.c
Original file line number Diff line number Diff line change
Expand Up @@ -2597,7 +2597,7 @@ vos_obj_iter_aggregate(daos_handle_t ih, bool range_discard)
* be aborted. Then it will be added and handled via GC when ktr_rec_free().
*/

rc = dbtree_iter_delete(oiter->it_hdl, NULL);
rc = dbtree_iter_delete(oiter->it_hdl, obj->obj_cont);
D_ASSERT(rc != -DER_NONEXIST);
} else if (rc == -DER_NONEXIST) {
/* Key no longer exists at epoch but isn't empty */
Expand Down
5 changes: 4 additions & 1 deletion src/vos/vos_obj_index.c
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,7 @@ oi_iter_aggregate(daos_handle_t ih, bool range_discard)
struct vos_container *cont = oiter->oit_cont;
struct vos_obj_df *obj;
daos_unit_oid_t oid;
struct oi_delete_arg del_arg;
d_iov_t rec_iov;
bool delete = false, invisible = false;
int rc;
Expand Down Expand Up @@ -915,7 +916,9 @@ oi_iter_aggregate(daos_handle_t ih, bool range_discard)
if (rc != 0)
D_ERROR("Could not evict object "DF_UOID" "DF_RC"\n",
DP_UOID(oid), DP_RC(rc));
rc = dbtree_iter_delete(oiter->oit_hdl, NULL);
del_arg.cont = oiter->oit_cont;
del_arg.only_delete_entry = 0;
rc = dbtree_iter_delete(oiter->oit_hdl, &del_arg);
D_ASSERT(rc != -DER_NONEXIST);
} else if (rc == -DER_NONEXIST) {
/** ilog isn't visible in range but still has some entries */
Expand Down

0 comments on commit 9ba1370

Please sign in to comment.