From: Vladimir Davydov <vdavydov.dev@gmail.com> To: kostja@tarantool.org Cc: tarantool-patches@freelists.org Subject: [PATCH v2 1/3] vinyl: make tx_manager_abort_writers_for_ddl more thorough Date: Thu, 28 Mar 2019 18:52:27 +0300 [thread overview] Message-ID: <1574fba56e436ce5129fc0a2853afb3f8c946279.1553787937.git.vdavydov.dev@gmail.com> (raw) In-Reply-To: <cover.1553787937.git.vdavydov.dev@gmail.com> In-Reply-To: <cover.1553787937.git.vdavydov.dev@gmail.com> We need to abort all transactions writing to an altered space when a new index is built. Currently, we use the write set to look up such transactions, but it isn't quite correct, because a transaction could yield on disk read before inserting a statement into the write set. To address this problem, this patch adds vy_tx->last_lsm, which points to the primary index of the space affected by the last prepared transaction. Now, tx_manager_abort_writers_for_ddl will look not only at the write set, but also at this variable to check if it needs to abort a transaction. Needed for #3420 --- src/box/vinyl.c | 4 +++- src/box/vy_tx.c | 8 ++++++-- src/box/vy_tx.h | 15 ++++++++++++++- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 3ef43e18..24f900cb 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -2433,8 +2433,10 @@ vinyl_engine_begin_statement(struct engine *engine, struct txn *txn) (void)engine; struct vy_tx *tx = txn->engine_tx; struct txn_stmt *stmt = txn_current_stmt(txn); + struct index *pk = space_index(stmt->space, 0); assert(tx != NULL); - return vy_tx_begin_statement(tx, &stmt->engine_savepoint); + return vy_tx_begin_statement(tx, pk != NULL ? vy_lsm(pk) : NULL, + &stmt->engine_savepoint); } static void diff --git a/src/box/vy_tx.c b/src/box/vy_tx.c index 1b8224f4..889e094e 100644 --- a/src/box/vy_tx.c +++ b/src/box/vy_tx.c @@ -322,6 +322,7 @@ vy_tx_read_set_free_cb(vy_tx_read_set_t *read_set, void vy_tx_create(struct tx_manager *xm, struct vy_tx *tx) { + tx->last_lsm = NULL; stailq_create(&tx->log); write_set_new(&tx->write_set); tx->write_set_version = 0; @@ -869,13 +870,14 @@ vy_tx_rollback(struct vy_tx *tx) } int -vy_tx_begin_statement(struct vy_tx *tx, void **savepoint) +vy_tx_begin_statement(struct vy_tx *tx, struct vy_lsm *lsm, void **savepoint) { if (tx->state == VINYL_TX_ABORT) { diag_set(ClientError, ER_TRANSACTION_CONFLICT); return -1; } assert(tx->state == VINYL_TX_READY); + tx->last_lsm = lsm; if (stailq_empty(&tx->log)) rlist_add_entry(&tx->xm->writers, tx, in_writers); *savepoint = stailq_last(&tx->log); @@ -1112,7 +1114,9 @@ tx_manager_abort_writers_for_ddl(struct tx_manager *xm, struct vy_lsm *lsm) { struct vy_tx *tx; rlist_foreach_entry(tx, &xm->writers, in_writers) { - if (tx->state == VINYL_TX_READY && + if (tx->state != VINYL_TX_READY) + continue; + if (tx->last_lsm == lsm || write_set_search_key(&tx->write_set, lsm, lsm->env->empty_key) != NULL) vy_tx_abort(tx); diff --git a/src/box/vy_tx.h b/src/box/vy_tx.h index aaa31bee..1767b509 100644 --- a/src/box/vy_tx.h +++ b/src/box/vy_tx.h @@ -140,6 +140,16 @@ struct vy_tx { /** Transaction manager. */ struct tx_manager *xm; /** + * Pointer to the primary index LSM tree of the space + * affected by the last prepared statement. + * + * We need it so that we can abort a transaction on DDL + * even if it hasn't inserted anything into the write set + * yet (e.g. yielded on unique check) and therefore would + * otherwise be ignored by tx_manager_abort_writers_for_ddl. + */ + struct vy_lsm *last_lsm; + /** * In memory transaction log. Contains both reads * and writes. */ @@ -325,9 +335,12 @@ vy_tx_rollback(struct vy_tx *tx); * Return the save point corresponding to the current * transaction state. The transaction can be rolled back * to a save point with vy_tx_rollback_statement(). + * + * @lsm is supposed to point to the LSM tree corresponding + * to the primary index of the affected space. */ int -vy_tx_begin_statement(struct vy_tx *tx, void **savepoint); +vy_tx_begin_statement(struct vy_tx *tx, struct vy_lsm *lsm, void **savepoint); /** * Rollback a transaction statement. -- 2.11.0
next prev parent reply other threads:[~2019-03-28 15:52 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-28 15:52 [PATCH v2 0/3] Fix DML vs DDL race Vladimir Davydov 2019-03-28 15:52 ` Vladimir Davydov [this message] 2019-03-28 16:03 ` [PATCH v2 1/3] vinyl: make tx_manager_abort_writers_for_ddl more thorough Vladimir Davydov 2019-03-28 17:28 ` [tarantool-patches] " Konstantin Osipov 2019-03-28 15:52 ` [PATCH v2 2/3] vinyl: abort affected transactions when space is removed from cache Vladimir Davydov 2019-03-28 15:52 ` [PATCH v2 3/3] Revert "test: skip ddl test for vinyl on travis" Vladimir Davydov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1574fba56e436ce5129fc0a2853afb3f8c946279.1553787937.git.vdavydov.dev@gmail.com \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [PATCH v2 1/3] vinyl: make tx_manager_abort_writers_for_ddl more thorough' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox