From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 5E06723F4B for ; Mon, 14 May 2018 14:19:39 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0OHwZBMduj-k for ; Mon, 14 May 2018 14:19:39 -0400 (EDT) Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 9EA4723EC8 for ; Mon, 14 May 2018 14:19:38 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH 02/12] vinyl: factor out vy_history_apply from vy_point_lookup_apply_history References: From: Vladislav Shpilevoy Message-ID: <86d2dca1-748b-afad-8e3a-5c8a9f34b214@tarantool.org> Date: Mon, 14 May 2018 21:19:36 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org, kostja@tarantool.org 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.