[tarantool-patches] Re: [PATCH 02/12] vinyl: factor out vy_history_apply from vy_point_lookup_apply_history

[Vladislav Shpilevoy]

Hello. I found one moment, that can be checked.

On 15/04/2018 22:55, Vladimir Davydov wrote:
> Apart from applying a key history, vy_point_lookup_apply_history also
> adds the resultant tuple to the cache and updates LSM stats. Let's
> factor out history manipulation into a separate function and put
> everything else in vy_point_lookup so that we can make vy_history an
> independent entity.
> ---
>   src/box/vy_point_lookup.c | 41 ++++++++++++++++++-----------------------
>   1 file changed, 18 insertions(+), 23 deletions(-)
> 
> diff --git a/src/box/vy_point_lookup.c b/src/box/vy_point_lookup.c
> index 32048654..5d3076d9 100644
> --- a/src/box/vy_point_lookup.c
> +++ b/src/box/vy_point_lookup.c
> @@ -440,14 +427,22 @@ vy_point_lookup_apply_history(struct vy_lsm *lsm,
>   
>   done:
>   	if (rc == 0) {
> -		rc = vy_point_lookup_apply_history(lsm, rv, key,
> -						   &history, ret);
> +		int upserts_applied;
> +		rc = vy_history_apply(&history, lsm->cmp_def, lsm->mem_format,
> +				      &upserts_applied, ret);
> +		lsm->stat.upsert.applied += upserts_applied;
>   	}
>   	vy_history_cleanup(&history, region_svp);
>   
>   	if (rc != 0)
>   		return -1;
>   
> +	if (*ret != NULL) {
> +		vy_stmt_counter_acct_tuple(&lsm->stat.get, *ret);
> +		if ((*rv)->vlsn == INT64_MAX)
> +			vy_cache_add(&lsm->cache, *ret, NULL, key, ITER_EQ);

In the previous history applier vy_cache_add was called always, even if *ret is
not found (== NULL). But now it is called only when it is not NULL.

Maybe it is ok, because vy_cached_add does nothing on [NULL, NULL] chain. I wrote
it just for record.