Skip to content

DAOS-19284 vos: check conflict among distributed transactions #18814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Nasf-Fan wants to merge 1 commit into
base: master
Choose a base branch
from
Open

DAOS-19284 vos: check conflict among distributed transactions #18814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/include/daos/dtx.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,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;
}
Expand Down
12 changes: 7 additions & 5 deletions src/vos/tests/vts_mvcc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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" :
Expand Down
9 changes: 4 additions & 5 deletions src/vos/tests/vts_pm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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);
Expand Down
9 changes: 6 additions & 3 deletions src/vos/vos_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1748,7 +1748,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) {
Expand Down Expand Up @@ -2690,8 +2691,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);
Expand Down
7 changes: 4 additions & 3 deletions src/vos/vos_iterator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion src/vos/vos_obj.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
20 changes: 15 additions & 5 deletions src/vos/vos_query.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
Expand All @@ -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:
Expand Down
6 changes: 2 additions & 4 deletions src/vos/vos_ts.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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
Expand Down
96 changes: 70 additions & 26 deletions src/vos/vos_ts.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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 "

@liw liw Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Nit] There seems to be no reason to warn users, because we can't tell if they have set up or done anything risky or wrong. I'd write D_DEBUG, but D_INFO might be tolerable too.

@Nasf-Fan Nasf-Fan Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, such log message is mainly for developer for more clue when duplicated epoch being used by multiple transactions. Usually, CI test (and user) may not enable DEBUG/INFO level log and such case is rare, then it is set as D_WARN(). We can consider to decrease the level to D_INFO or D_DEBUG sometime later or when we found such log message too noisy.

"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 */
Expand Down Expand Up @@ -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
Comment thread
knard38 marked this conversation as resolved.
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;
Expand All @@ -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
Expand Down
Loading