Skip to content

Commit

Permalink
vinyl: check key uniqueness before modifying tx write set
Browse files Browse the repository at this point in the history
Currently, we handle INSERT/REPLACE/UPDATE requests by iterating over
all space indexes starting from the primary and inserting the
corresponding statements to tx write set, checking key uniqueness if
necessary. This means that by the time we write a REPLACE to the write
set of a secondary index, it has already been written to the primary
index write set. This is OK, and vy_tx_prepare() relies on that to
implement the common memory level. However, this also means that when we
check uniqueness of a secondary index, the new REPLACE can be found via
the primary index. This is OK now, because all indexes are fully
independent, but it isn't going to fly after #2129 is implemented. The
problem is in order to check if a tuple is present in a secondary index,
we will have to look up the corresponding full tuple in the primary
index. To illustrate the problem, consider the following situation:

  Primary index covers field 1.
  Secondary index covers field 2.

  Committed statements:

    REPLACE{1, 10, lsn=1} - present in both indexes
    DELETE{1, lsn=2} - present only in the primary index

  Transaction:

    REPLACE{1, 10}

When we check uniqueness of the secondary index, we find committed
statement REPLACE{1, 10, lsn=1}, then look up the corresponding full
tuple in the primary index and find REPLACE{1, 10}. Since the two tuples
match, we mistakenly assume that there's a conflict.

To avoid a situation like that, let's check uniqueness before modifying
the write set of any index.

Needed for #2129
  • Loading branch information
