From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 13 Jun 2019 17:17:02 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] [PATCH v3 07/14] wal: remove fiber from a journal_entry structure Message-ID: <20190613141702.rd2el6drnu7keku5@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: To: Georgy Kirichenko Cc: tarantool-patches@freelists.org List-ID: On Sun, Jun 09, 2019 at 11:44:36PM +0300, Georgy Kirichenko wrote: > Use a fiber_cond to signal a condition and wake up a waiting fiber. > This relaxes friction between fiber and transaction life cycles. I don't see how it relaxes anything ;-) Please be more specific when writing comments. > > Prerequisites: #1254 > --- > src/box/box.cc | 4 +++- > src/box/journal.c | 7 ++++--- > src/box/journal.h | 9 +++++++-- > src/box/wal.c | 29 +++++++++-------------------- > 4 files changed, 23 insertions(+), 26 deletions(-) > > diff --git a/src/box/wal.c b/src/box/wal.c > index 0ea15a432..5951817d0 100644 > --- a/src/box/wal.c > +++ b/src/box/wal.c > @@ -1172,15 +1166,10 @@ wal_write(struct journal *journal, struct journal_entry *entry) > batch->approx_len += entry->approx_len; > writer->wal_pipe.n_input += entry->n_rows * XROW_IOVMAX; > cpipe_flush_input(&writer->wal_pipe); > - /** > - * It's not safe to spuriously wakeup this fiber > - * since in that case it will ignore a possible > - * error from WAL writer and not roll back the > - * transaction. > - */ > - bool cancellable = fiber_set_cancellable(false); > - fiber_yield(); /* Request was inserted. */ > - fiber_set_cancellable(cancellable); > + > + while (!entry->done) > + fiber_cond_wait(&entry->done_cond); > + Using a fiber_cond to wake up a single fiber is an overkill. You could as well do while (!entry->done) fiber_yield_timeout(TIMEOUT_INFINITY); Anyway, I have my reservations re how you handle WAL writer wakeups. Please see my comments to the final patch.