From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 6B8ED46970E for ; Sun, 19 Jan 2020 20:32:47 +0300 (MSK) References: <52419d26967890ace8245f46fdff0604f919a029.1579211601.git.v.shpilevoy@tarantool.org> <2144259.ElGaqSPkdT@localhost> From: Vladislav Shpilevoy Message-ID: Date: Sun, 19 Jan 2020 18:32:45 +0100 MIME-Version: 1.0 In-Reply-To: <2144259.ElGaqSPkdT@localhost> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 3/3] box: remove dead code from box_process_call/eval() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Georgy Kirichenko , tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com Hi! Thanks for the review! On 17/01/2020 18:41, Georgy Kirichenko wrote: > On Friday, 17 January 2020 00:54:23 MSK Vladislav Shpilevoy wrote: >> box_process_call/eval() in the end check if there is an >> active transaction. If there is, it is rolled back, and >> an error is set. >> >> But rollback is not needed anymore, because anyway in >> the end of the request the fiber is stopped, and its >> not finished transaction is rolled back. Just setting >> of the error is enough. > > Hi! > Thanks for the patch, but I do not think that to remove an explicit rollback > is a a good idea because of broken encapsulation - call and eval handlers > should not rely on its execution context - a simple fiber, iproto fiber pool > member or whatever else. Also I would like to mention that box_call and > box_eval are members of the public api and it is not necessary that user will > stop a fiber. Functions box_call and box_eval don't exist. So I don't understand what you are talking about. Assume you talked about box_process_call/eval. In that case you are wrong - they are not a part of the public API. They are always called from iproto.cc only. Besides, we will need to remove ER_FUNCTION_TX_ACTIVE and all the other txn stuff from them anyway after interactive transactions are ready.