From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E5FBB445320 for ; Thu, 16 Jul 2020 03:11:00 +0300 (MSK) References: <1594821336-14468-1-git-send-email-alyapunov@tarantool.org> <1594821336-14468-9-git-send-email-alyapunov@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Thu, 16 Jul 2020 02:10:59 +0200 MIME-Version: 1.0 In-Reply-To: <1594821336-14468-9-git-send-email-alyapunov@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v3 08/13] txm: introduce tx manager List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aleksandr Lyapunov , tarantool-patches@dev.tarantool.org Thanks for the patch! > diff --git a/src/box/txn.h b/src/box/txn.h > index 962ada0..a2374f3 100644 > --- a/src/box/txn.h > +++ b/src/box/txn.h > @@ -700,6 +706,12 @@ box_txn_savepoint(void); > API_EXPORT int > box_txn_rollback_to_savepoint(box_txn_savepoint_t *savepoint); > > +void > +tx_manager_init(); In addition to Nikita's comment about adding memtx_ prefix to the tx manager, there a more global problem - the memtx tx manager lives in txn.h/.c. But since it is memtx-specific, it would be more correct to have a separate file for it, memtx_tx.h, like we have for vinyl vy_tx.h/.c. Otherwise this manager took place of the future global tx manager. What is also bothering me is that you use struct txn and struct txn_stmt for the memtx-specific tx manager things. But specially for engine-local transactions we already have txn->engine_tx and txn_stmt->engine_savepoint. The same for txm_story, conflict manager and all the other new structs and functions. They all are called txm, and are located in txn.c, but are related to memtx only, and are used only in it. So my old comment about moving the new code into memtx-specific structs is relevant again.