From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 36FB929E28 for ; Thu, 28 Mar 2019 09:25:55 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id yNezD2nrmcRM for ; Thu, 28 Mar 2019 09:25:55 -0400 (EDT) Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id E1CF829DA5 for ; Thu, 28 Mar 2019 09:25:54 -0400 (EDT) Received: by smtp48.i.mail.ru with esmtpa (envelope-from ) id 1h9V2W-0004z8-Qp for tarantool-patches@freelists.org; Thu, 28 Mar 2019 16:25:53 +0300 Date: Thu, 28 Mar 2019 16:25:52 +0300 From: Konstantin Osipov Subject: [tarantool-patches] Re: [PATCH 1/4] vinyl: introduce hash of spaces affected by transaction Message-ID: <20190328132552.GA19174@chai> References: <77567cf07633e5b3f8c60772cbf7fc702cf15297.1553373792.git.vdavydov.dev@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <77567cf07633e5b3f8c60772cbf7fc702cf15297.1553373792.git.vdavydov.dev@gmail.com> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org * Vladimir Davydov [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 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. 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. 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. -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov