From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 BE153445320 for ; Mon, 27 Jul 2020 23:41:18 +0300 (MSK) References: <20200727140650.447750-1-gorcunov@gmail.com> <20200727140650.447750-6-gorcunov@gmail.com> From: Vladislav Shpilevoy Message-ID: <67376e86-126e-f6c7-1684-c69980e4860b@tarantool.org> Date: Mon, 27 Jul 2020 22:41:17 +0200 MIME-Version: 1.0 In-Reply-To: <20200727140650.447750-6-gorcunov@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v3 5/6] qsync: implement direct write of CONFIRM/ROLLBACK into a journal List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov , tml Thanks for the patch! > diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c > index a74bfe244..d8cf6a6f6 100644 > --- a/src/box/txn_limbo.c > +++ b/src/box/txn_limbo.c > @@ -237,62 +240,73 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry) > return 0; > } > > +/** > + * A callback for synchronous write: txn_limbo_write fiber > + * waiting to proceed once a record is written to WAL. > + */ > +static void > +txn_limbo_write_cb(struct journal_entry *entry) > +{ > + assert(entry->complete_data != NULL); > + fiber_wakeup(entry->complete_data); > +} > + > +/** > + * Write CONFIRM or ROLLBACK message to a journal directly > + * without involving transaction engine because using txn > + * engine is far from being cheap while we only need to > + * write a small journal entry. > + */ > static int > -txn_limbo_write_confirm_rollback(struct txn_limbo *limbo, int64_t lsn, > - bool is_confirm) > +txn_limbo_write(uint32_t replica_id, int64_t lsn, int type) > { > + assert(replica_id != REPLICA_ID_NIL); > + assert(type == IPROTO_CONFIRM || type == IPROTO_ROLLBACK); > assert(lsn > 0); > > + /* > + * When allocated statically some compilers (such as > + * clang + asan) requires the journal_entry::rows to > + * be last in a container structure. So it it simplier > + * just to create a cummulative buffer. > + */ > + char buf[sizeof(struct journal_entry) + > + sizeof(struct xrow_header *)]; > + > + struct synchro_body_bin body_bin; > struct xrow_header row; > - struct request request = { > - .header = &row, > - }; > > - struct txn *txn = txn_begin(); > - if (txn == NULL) > - return -1; > + struct journal_entry *entry = (struct journal_entry *)buf; > + entry->rows[0] = &row; > > - int res = 0; > - if (is_confirm) { > - res = xrow_encode_confirm(&row, &txn->region, > - limbo->instance_id, lsn); > - } else { > - /* > - * This LSN is the first to be rolled back, so > - * the last "safe" lsn is lsn - 1. > - */ > - res = xrow_encode_rollback(&row, &txn->region, > - limbo->instance_id, lsn); > + xrow_encode_synchro(&row, &body_bin, replica_id, lsn, type); > + > + journal_entry_create(entry, 1, xrow_approx_len(&row), > + txn_limbo_write_cb, fiber()); > + > + if (journal_write(entry) != 0) { > + diag_set(ClientError, ER_WAL_IO); > + diag_log(); > + return -1; > } > - if (res == -1) > - goto rollback; > - /* > - * This is not really a transaction. It just uses txn API > - * to put the data into WAL. And obviously it should not > - * go to the limbo and block on the very same sync > - * transaction which it tries to confirm now. > - */ > - txn_set_flag(txn, TXN_FORCE_ASYNC); > > - if (txn_begin_stmt(txn, NULL) != 0) > - goto rollback; > - if (txn_commit_stmt(txn, &request) != 0) > - goto rollback; > + if (entry->res < 0) { I suggest simply add '|| entry->res < 0' to the check above. To make a single error handling point. > + diag_set(ClientError, ER_WAL_IO); > + diag_log(); > + return -1; > + } >