From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id B8376469719 for ; Wed, 19 Feb 2020 20:16:03 +0300 (MSK) Date: Wed, 19 Feb 2020 20:16:02 +0300 From: Nikita Pettik Message-ID: <20200219171602.GB40100@tarantool.org> References: <1581359737.397395198@f221.i.mail.ru> <1582130253.193228635@f540.i.mail.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1582130253.193228635@f540.i.mail.ru> Subject: Re: [Tarantool-patches] [PATCH] box: sql prepare and execute statistics should be reflected in box.stat() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Maria Khaydich Cc: tarantool-patches , Vladislav Shpilevoy On 19 Feb 19:37, Maria Khaydich wrote: > > Thank you for the review! > Proposed fixes are done. >   > ---------------------------------------------------------------------- > Calling prepare and execute did not update corresponding request statistics > in the box.stat table. This happened because methods that collect stats were > never called where they should have been. >   > Closes #4756 Please also add @ChangeLog according to our new sop guide. For instance, see: https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014301.html >  space = nil >  --- >  ... > +-- > +-- gh-4756: PREPARE and EXECUTE statistics should be present in box.stat() > +-- > +p = box.stat().PREPARE.total > +--- > +... > +e = box.stat().EXECUTE.total > +--- > +... > +s = box.prepare([[ SELECT ?; ]]) > +--- > +... > +s:execute({42}) > +--- > +- metadata: > +  - name: '?' > +    type: integer > +  rows: > +  - [42] > +... > +box.stat().PREPARE.total == p + 1 > +--- > +- true > +... > +box.stat().EXECUTE.total == e + 1 > +--- > +- true > +... >  -- Cleanup xlog >  box.snapshot() >  --- >   > diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua > index 4019fb7a4..1417aa32b 100644 > --- a/test/sql/iproto.test.lua > +++ b/test/sql/iproto.test.lua > @@ -242,5 +242,17 @@ box.schema.user.revoke('guest', 'read,write,execute', 'universe') >  box.schema.user.revoke('guest', 'create', 'space') >  space = nil >   > +-- > +-- gh-4756: PREPARE and EXECUTE statistics should be present in box.stat() > +-- > +p = box.stat().PREPARE.total > +e = box.stat().EXECUTE.total > + > +s = box.prepare([[ SELECT ?; ]]) > +s:execute({42}) You also have to unprepare statement. Otherwise it gets stuck in cache and sql/prepare.test.lua may fail since at the start of execution it checks cahce size: https://travis-ci.org/tarantool/tarantool/jobs/652552114?utm_medium=notification&utm_source=github_status It is sort of basic clean-up which is provided for any schema object (spaces, triggers etc). > +box.stat().PREPARE.total == p + 1 > +box.stat().EXECUTE.total == e + 1 > + Nit: I'd better wrap these statements into asssertions: assert(box.stat().PREPARE.total == p + 1)