From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 30 Jul 2019 13:54:32 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] Re: [PATCH 4/4] txn: undo commit/rollback triggers when reverting to savepoint Message-ID: <20190730105432.GH4659@esperanza> References: <20190729131150.GA16930@atlas> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190729131150.GA16930@atlas> To: Konstantin Osipov Cc: tarantool-patches@freelists.org List-ID: Please take a look at an alternative solution then: https://www.freelists.org/post/tarantool-patches/PATCH-v2-03-Support-savepoints-in-DDL-transactions Rather than trying to maintain global commit/rollback trigger list and removing triggers from them, the second version of this patch simply attaches triggers to txn statements. I hope you'll find it easier for understanding. On Mon, Jul 29, 2019 at 04:11:50PM +0300, Konstantin Osipov wrote: > * Vladimir Davydov [19/07/19 21:09]: > > I spent some time thinking about this patch. > > I agree we need to make commit and rollback > triggers, as well as some SQL related members, part of the > savepoint state. > > DDL now can be part of a transaction, and since we may roll back > to a savepoint, we need to make sure that all parts of the > transaction, including a commit/rollback trigger, are part of > the savepoint. > > However the patch itself suffers from quite a bit of > rot. > > Initially, the idea of struct txn was very simple: it is a linked > list of txn_stmt objects, and each txn_stmt object represents a > change in transaction state. The history is easy to navigate over, > and a savepoint is just a link inside the history chain. > > commit/rollback triggers were not part of this story because they > were only used for DDL which was non-transactional. > > This code was allowed to rot and when some SQL related members > were added directly to struct txn without considering the issue of > sub-statements and savepoints. > > This patch, in this vein, instead of designing the transaction > system to make sure that every part of transaction state is part > of a change history, adds a few extra savepoint members and > makes sure that transaction state is carefully copied to the > savepoint when it is created. This is contrary to how savepoints > used to work, when a savepoint was just a link in the history of > changes of the transaction state. > > As an ugly artefact, we store such members as has_triggers, > on_commit, on_rollback in struct txn twice: once in struct txn > itself, and another time in txn->in_sub_stmt[0], which is > initialized along with the txn. > > So I think that some work needs to be done to move > transaction state which can participate in a savepoint to > either an existing object representing a piece of the transaction > history (struct txn_stmt) or a new object (txn_stmt_chain, > txn_savepoint_state or something similar), the transaction is > represented as a chain of such objects, the first > object in the chain is a member of struct txn and is created when > the transaction starts, and the savepoint can continue to be > just a link in that chain. > > Let's discuss this when you have a chance.