[tarantool-patches] Re: [PATCH 3/5] vinyl: encapsulate reader thread selection logic in a helper function

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev at gmail.com> [19/05/29 21:03]:
> Page reading code is intermixed with the reader thread selection in the
> same function, which makes it difficult to extend the former. So let's
> introduce a helper function encapsulating a call on behalf of a reader
> thread.
>  /**
> + * Execute a task on behalf of a reader thread.
> + * Calls free_cb on failure.
> + */
> +static int
> +vy_run_env_coio_call(struct vy_run_env *env, struct cbus_call_msg *msg,
> +		     cbus_call_f func, cbus_call_f free_cb, double timeout)
> +{
> +	/* Optimization: use blocking I/O during WAL recovery. */
> +	if (env->reader_pool == NULL) {
> +		if (func(msg) != 0)
> +			goto fail;
> +		return 0;
> +	}

this would be a great api if not for this optimization. Today not
involving free_cb on cbus_msg is harmless, but tomorrow may break
some logic. 

Please consider designing an API in a way that would work with
resources in the same way whether or not it is boot time or a coio
call.

The comment could also be extended, not reduced - e.g. explain why
we consider this an optimization, when exactly we perform reads
during recovery (for what statements, for example).


-- 
Konstantin Osipov, Moscow, Russia