locker authored and kostja committed Aug 15, 2018
1 parent 3800e36 commit fc3834c
Showing 1 changed file with 62 additions and 66 deletions.
128 changes: 62 additions & 66 deletions src/box/vinyl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1480,6 +1480,17 @@ vy_check_is_unique_secondary(struct vy_env *env, struct vy_tx *tx,
tuple_unref(key);
if (rc != 0)
return -1;
if (found != NULL && vy_tuple_compare(stmt, found,
lsm->pk->key_def) == 0) {
/*
* If the old and new tuples are the same in
* terms of the primary key definition, the
* statement doesn't modify the secondary key
* and so there's actually no conflict.
*/
tuple_unref(found);
return 0;
}
if (found != NULL) {
tuple_unref(found);
diag_set(ClientError, ER_TUPLE_FOUND,
Expand All @@ -1490,68 +1501,51 @@ vy_check_is_unique_secondary(struct vy_env *env, struct vy_tx *tx,
}

/**
* Insert a tuple in a primary index LSM tree.
* @param env Vinyl environment.
* @param tx Current transaction.
* @param space Target space.
* @param pk Primary index LSM tree.
* @param stmt Tuple to insert.
*
* @retval 0 Success.
* @retval -1 Memory error or duplicate key error.
*/
static inline int
vy_insert_primary(struct vy_env *env, struct vy_tx *tx, struct space *space,
struct vy_lsm *pk, struct tuple *stmt)
{
assert(vy_stmt_type(stmt) == IPROTO_INSERT);
assert(tx != NULL && tx->state == VINYL_TX_READY);
assert(pk->index_id == 0);
/*
* A primary index is always unique and the new tuple must not
* conflict with existing tuples.
*/
if (vy_check_is_unique_primary(env, tx, vy_tx_read_view(tx),
space_name(space),
index_name_by_id(space, pk->index_id),
pk, stmt) != 0)
return -1;
return vy_tx_set(tx, pk, stmt);
}

/**
* Insert a tuple in a secondary index LSM tree.
* @param env Vinyl environment.
* @param tx Current transaction.
* @param space Target space.
* @param lsm Secondary index LSM tree.
* @param stmt Tuple to replace.
* Check if insertion of a new tuple violates unique constraint
* of any index of the space.
* @param env Vinyl environment.
* @param tx Current transaction.
* @param space Space to check.
* @param stmt New tuple.
*
* @retval 0 Success.
* @retval -1 Memory error or duplicate key error.
* @retval 0 Success, unique constraint is satisfied.
* @retval -1 Duplicate is found or read error occurred.
*/
static int
vy_insert_secondary(struct vy_env *env, struct vy_tx *tx, struct space *space,
struct vy_lsm *lsm, struct tuple *stmt)
vy_check_is_unique(struct vy_env *env, struct vy_tx *tx,
struct space *space, struct tuple *stmt)
{
assert(space->index_count > 0);
assert(vy_stmt_type(stmt) == IPROTO_INSERT ||
vy_stmt_type(stmt) == IPROTO_REPLACE);
assert(tx != NULL && tx->state == VINYL_TX_READY);
assert(lsm->index_id > 0);

if (vy_check_is_unique_secondary(env, tx, vy_tx_read_view(tx),
space_name(space),
index_name_by_id(space, lsm->index_id),
lsm, stmt) != 0)
return -1;
const struct vy_read_view **rv = vy_tx_read_view(tx);

/*
* We must always append the statement to transaction write set
* of each LSM tree, even if operation itself does not update
* the LSM tree, e.g. it's an UPDATE, to ensure we read our
* own writes.
* We only need to check the uniqueness of the primary index
* if this is INSERT, because REPLACE will silently overwrite
* the existing tuple, if any.
*/
return vy_tx_set(tx, lsm, stmt);
if (vy_stmt_type(stmt) == IPROTO_INSERT) {
struct vy_lsm *lsm = vy_lsm(space->index[0]);
if (vy_check_is_unique_primary(env, tx, rv, space_name(space),
index_name_by_id(space, 0),
lsm, stmt) != 0)
return -1;
}

/*
* For secondary indexes, uniqueness must be checked on both
* INSERT and REPLACE.
*/
for (uint32_t i = 1; i < space->index_count; i++) {
struct vy_lsm *lsm = vy_lsm(space->index[i]);
if (vy_check_is_unique_secondary(env, tx, rv, space_name(space),
index_name_by_id(space, i),
lsm, stmt) != 0)
return -1;
}
return 0;
}

/**
Expand Down Expand Up @@ -1772,6 +1766,8 @@ vy_update(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
if (vy_check_update(space, pk, stmt->old_tuple, stmt->new_tuple,
column_mask) != 0)
return -1;
if (vy_check_is_unique(env, tx, space, stmt->new_tuple) != 0)
return -1;

/*
* In the primary index the tuple can be replaced without
Expand All @@ -1794,7 +1790,7 @@ vy_update(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
continue;
if (vy_tx_set(tx, lsm, delete) != 0)
goto error;
if (vy_insert_secondary(env, tx, space, lsm, stmt->new_tuple))
if (vy_tx_set(tx, lsm, stmt->new_tuple) != 0)
goto error;
}
tuple_unref(delete);
Expand Down Expand Up @@ -1822,13 +1818,15 @@ vy_insert_first_upsert(struct vy_env *env, struct vy_tx *tx,
assert(tx != NULL && tx->state == VINYL_TX_READY);
assert(space->index_count > 0);
assert(vy_stmt_type(stmt) == IPROTO_INSERT);
if (vy_check_is_unique(env, tx, space, stmt) != 0)
return -1;
struct vy_lsm *pk = vy_lsm(space->index[0]);
assert(pk->index_id == 0);
if (vy_tx_set(tx, pk, stmt) != 0)
return -1;
for (uint32_t i = 1; i < space->index_count; ++i) {
struct vy_lsm *lsm = vy_lsm(space->index[i]);
if (vy_insert_secondary(env, tx, space, lsm, stmt) != 0)
if (vy_tx_set(tx, lsm, stmt) != 0)
return -1;
}
return 0;
Expand Down Expand Up @@ -2053,6 +2051,8 @@ vy_upsert(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
*/
return 0;
}
if (vy_check_is_unique(env, tx, space, stmt->new_tuple) != 0)
return -1;
if (vy_tx_set(tx, pk, stmt->new_tuple))
return -1;
if (space->index_count == 1)
Expand All @@ -2071,8 +2071,7 @@ vy_upsert(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
continue;
if (vy_tx_set(tx, lsm, delete) != 0)
goto error;
if (vy_insert_secondary(env, tx, space, lsm,
stmt->new_tuple) != 0)
if (vy_tx_set(tx, lsm, stmt->new_tuple) != 0)
goto error;
}
tuple_unref(delete);
Expand Down Expand Up @@ -2115,15 +2114,16 @@ vy_insert(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
request->tuple_end);
if (stmt->new_tuple == NULL)
return -1;
if (vy_insert_primary(env, tx, space, pk, stmt->new_tuple) != 0)
if (vy_check_is_unique(env, tx, space, stmt->new_tuple) != 0)
return -1;
if (vy_tx_set(tx, pk, stmt->new_tuple) != 0)
return -1;

for (uint32_t iid = 1; iid < space->index_count; ++iid) {
struct vy_lsm *lsm = vy_lsm(space->index[iid]);
if (vy_is_committed_one(env, lsm))
continue;
if (vy_insert_secondary(env, tx, space, lsm,
stmt->new_tuple) != 0)
if (vy_tx_set(tx, lsm, stmt->new_tuple) != 0)
return -1;
}
return 0;
Expand Down Expand Up @@ -2166,6 +2166,8 @@ vy_replace(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
request->tuple_end);
if (stmt->new_tuple == NULL)
return -1;
if (vy_check_is_unique(env, tx, space, stmt->new_tuple) != 0)
return -1;
/*
* Get the overwritten tuple from the primary index if
* the space has on_replace triggers, in which case we
Expand Down Expand Up @@ -2209,18 +2211,12 @@ vy_replace(struct vy_env *env, struct vy_tx *tx, struct txn_stmt *stmt,
struct vy_lsm *lsm = vy_lsm(space->index[i]);
if (vy_is_committed_one(env, lsm))
continue;
/*
* DELETE goes first, so if old and new keys
* fully match, there is no look up beyond the
* transaction write set.
*/
if (delete != NULL) {
rc = vy_tx_set(tx, lsm, delete);
if (rc != 0)
break;
}
rc = vy_insert_secondary(env, tx, space, lsm,
stmt->new_tuple);
rc = vy_tx_set(tx, lsm, stmt->new_tuple);
if (rc != 0)
break;
}
Expand Down

0 comments on commit fc3834c

Please sign in to comment.