From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109]) (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 4F3D642EF5C for ; Thu, 2 Jul 2020 12:13:26 +0300 (MSK) References: <4a02e7dcec2cdddc5c45d5eca3f99d7f618af1d4.1593472477.git.v.shpilevoy@tarantool.org> From: Serge Petrenko Message-ID: Date: Thu, 2 Jul 2020 12:13:25 +0300 MIME-Version: 1.0 In-Reply-To: <4a02e7dcec2cdddc5c45d5eca3f99d7f618af1d4.1593472477.git.v.shpilevoy@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH v2 14/19] applier: remove writer_cond List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org 30.06.2020 02:15, Vladislav Shpilevoy пишет: > Writer condition variable was used by the writer fiber to be woken > up when it is time to send a heartbeat or an ACK. > > However it is not really needed, because writer fiber pointer is > always available in the same structure as writer_cond, and can be > used to call fiber_wakeup() directly. > > Note, fiber_cond_signal() is basically the same fiber_wakeup(). > > The patch is not just refactoring for nothing. It is a > prerequisite for #5100. In this issue it will be needed to wakeup > the applier's writer fiber directly on each WAL write from txn.c > module. So the writer_cond won't be available. The only usable > thing will be txn->fiber, which will be set to applier's writer. > > Part of #5100 > --- > src/box/applier.cc | 11 ++++------- > src/box/applier.h | 2 -- > 2 files changed, 4 insertions(+), 9 deletions(-) > > diff --git a/src/box/applier.cc b/src/box/applier.cc > index fbb452dc0..635a9849c 100644 > --- a/src/box/applier.cc > +++ b/src/box/applier.cc > @@ -155,11 +155,9 @@ applier_writer_f(va_list ap) > * replication_timeout seconds any more. > */ > if (applier->version_id >= version_id(1, 7, 7)) > - fiber_cond_wait_timeout(&applier->writer_cond, > - TIMEOUT_INFINITY); > + fiber_yield_timeout(TIMEOUT_INFINITY); > else > - fiber_cond_wait_timeout(&applier->writer_cond, > - replication_timeout); > + fiber_yield_timeout(replication_timeout); > /* > * A writer fiber is going to be awaken after a commit or > * a heartbeat message. So this is an appropriate place to > @@ -928,7 +926,7 @@ applier_on_commit(struct trigger *trigger, void *event) > { > (void) event; > struct applier *applier = (struct applier *)trigger->data; > - fiber_cond_signal(&applier->writer_cond); > + fiber_wakeup(applier->writer); > return 0; > } > > @@ -1093,7 +1091,7 @@ applier_subscribe(struct applier *applier) > */ > if (stailq_first_entry(&rows, struct applier_tx_row, > next)->row.lsn == 0) > - fiber_cond_signal(&applier->writer_cond); > + fiber_wakeup(applier->writer); > else if (applier_apply_tx(&rows) != 0) > diag_raise(); > > @@ -1289,7 +1287,6 @@ applier_new(const char *uri) > applier->last_row_time = ev_monotonic_now(loop()); > rlist_create(&applier->on_state); > fiber_cond_create(&applier->resume_cond); > - fiber_cond_create(&applier->writer_cond); > diag_create(&applier->diag); > > return applier; > diff --git a/src/box/applier.h b/src/box/applier.h > index c9fdc2955..4f8bee84e 100644 > --- a/src/box/applier.h > +++ b/src/box/applier.h > @@ -78,8 +78,6 @@ struct applier { > struct fiber *reader; > /** Background fiber to reply with vclock */ > struct fiber *writer; > - /** Writer cond. */ > - struct fiber_cond writer_cond; > /** Finite-state machine */ > enum applier_state state; > /** Local time of this replica when the last row has been received */ Thanks for the patch! LGTM. -- Serge Petrenko