From: "Maria Khaydich" <maria.khaydich@tarantool.org> To: "Nikita Pettik" <korablev@tarantool.org> Cc: tarantool-patches <tarantool-patches@dev.tarantool.org>, "Vladislav Shpilevoy" <v.shpilevoy@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH] box: sql prepare and execute statistics should be reflected in box.stat() Date: Tue, 25 Feb 2020 14:08:11 +0300 [thread overview] Message-ID: <1582628891.457674764@f461.i.mail.ru> (raw) In-Reply-To: <20200219171602.GB40100@tarantool.org> [-- Attachment #1: Type: text/plain, Size: 6241 bytes --] Thanks for the review, all 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 --- Issue: https://github.com/tarantool/tarantool/issues/4756 Branch: https://github.com/tarantool/tarantool/compare/eljashm/gh-4756-box-stat-execute-and-prepare-not-updated @ChangeLog - Fixed box.stat behavior - now it collects statistics on prepare and execute methods as it should. gh-4756 src/box/execute.c | 4 ++++ test/sql/iproto.result | 31 +++++++++++++++++++++++++++++++ test/sql/iproto.test.lua | 13 +++++++++++++ 3 files changed, 48 insertions(+) diff --git a/src/box/execute.c b/src/box/execute.c index dc8dce81c..3daa09205 100644 --- a/src/box/execute.c +++ b/src/box/execute.c @@ -48,6 +48,7 @@ #include "box/lua/execute.h" #include "box/sql_stmt_cache.h" #include "session.h" +#include "rmean.h" const char *sql_info_key_strs[] = { "row_count", @@ -608,6 +609,7 @@ sql_prepare(const char *sql, int len, struct port *port) { uint32_t stmt_id = sql_stmt_calculate_id(sql, len); struct sql_stmt *stmt = sql_stmt_cache_find(stmt_id); + rmean_collect(rmean_box, IPROTO_PREPARE, 1); if (stmt == NULL) { if (sql_stmt_compile(sql, len, NULL, &stmt, NULL) != 0) return -1; @@ -669,6 +671,7 @@ static inline int sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region) { int rc, column_count = sql_column_count(stmt); + rmean_collect(rmean_box, IPROTO_EXECUTE, 1); if (column_count > 0) { /* Either ROW or DONE or ERROR. */ while ((rc = sql_step(stmt)) == SQL_ROW) { @@ -732,6 +735,7 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind, if (sql_stmt_compile(sql, len, NULL, &stmt, NULL) != 0) return -1; assert(stmt != NULL); + rmean_collect(rmean_box, IPROTO_EXECUTE, 1); enum sql_serialization_format format = sql_column_count(stmt) > 0 ? DQL_EXECUTE : DML_EXECUTE; port_sql_create(port, stmt, format, true); diff --git a/test/sql/iproto.result b/test/sql/iproto.result index 7df11b0bf..a391307d1 100644 --- a/test/sql/iproto.result +++ b/test/sql/iproto.result @@ -788,6 +788,37 @@ 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}) +--- +- metadata: + - name: '?' + type: integer + rows: + - [42] +... +res, err = box.unprepare(s) +--- +... +assert(box.stat().PREPARE.total == p + 1) +--- +- true +... +assert(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..9eac91d2c 100644 --- a/test/sql/iproto.test.lua +++ b/test/sql/iproto.test.lua @@ -242,5 +242,18 @@ 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}) +res, err = box.unprepare(s) + +assert(box.stat().PREPARE.total == p + 1) +assert(box.stat().EXECUTE.total == e + 1) + -- Cleanup xlog box.snapshot() -- 2.24.0 >Среда, 19 февраля 2020, 20:16 +03:00 от Nikita Pettik <korablev@tarantool.org>: > >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) > -- Maria Khaydich [-- Attachment #2: Type: text/html, Size: 8572 bytes --]
next prev parent reply other threads:[~2020-02-25 11:08 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-10 18:35 Maria Khaydich 2020-02-10 23:06 ` Nikita Pettik 2020-02-13 21:44 ` Vladislav Shpilevoy 2020-02-19 16:37 ` Maria Khaydich 2020-02-19 17:16 ` Nikita Pettik 2020-02-25 11:08 ` Maria Khaydich [this message] 2020-02-25 13:02 ` Nikita Pettik 2020-02-25 20:29 ` Vladislav Shpilevoy 2020-02-26 15:50 ` Maria Khaydich 2020-02-27 0:08 ` Vladislav Shpilevoy 2020-02-27 13:21 ` Nikita Pettik 2020-03-03 16:42 ` Maria Khaydich 2020-03-03 22:39 ` Vladislav Shpilevoy 2020-03-06 11:34 ` Maria Khaydich 2020-03-06 14:32 ` Nikita Pettik 2020-03-06 15:09 ` Nikita Pettik 2020-03-04 13:47 ` Nikita Pettik 2020-02-25 22:26 ` Kirill Yukhin 2020-02-25 23:30 ` Nikita Pettik
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1582628891.457674764@f461.i.mail.ru \ --to=maria.khaydich@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] box: sql prepare and execute statistics should be reflected in box.stat()' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox