diff --git a/src/include/daos/dtx.h b/src/include/daos/dtx.h index b91e5a1084c..acdc15bc961 100644 --- a/src/include/daos/dtx.h +++ b/src/include/daos/dtx.h @@ -1,6 +1,6 @@ /** * (C) Copyright 2019-2023 Intel Corporation. - * (C) Copyright 2025 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -256,7 +256,7 @@ daos_is_zero_dti(const struct dtx_id *dti) } static inline bool -daos_dti_equal(struct dtx_id *dti0, struct dtx_id *dti1) +daos_dti_equal(const struct dtx_id *dti0, const struct dtx_id *dti1) { return memcmp(dti0, dti1, sizeof(*dti0)) == 0; } diff --git a/src/vos/tests/vts_mvcc.c b/src/vos/tests/vts_mvcc.c index f6f1e40dc5d..84d05743c7d 100644 --- a/src/vos/tests/vts_mvcc.c +++ b/src/vos/tests/vts_mvcc.c @@ -1,6 +1,6 @@ /* * (C) Copyright 2020-2023 Intel Corporation. - * (C) Copyright 2025 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -1273,11 +1273,13 @@ conflicting_rw_exec_one(struct io_test_args *arg, int i, int j, bool empty, expect_inprogress = true; } } - if (w->o_rtype == R_E && !e) - expected_wrc = -DER_EXIST; - else if (w->o_rtype == R_NE && e) - expected_wrc = -DER_NONEXIST; + if (re != we || same_tx) { + if (w->o_rtype == R_E && !e) + expected_wrc = -DER_EXIST; + else if (w->o_rtype == R_NE && e) + expected_wrc = -DER_NONEXIST; + } } print_message(" %s(%s, "DF_X64") (expect %s): ", w->o_name, wp, we, expect_inprogress ? "DER_INPROGRESS" : diff --git a/src/vos/tests/vts_pm.c b/src/vos/tests/vts_pm.c index 97049b9fc19..f29d4430e57 100644 --- a/src/vos/tests/vts_pm.c +++ b/src/vos/tests/vts_pm.c @@ -1,6 +1,6 @@ /** * (C) Copyright 2019-2022 Intel Corporation. - * (C) Copyright 2025 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -1310,12 +1310,11 @@ cond_test(void **state) cond_fetch_op(state, arg->ctx.tc_co_hdl, oid, epoch++, true, "a", "b", 0, 0, sgl, "xxxx", 'x'); /** Conditional update of non-existent key should fail */ - cond_update_op(state, arg->ctx.tc_co_hdl, oid, epoch - 1, "a", "b", - VOS_OF_COND_DKEY_UPDATE, + cond_update_op(state, arg->ctx.tc_co_hdl, oid, epoch++, "a", "b", VOS_OF_COND_DKEY_UPDATE, -DER_NONEXIST, sgl, "foo"); /** Conditional punch of non-existent akey should fail */ - cond_akey_punch_op(state, arg->ctx.tc_co_hdl, oid, epoch, "a", "b", - VOS_OF_COND_PUNCH, -DER_NONEXIST); + cond_akey_punch_op(state, arg->ctx.tc_co_hdl, oid, epoch++, "a", "b", VOS_OF_COND_PUNCH, + -DER_NONEXIST); /** Key doesn't exist still, that supersedes read conflict */ cond_dkey_punch_op(state, arg->ctx.tc_co_hdl, oid, epoch++, "a", VOS_OF_COND_PUNCH, -DER_NONEXIST); diff --git a/src/vos/vos_io.c b/src/vos/vos_io.c index e599677e3c5..6a7a5bc578e 100644 --- a/src/vos/vos_io.c +++ b/src/vos/vos_io.c @@ -1721,7 +1721,8 @@ vos_fetch_begin(daos_handle_t coh, daos_unit_oid_t oid, daos_epoch_t epoch, if (rc == -DER_NONEXIST || rc == 0) { vos_fetch_add_missing(ioc->ic_ts_set, dkey, iod_nr, iods); - vos_ts_set_update(ioc->ic_ts_set, ioc->ic_epr.epr_hi); + if (!vos_ts_set_update(ioc->ic_ts_set, ioc->ic_epr.epr_hi)) + rc = -DER_TX_RESTART; } if (rc != 0) { @@ -2663,8 +2664,10 @@ vos_update_end(daos_handle_t ioh, uint32_t pm_ver, daos_key_t *dkey, int err, if (err == 0) vos_ts_set_upgrade(ioc->ic_ts_set); - if (err == -DER_NONEXIST || err == -DER_EXIST || err == 0) - vos_ts_set_update(ioc->ic_ts_set, ioc->ic_epr.epr_hi); + if (err == -DER_NONEXIST || err == -DER_EXIST || err == 0) { + if (!vos_ts_set_update(ioc->ic_ts_set, ioc->ic_epr.epr_hi)) + err = -DER_TX_RESTART; + } if (err == 0) vos_ts_set_wupdate(ioc->ic_ts_set, ioc->ic_epr.epr_hi); diff --git a/src/vos/vos_iterator.c b/src/vos/vos_iterator.c index 71cc4a1e4d9..1b5a68f0f82 100644 --- a/src/vos/vos_iterator.c +++ b/src/vos/vos_iterator.c @@ -269,8 +269,8 @@ vos_iter_prepare(vos_iter_type_t type, vos_iter_param_t *param, if (rc == -DER_NONEXIST && dtx_is_valid_handle(dth)) { if (vos_ts_wcheck(ts_set, dth->dth_epoch, dth->dth_epoch_bound)) rc = -DER_TX_RESTART; - else - vos_ts_set_update(ts_set, dth->dth_epoch); + else if (!vos_ts_set_update(ts_set, dth->dth_epoch)) + rc = -DER_TX_RESTART; } if (rc != 0) vos_ts_set_free(ts_set); @@ -306,7 +306,8 @@ vos_iter_ts_set_update(daos_handle_t ih, daos_epoch_t read_time, int rc) if (vos_ts_wcheck(iter->it_ts_set, read_time, iter->it_bound)) return -DER_TX_RESTART; - vos_ts_set_update(iter->it_ts_set, read_time); + if (!vos_ts_set_update(iter->it_ts_set, read_time)) + return -DER_TX_RESTART; return rc; } diff --git a/src/vos/vos_obj.c b/src/vos/vos_obj.c index 4d376d79551..70f13c85b90 100644 --- a/src/vos/vos_obj.c +++ b/src/vos/vos_obj.c @@ -578,7 +578,8 @@ vos_obj_punch(daos_handle_t coh, daos_unit_oid_t oid, daos_epoch_t epoch, if (rc == -DER_NONEXIST || rc == 0) { vos_punch_add_missing(ts_set, dkey, akey_nr, akeys); - vos_ts_set_update(ts_set, epr.epr_hi); + if (!vos_ts_set_update(ts_set, epr.epr_hi)) + rc = -DER_TX_RESTART; } if (rc == 0) { diff --git a/src/vos/vos_query.c b/src/vos/vos_query.c index b4d414012e5..371c88bf203 100644 --- a/src/vos/vos_query.c +++ b/src/vos/vos_query.c @@ -1,5 +1,6 @@ /** * (C) Copyright 2019-2024 Intel Corporation. + * (C) Copyright 2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -764,8 +765,11 @@ vos_obj_query_key(daos_handle_t coh, daos_unit_oid_t oid, uint32_t flags, /** Go ahead and save timestamps for * things we read */ - vos_ts_set_update(query->qt_ts_set, - obj_epr.epr_hi); + if (!vos_ts_set_update(query->qt_ts_set, obj_epr.epr_hi)) { + rc = -DER_TX_RESTART; + break; + } + vos_ts_set_restore(query->qt_ts_set, &akey_save); continue; @@ -776,7 +780,11 @@ vos_obj_query_key(daos_handle_t coh, daos_unit_oid_t oid, uint32_t flags, if (rc == -DER_NONEXIST && query->qt_flags & VOS_GET_DKEY) { /** Go ahead and save timestamps for things we read */ - vos_ts_set_update(query->qt_ts_set, obj_epr.epr_hi); + if (!vos_ts_set_update(query->qt_ts_set, obj_epr.epr_hi)) { + rc = -DER_TX_RESTART; + break; + } + vos_ts_set_restore(query->qt_ts_set, &dkey_save); continue; } @@ -801,8 +809,10 @@ vos_obj_query_key(daos_handle_t coh, daos_unit_oid_t oid, uint32_t flags, rc = -DER_TX_RESTART; } - if (rc == 0 || rc == -DER_NONEXIST) - vos_ts_set_update(query->qt_ts_set, obj_epr.epr_hi); + if (rc == 0 || rc == -DER_NONEXIST) { + if (!vos_ts_set_update(query->qt_ts_set, obj_epr.epr_hi)) + rc = -DER_TX_RESTART; + } vos_ts_set_free(query->qt_ts_set); free_query: diff --git a/src/vos/vos_ts.c b/src/vos/vos_ts.c index cc8c214a72a..3ed2883a374 100644 --- a/src/vos/vos_ts.c +++ b/src/vos/vos_ts.c @@ -1,5 +1,6 @@ /** * (C) Copyright 2020-2024 Intel Corporation. + * (C) Copyright 2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -375,10 +376,7 @@ vos_ts_check_conflict(daos_epoch_t read_time, const struct dtx_id *read_id, if (write_time != read_time) return true; - if (read_id->dti_hlc != write_id->dti_hlc) - return true; - - return uuid_compare(read_id->dti_uuid, write_id->dti_uuid) != 0; + return !daos_dti_equal(write_id, read_id); } bool diff --git a/src/vos/vos_ts.h b/src/vos/vos_ts.h index 379f59f819c..7239cc15cc4 100644 --- a/src/vos/vos_ts.h +++ b/src/vos/vos_ts.h @@ -1,5 +1,6 @@ /** * (C) Copyright 2020-2023 Intel Corporation. + * (C) Copyright 2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -682,27 +683,67 @@ vos_ts_copy(daos_epoch_t *dest_epc, struct dtx_id *dest_id, } /** Internal API to update low read timestamp and tx id */ -static inline void -vos_ts_rl_update(struct vos_ts_entry *entry, daos_epoch_t read_time, - const struct dtx_id *tx_id) +static inline int +vos_ts_rl_update(struct vos_ts_entry *entry, daos_epoch_t read_time, const struct dtx_id *tx_id) { if (entry == NULL || read_time < entry->te_ts.tp_ts_rl) - return; + return 0; + + /* + * Different servers may generate the same timestamp that may be used as + * the epoch for different distributed transactions. So do NOT update if + * "read_time == entry->te_ts.tp_ts_rl" to avoid lost read timestamp for + * others by race. In theory, two read transactions do not conflict with + * each other, but read maybe just part of the transaction. Since we can + * keep only one on such entry, then have to request another to restart. + * That may cause some fake conflict, but correctness is more important. + */ + if (read_time == entry->te_ts.tp_ts_rl) { + if (likely(daos_dti_equal(tx_id, &entry->te_ts.tp_tx_rl))) + return 0; + + /* It is very rare, so warning message will not be much trouble. */ + D_WARN("Refuse low read-TS update with the same epoch " DF_X64 " from multiple " + "sponsors: " DF_DTI " vs " DF_DTI "\n", + read_time, DP_DTI(tx_id), DP_DTI(&entry->te_ts.tp_tx_rl)); + return -DER_TX_RESTART; + } vos_ts_copy(&entry->te_ts.tp_ts_rl, &entry->te_ts.tp_tx_rl, read_time, tx_id); + return 0; } /** Internal API to update high read timestamp and tx id */ -static inline void -vos_ts_rh_update(struct vos_ts_entry *entry, daos_epoch_t read_time, - const struct dtx_id *tx_id) +static inline int +vos_ts_rh_update(struct vos_ts_entry *entry, daos_epoch_t read_time, const struct dtx_id *tx_id) { if (entry == NULL || read_time < entry->te_ts.tp_ts_rh) - return; + return 0; + + /* + * Different servers may generate the same timestamp that may be used as + * the epoch for different distributed transactions. So do NOT update if + * "read_time == entry->te_ts.tp_ts_rh" to avoid lost read timestamp for + * others by race. In theory, two read transactions do not conflict with + * each other, but read maybe just part of the transaction. Since we can + * keep only one on such entry, then have to request another to restart. + * That may cause some fake conflict, but correctness is more important. + */ + if (read_time == entry->te_ts.tp_ts_rh) { + if (likely(daos_dti_equal(tx_id, &entry->te_ts.tp_tx_rh))) + return 0; + + /* It is very rare, so warning message will not be much trouble. */ + D_WARN("Refuse high read-TS update with the same epoch " DF_X64 " from multiple " + "sponsors: " DF_DTI " vs " DF_DTI "\n", + read_time, DP_DTI(tx_id), DP_DTI(&entry->te_ts.tp_tx_rh)); + return -DER_TX_RESTART; + } vos_ts_copy(&entry->te_ts.tp_ts_rh, &entry->te_ts.tp_tx_rh, read_time, tx_id); + return 0; } /** Internal API to check read conflict of a given entry */ @@ -784,24 +825,27 @@ vos_ts_set_append_cflags(struct vos_ts_set *ts_set, uint16_t flags) /** Update the read timestamps for the set after a successful operation * - * \param[in] ts_set The timestamp set - * \param[in] read_time The new read timestamp + * \param[in] ts_set The timestamp set + * \param[in] read_time The new read timestamp + * + * \return true on success, false if need to restart. */ -static inline void +static inline bool vos_ts_set_update(struct vos_ts_set *ts_set, daos_epoch_t read_time) { - struct vos_ts_set_entry *se; - int i; - uint16_t read_level; + struct vos_ts_set_entry *se; + uint16_t read_level; + int i; + int rc; if (!vos_ts_in_tx(ts_set)) - return; + return true; if (DAOS_FAIL_CHECK(DAOS_DTX_NO_READ_TS)) - return; + return true; if ((ts_set->ts_cflags & VOS_TS_READ_MASK) == 0) - return; + return true; if (ts_set->ts_max_type < ts_set->ts_rd_level) read_level = ts_set->ts_max_type; @@ -812,16 +856,16 @@ vos_ts_set_update(struct vos_ts_set *ts_set, daos_epoch_t read_time) se = &ts_set->ts_entries[i]; if (se->se_etype > read_level) - continue; /** We would have updated the high - * timestamp at a higher level - */ - - if (se->se_etype == read_level) - vos_ts_rl_update(se->se_entry, read_time, - &ts_set->ts_tx_id); - vos_ts_rh_update(se->se_entry, read_time, - &ts_set->ts_tx_id); + continue; + + rc = vos_ts_rh_update(se->se_entry, read_time, &ts_set->ts_tx_id); + if (rc == 0 && se->se_etype == read_level) + rc = vos_ts_rl_update(se->se_entry, read_time, &ts_set->ts_tx_id); + if (rc != 0) + return false; } + + return true; } static inline void