DAOS-16876 vos: remove DTX record after partial commit - b26 (#15858)

Otherwise, the partial committed DTX entry will be re-committed when reopen the container. Then access related dangling DTX record(s) may trigger assertion and cause corruption. Signed-off-by: Fan Yong <[email protected]>
daos-stack · Feb 10, 2025 · 15912c1 · 15912c1
1 parent e9f1697
commit 15912c1
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 39 deletions.
diff --git a/src/dtx/dtx_resync.c b/src/dtx/dtx_resync.c
@@ -1,5 +1,6 @@
 /**
  * (C) Copyright 2019-2024 Intel Corporation.
+ * (C) Copyright 2025 Hewlett Packard Enterprise Development LP
  *
  * SPDX-License-Identifier: BSD-2-Clause-Patent
  */
@@ -87,30 +88,18 @@ dtx_resync_commit(struct ds_cont_child *cont,
 		 */
 		rc = vos_dtx_check(cont->sc_hdl, &dre->dre_xid, NULL, NULL, NULL, false);
 
-		/* Skip this DTX since it has been committed or aggregated. */
-		if (rc == DTX_ST_COMMITTED || rc == DTX_ST_COMMITTABLE || rc == -DER_NONEXIST)
-			goto next;
-
-		/* Remote ones are all ready, but local is not, then abort such DTX.
-		 * If related RPC sponsor is still alive, related RPC will be resent.
-		 */
-		if (unlikely(rc == DTX_ST_INITED)) {
-			rc = dtx_abort(cont, &dre->dre_dte, dre->dre_epoch);
-			D_DEBUG(DB_TRACE, "As new leader for DTX "DF_DTI", abort it (1): "DF_RC"\n",
-				DP_DTI(&dre->dre_dte.dte_xid), DP_RC(rc));
-			goto next;
-		}
-
-		/* If we failed to check the status, then assume that it is
+		/*
+		 * Skip this DTX since it has been committed or aggregated.
+		 * If we failed to check the status, then assume that it is
 		 * not committed, then commit it (again), that is harmless.
 		 */
+		if (rc != DTX_ST_COMMITTED && rc != -DER_NONEXIST) {
+			dtes[j] = dtx_entry_get(&dre->dre_dte);
+			dcks[j].oid = dre->dre_oid;
+			dcks[j].dkey_hash = dre->dre_dkey_hash;
+			j++;
+		}
 
-		dtes[j] = dtx_entry_get(&dre->dre_dte);
-		dcks[j].oid = dre->dre_oid;
-		dcks[j].dkey_hash = dre->dre_dkey_hash;
-		j++;
-
-next:
 		dtx_dre_release(drh, dre);
 	}
 

diff --git a/src/vos/vos_dtx.c b/src/vos/vos_dtx.c
@@ -1,5 +1,6 @@
 /**
  * (C) Copyright 2019-2024 Intel Corporation.
+ * (C) Copyright 2025 Hewlett Packard Enterprise Development LP
  *
  * SPDX-License-Identifier: BSD-2-Clause-Patent
  */
@@ -208,6 +209,7 @@ dtx_act_ent_cleanup(struct vos_container *cont, struct vos_dtx_act_ent *dae,
 		dae->dae_oid_cnt = 0;
 	}
 
+	DAE_REC_OFF(dae) = UMOFF_NULL;
 	D_FREE(dae->dae_records);
 	dae->dae_rec_cap = 0;
 	DAE_REC_CNT(dae) = 0;
@@ -683,50 +685,61 @@ dtx_rec_release(struct vos_container *cont, struct vos_dtx_act_ent *dae, bool ab
 
 	if (dae->dae_records != NULL) {
 		D_ASSERT(DAE_REC_CNT(dae) > DTX_INLINE_REC_CNT);
+		D_ASSERT(!UMOFF_IS_NULL(dae_df->dae_rec_off));
 
-		for (i = DAE_REC_CNT(dae) - DTX_INLINE_REC_CNT - 1;
-		     i >= 0; i--) {
-			rc = do_dtx_rec_release(umm, cont, dae,
-						dae->dae_records[i], abort);
+		for (i = DAE_REC_CNT(dae) - DTX_INLINE_REC_CNT - 1; i >= 0; i--) {
+			rc = do_dtx_rec_release(umm, cont, dae, dae->dae_records[i], abort);
 			if (rc != 0)
 				return rc;
 		}
 
+		rc = umem_free(umm, dae_df->dae_rec_off);
+		if (rc != 0)
+			return rc;
+
+		if (keep_act) {
+			rc = umem_tx_add_ptr(umm, &dae_df->dae_rec_off, sizeof(dae_df->dae_rec_off));
+			if (rc != 0)
+				return rc;
+
+			dae_df->dae_rec_off = UMOFF_NULL;
+		}
+
 		count = DTX_INLINE_REC_CNT;
 	} else {
+		D_ASSERT(DAE_REC_CNT(dae) <= DTX_INLINE_REC_CNT);
+
 		count = DAE_REC_CNT(dae);
 	}
 
 	for (i = count - 1; i >= 0; i--) {
-		rc = do_dtx_rec_release(umm, cont, dae, DAE_REC_INLINE(dae)[i],
-					abort);
-		if (rc != 0)
-			return rc;
-	}
-
-	if (!UMOFF_IS_NULL(dae_df->dae_rec_off)) {
-		rc = umem_free(umm, dae_df->dae_rec_off);
+		rc = do_dtx_rec_release(umm, cont, dae, DAE_REC_INLINE(dae)[i], abort);
 		if (rc != 0)
 			return rc;
 	}
 
 	if (keep_act) {
+		/* When re-commit partial committed DTX, the count can be zero. */
+		if (dae_df->dae_rec_cnt > 0) {
+			rc = umem_tx_add_ptr(umm, &dae_df->dae_rec_cnt,
+					     sizeof(dae_df->dae_rec_cnt));
+			if (rc != 0)
+				return rc;
+
+			dae_df->dae_rec_cnt = 0;
+		}
+
 		/*
 		 * If it is required to keep the active DTX entry, then it must be for partial
 		 * commit. Let's mark it as DTE_PARTIAL_COMMITTED.
 		 */
 		if ((DAE_FLAGS(dae) & DTE_PARTIAL_COMMITTED))
 			return 0;
 
-		rc = umem_tx_add_ptr(umm, &dae_df->dae_rec_off, sizeof(dae_df->dae_rec_off));
-		if (rc != 0)
-			return rc;
-
 		rc = umem_tx_add_ptr(umm, &dae_df->dae_flags, sizeof(dae_df->dae_flags));
 		if (rc != 0)
 			return rc;
 
-		dae_df->dae_rec_off = UMOFF_NULL;
 		dae_df->dae_flags |= DTE_PARTIAL_COMMITTED;
 
 		return 0;
@@ -929,7 +942,7 @@ vos_dtx_commit_one(struct vos_container *cont, struct dtx_id *dti, daos_epoch_t
 		D_FREE(dce);
 
 	if (rm_cos != NULL &&
-	    (rc == 0 || rc == -DER_NONEXIST || (rc == -DER_ALREADY && dae == NULL)))
+	    ((rc == 0 && !keep_act) || rc == -DER_NONEXIST || (rc == -DER_ALREADY && dae == NULL)))
 		*rm_cos = true;
 
 	return rc;