From: Vladimir Davydov <vdavydov.dev@gmail.com> To: Konstantin Osipov <kostja@tarantool.org> Cc: tarantool-patches@freelists.org Subject: Re: [tarantool-patches] Re: [PATCH 1/4] vinyl: introduce hash of spaces affected by transaction Date: Thu, 28 Mar 2019 16:58:51 +0300 [thread overview] Message-ID: <20190328135851.i5ugcq7ovqfpyddz@esperanza> (raw) In-Reply-To: <20190328132552.GA19174@chai> On Thu, Mar 28, 2019 at 04:25:52PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov <vdavydov.dev@gmail.com> [19/03/24 00:10]: > > We need to abort all transactions writing to an altered space when > > a new index is build. 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 introduces a hash of spaces affected > > by each transaction and makes tx_manager_abort_writers_for_ddl() use it > > instead of the write set to abort transactions for DDL. > > I think this is an overkill for the problem you're dealing with. > > The root of the problem is that a vinyl statement is only added to tx > write set after a read/write has been performed, not before. The > moment we started using tx write set for aborts, we should have > fixed this, since one locks before one writes. > > The approach you chose also adds struct space to vy_tx. I know > that there already is a dependency on struct space in vinyl, but > why make it stronger? > > Now, to track before use, you would need to add a surrogate txv to > tx write set at the beginning of vy_update/vy_replace/etc, not at > the end of it. It doesn't seem difficult to do, but let's assume We'd have to handle this in txw iterator. I'd rather avoid it. At least, at this point. > for a moment it is, and it is indeed perhaps more than we want to > do in 1.10 at this point. > > > Provided we can't track before use, do we really need a hash of all > spaces just to cover the small gap between a read and a track? Why > not have a single vy_tx member struct space *last_stmt_space, > for the purposes of abort? It could be set at begin_statement() and > cleared in rollback_statement() just the same. This sounds perfect to me. I'll rework the patch accordingly. > > In future XM object will be global and we'll keep a list of > struct txn in it, not struct vy_tx. Once we begin doing it, > aborting writers for ddl and ro will also become straightforward. > This is a yet another direction at which one could develop this > patch - but I guess it's only acceptable for 2.x, not for 1.10. Yeah. I'd even refrain from doing this in 2.x until we have a roadmap for the common transaction manager. > > I know there is a subsequent patch that adds a nice feature - > local transactions are not aborted when switching to RO. Well, > it's nice, but not important for this problem, so I think this > patch could be dropped. Okay.
next prev parent reply other threads:[~2019-03-28 13:58 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-23 21:07 [PATCH 0/4] Fix DML vs DDL race Vladimir Davydov 2019-03-23 21:07 ` [PATCH 1/4] vinyl: introduce hash of spaces affected by transaction Vladimir Davydov 2019-03-28 13:25 ` [tarantool-patches] " Konstantin Osipov 2019-03-28 13:58 ` Vladimir Davydov [this message] 2019-03-23 21:07 ` [PATCH 2/4] vinyl: don't abort transactions that modify only local spaces for ro Vladimir Davydov 2019-03-25 5:27 ` [tarantool-patches] " Георгий Кириченко 2019-03-25 8:13 ` Vladimir Davydov 2019-03-25 8:58 ` Георгий Кириченко 2019-03-25 9:30 ` Vladimir Davydov 2019-03-23 21:07 ` [PATCH 3/4] vinyl: abort affected transactions when space is removed from cache Vladimir Davydov 2019-03-25 5:26 ` [tarantool-patches] " Георгий Кириченко 2019-03-25 8:17 ` Vladimir Davydov [not found] ` <1564677.EMV258VVK2@home.lan> 2019-03-25 9:51 ` Vladimir Davydov 2019-03-25 5:45 ` Георгий Кириченко 2019-03-25 8:21 ` Vladimir Davydov 2019-03-25 9:03 ` Георгий Кириченко 2019-03-28 13:45 ` [tarantool-patches] " Konstantin Osipov 2019-03-28 14:02 ` Vladimir Davydov 2019-03-28 14:12 ` Konstantin Osipov 2019-03-23 21:07 ` [PATCH 4/4] Revert "test: skip ddl test for vinyl on travis" Vladimir Davydov 2019-03-28 13:46 ` [tarantool-patches] " Konstantin Osipov
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=20190328135851.i5ugcq7ovqfpyddz@esperanza \ --to=vdavydov.dev@gmail.com \ --cc=kostja@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] Re: [PATCH 1/4] vinyl: introduce hash of spaces affected by transaction' \ /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