From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <vdavydov.dev@gmail.com>
Date: Thu, 13 Jun 2019 17:34:00 +0300
From: Vladimir Davydov <vdavydov.dev@gmail.com>
Subject: Re: [tarantool-patches] [PATCH v3 11/14] txn: introduce asynchronous
 txn commit
Message-ID: <20190613143400.uminobjjgnkeleat@esperanza>
References: <cover.1560112747.git.georgy@tarantool.org>
 <697f3313f6ad706a71c74ec259c0ea37d2702184.1560112747.git.georgy@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <697f3313f6ad706a71c74ec259c0ea37d2702184.1560112747.git.georgy@tarantool.org>
To: Georgy Kirichenko <georgy@tarantool.org>
Cc: tarantool-patches@freelists.org
List-ID: <tarantool-patches.dev.tarantool.org>

On Sun, Jun 09, 2019 at 11:44:40PM +0300, Georgy Kirichenko wrote:
> diff --git a/src/box/txn.c b/src/box/txn.c
> index a08652af1..815d635fe 100644
> --- a/src/box/txn.c
> +++ b/src/box/txn.c
> @@ -318,22 +319,77 @@ fail:
>  	return -1;
>  }
>  
> +/**
> + * Complete transaction processing.
> + */
> +static void
> +txn_complete(struct txn *txn)
> +{
> +	if (txn->signature < 0) {
> +		if (txn->engine)
> +			engine_rollback(txn->engine, txn);
> +		/* Rollback triggers must not throw. */
> +		fiber_set_txn(fiber(), txn);
> +		if (txn->has_triggers &&
> +		    trigger_run(&txn->on_rollback, txn) != 0) {
> +			diag_log();
> +			unreachable();
> +			panic("rollback trigger failed");
> +		}
> +		fiber_set_txn(fiber(), NULL);
> +
> +		return;
> +	}
> +	/*
> +	 * Engine can be NULL if transaction contains IPROTO_NOP
> +	 * statements only.
> +	 */
> +	if (txn->engine != NULL)
> +		engine_commit(txn->engine, txn);
> +	/*
> +	 * The transaction is in the binary log. No action below
> +	 * may throw. In case an error has happened, there is
> +	 * no other option but terminate.
> +	 */
> +	fiber_set_txn(fiber(), txn);
> +	if (txn->has_triggers &&
> +	    trigger_run(&txn->on_commit, txn) != 0) {
> +		diag_log();
> +		unreachable();
> +		panic("commit trigger failed");
> +	}
> +
> +	fiber_set_txn(fiber(), NULL);

I don't see why we need to have in_txn() in on_commit/rollback triggers.
Could you please point me?

> diff --git a/src/box/wal.c b/src/box/wal.c
> index e868a8e71..eff48b4fe 100644
> --- a/src/box/wal.c
> +++ b/src/box/wal.c
> @@ -272,6 +272,8 @@ tx_schedule_f(va_list ap)
>  			struct journal_entry *req =
>  				stailq_shift_entry(&writer->schedule_queue,
>  						   struct journal_entry, fifo);
> +			if (req->on_done_cb != NULL)
> +				req->on_done_cb(req, req->on_done_cb_data);
>  			req->done = true;
>  			fiber_cond_broadcast(&req->done_cond);

Why do we need cond if we have a callback? Can't we wake up the awaiting
fiber from the callback?