From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 13 Jun 2019 17:24:52 +0300 From: Vladimir Davydov Subject: Re: [tarantool-patches] [PATCH v3 09/14] wal: a dedicated wal scheduling fiber Message-ID: <20190613142452.j63yqr4q53xszo4r@esperanza> References: <64f63cc1cea7a3c632561f36eb043cb386c4e472.1560112747.git.georgy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <64f63cc1cea7a3c632561f36eb043cb386c4e472.1560112747.git.georgy@tarantool.org> To: Georgy Kirichenko Cc: tarantool-patches@freelists.org List-ID: On Sun, Jun 09, 2019 at 11:44:38PM +0300, Georgy Kirichenko wrote: > In order to implement asynchronous transaction we need to run a > transaction completion handler but tx_prio is not able to yield. > > Prerequisites: #1254 > --- > src/box/wal.c | 48 +++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 41 insertions(+), 7 deletions(-) > > diff --git a/src/box/wal.c b/src/box/wal.c > index 86d021896..e868a8e71 100644 > --- a/src/box/wal.c > +++ b/src/box/wal.c > @@ -100,6 +100,12 @@ struct wal_writer > struct cpipe wal_pipe; > /** A memory pool for messages. */ > struct mempool msg_pool; > + /** A queue to schedule journal entry completions. */ > + struct stailq schedule_queue; > + /** True if writer is in rollback state. */ > + bool is_in_rollback; > + /** A condition to signal about new schedule queue entries. */ > + struct fiber_cond schedule_cond; > /* ----------------- wal ------------------- */ > /** A setting from instance configuration - rows_per_wal */ > int64_t wal_max_rows; > @@ -254,17 +260,36 @@ xlog_write_entry(struct xlog *l, struct journal_entry *entry) > return xlog_tx_commit(l); > } > > +/* > + * Tx schedule fiber function. > + */ > +static int > +tx_schedule_f(va_list ap) > +{ > + struct wal_writer *writer = va_arg(ap, struct wal_writer *); > + while (!fiber_is_cancelled()) { > + while (!stailq_empty(&writer->schedule_queue)) { > + struct journal_entry *req = > + stailq_shift_entry(&writer->schedule_queue, > + struct journal_entry, fifo); > + req->done = true; > + fiber_cond_broadcast(&req->done_cond); > + } > + writer->is_in_rollback = false; > + fiber_cond_wait(&writer->schedule_cond); > + } > + return 0; > +} > + > /** > - * Signal done condition. > + * Attach requests to a scheduling queue. > */ > static void > tx_schedule_queue(struct stailq *queue) > { > - struct journal_entry *req; > - stailq_foreach_entry(req, queue, fifo) { > - req->done = true; > - fiber_cond_broadcast(&req->done_cond); > - } > + struct wal_writer *writer = &wal_writer_singleton; > + stailq_concat(&writer->schedule_queue, queue); > + fiber_cond_signal(&writer->schedule_cond); This adds an extra ctxsw to the relatively hot WAL writer wakeup path so we can't commit it. I assume it's a temporary hack you added so that you can submit this patches for review. I understand why you're doing this, but the right way would be removing yields from on_commit triggers. We need to make vy_log_write yield-free.