From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f65.google.com (mail-lf1-f65.google.com [209.85.167.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BBF22469719 for ; Fri, 20 Mar 2020 13:58:44 +0300 (MSK) Received: by mail-lf1-f65.google.com with SMTP id m15so4165456lfp.2 for ; Fri, 20 Mar 2020 03:58:44 -0700 (PDT) Date: Fri, 20 Mar 2020 13:58:42 +0300 From: Konstantin Osipov Message-ID: <20200320105842.GA30252@atlas> References: <20200320081956.30650-1-gorcunov@gmail.com> <20200320081956.30650-12-gorcunov@gmail.com> <20200320102254.GB20273@atlas> <20200320102956.GD8326@uranus> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200320102956.GD8326@uranus> Subject: Re: [Tarantool-patches] [PATCH v15 11/11] box/journal: redesign journal operations List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tml * Cyrill Gorcunov [20/03/20 13:34]: > > > > > > - if (txn_write_to_wal(req) != 0) > > > + fiber_set_txn(fiber(), NULL); > > > + if (journal_write(req) != 0) { > > > + fiber_set_txn(fiber(), txn); > > > > I wonder why do you need to clear/set txn in txn_commit()? Forgive me for being really painful about it, but why not use different complete callbacks for sync and async wal writes?-) Under the hood they will still call txn_complete(), but one will assert, and another will not? > > Because async write engine implies that fiber's txn is dropped. > In particular > > void > txn_complete_async(struct journal_entry *entry) > { > struct txn *txn = entry->complete_data; > txn->signature = entry->res; > /* > * Some commit/rollback triggers require for in_txn fiber > * variable to be set so restore it for the time triggers > * are in progress. > */ > --> assert(in_txn() == NULL); > fiber_set_txn(fiber(), txn); > txn_complete(txn); > fiber_set_txn(fiber(), NULL); > } > > Thus to not use txn.h inside journal/wall I clear it before > the write and restore it back in case of error because rollback > needs txn bound to a fiber. > > > > + txn_rollback(txn); > > > + txn_free(txn); > > > + > > > + diag_set(ClientError, ER_WAL_IO); > > > + diag_log(); > > > return -1; > > > + } > > > > > > - /* > > > - * In case of non-yielding journal the transaction could already > > > - * be done and there is nothing to wait in such cases. > > > - */ > > > if (!txn_has_flag(txn, TXN_IS_DONE)) { > > > - bool cancellable = fiber_set_cancellable(false); > > > - fiber_yield(); > > > - fiber_set_cancellable(cancellable); > > > + txn->signature = req->res; > > > + txn_complete(txn); > > > + fiber_set_txn(fiber(), NULL); > > > > Seems you do it twice ? > > If you mean the txn_complete call then no, in async write > we call the completion wich sets up TXN_IS_DONE flag, in > turn sync write (without wal) doesn't call the completion > and this flag is clear. > > Or you meant something different? I mean fiber_set_txn() is done twice, if I am not mistaken. -- Konstantin Osipov, Moscow, Russia