<HTML><BODY><div><div>Thanks for the review, all done:</div><hr><div><div>Calling prepare and execute did not update corresponding request statistics<br>in the box.stat table. This happened because methods that collect stats were<br>never called where they should have been.</div><div> </div><div>Closes #4756<br>---<br>Issue:<br><a href="https://github.com/tarantool/tarantool/issues/4756">https://github.com/tarantool/tarantool/issues/4756</a> </div><div>Branch:<br><a href="https://github.com/tarantool/tarantool/compare/eljashm/gh-4756-box-stat-execute-and-prepare-not-updated">https://github.com/tarantool/tarantool/compare/eljashm/gh-4756-box-stat-execute-and-prepare-not-updated</a> </div><div> </div><div>@ChangeLog<br>- Fixed box.stat behavior - now it collects statistics on<br> prepare and execute methods as it should.<br> gh-4756</div><div> </div><div> src/box/execute.c | 4 ++++<br> test/sql/iproto.result | 31 +++++++++++++++++++++++++++++++<br> test/sql/iproto.test.lua | 13 +++++++++++++<br> 3 files changed, 48 insertions(+)</div><div> </div><div>diff --git a/src/box/execute.c b/src/box/execute.c<br>index dc8dce81c..3daa09205 100644<br>--- a/src/box/execute.c<br>+++ b/src/box/execute.c<br>@@ -48,6 +48,7 @@<br> #include "box/lua/execute.h"<br> #include "box/sql_stmt_cache.h"<br> #include "session.h"<br>+#include "rmean.h"<br> <br> const char *sql_info_key_strs[] = {<br> "row_count",<br>@@ -608,6 +609,7 @@ sql_prepare(const char *sql, int len, struct port *port)<br> {<br> uint32_t stmt_id = sql_stmt_calculate_id(sql, len);<br> struct sql_stmt *stmt = sql_stmt_cache_find(stmt_id);<br>+ rmean_collect(rmean_box, IPROTO_PREPARE, 1);<br> if (stmt == NULL) {<br> if (sql_stmt_compile(sql, len, NULL, &stmt, NULL) != 0)<br> return -1;<br>@@ -669,6 +671,7 @@ static inline int<br> sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region)<br> {<br> int rc, column_count = sql_column_count(stmt);<br>+ rmean_collect(rmean_box, IPROTO_EXECUTE, 1);<br> if (column_count > 0) {<br> /* Either ROW or DONE or ERROR. */<br> while ((rc = sql_step(stmt)) == SQL_ROW) {<br>@@ -732,6 +735,7 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,<br> if (sql_stmt_compile(sql, len, NULL, &stmt, NULL) != 0)<br> return -1;<br> assert(stmt != NULL);<br>+ rmean_collect(rmean_box, IPROTO_EXECUTE, 1);<br> enum sql_serialization_format format = sql_column_count(stmt) > 0 ?<br> DQL_EXECUTE : DML_EXECUTE;<br> port_sql_create(port, stmt, format, true);</div><div><br>diff --git a/test/sql/iproto.result b/test/sql/iproto.result<br>index 7df11b0bf..a391307d1 100644<br>--- a/test/sql/iproto.result<br>+++ b/test/sql/iproto.result<br>@@ -788,6 +788,37 @@ box.schema.user.revoke('guest', 'create', 'space')<br> space = nil<br> ---<br> ...<br>+--<br>+-- gh-4756: PREPARE and EXECUTE statistics should be present in box.stat()<br>+--<br>+p = box.stat().PREPARE.total<br>+---<br>+...<br>+e = box.stat().EXECUTE.total<br>+---<br>+...<br>+s = box.prepare([[ SELECT ?; ]])<br>+---<br>+...<br>+s:execute({42})<br>+---<br>+- metadata:<br>+ - name: '?'<br>+ type: integer<br>+ rows:<br>+ - [42]<br>+...<br>+res, err = box.unprepare(s)<br>+---<br>+...<br>+assert(box.stat().PREPARE.total == p + 1)<br>+---<br>+- true<br>+...<br>+assert(box.stat().EXECUTE.total == e + 1)<br>+---<br>+- true<br>+...<br> -- Cleanup xlog<br> box.snapshot()<br> ---</div><div><br>diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>index 4019fb7a4..9eac91d2c 100644<br>--- a/test/sql/iproto.test.lua<br>+++ b/test/sql/iproto.test.lua<br>@@ -242,5 +242,18 @@ box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br> box.schema.user.revoke('guest', 'create', 'space')<br> space = nil<br> <br>+--<br>+-- gh-4756: PREPARE and EXECUTE statistics should be present in box.stat()<br>+--<br>+p = box.stat().PREPARE.total<br>+e = box.stat().EXECUTE.total<br>+<br>+s = box.prepare([[ SELECT ?; ]])<br>+s:execute({42})<br>+res, err = box.unprepare(s)<br>+<br>+assert(box.stat().PREPARE.total == p + 1)<br>+assert(box.stat().EXECUTE.total == e + 1)<br>+<br> -- Cleanup xlog<br> box.snapshot()<br>-- <br>2.24.0</div></div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Среда, 19 февраля 2020, 20:16 +03:00 от Nikita Pettik <korablev@tarantool.org>:<br> <div id=""><div class="js-helper js-readmsg-msg"><style type="text/css"></style><div><div id="style_15821325631921819214_BODY">On 19 Feb 19:37, Maria Khaydich wrote:<br>><br>> Thank you for the review!<br>> Proposed fixes are done.<br>> <br>> ----------------------------------------------------------------------<br>> Calling prepare and execute did not update corresponding request statistics<br>> in the box.stat table. This happened because methods that collect stats were<br>> never called where they should have been.<br>> <br>> Closes #4756<br><br>Please also add @ChangeLog according to our new sop guide. For instance,<br>see: <a href="https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014301.html" target="_blank">https://lists.tarantool.org/pipermail/tarantool-patches/2020-February/014301.html</a><br> <br>> space = nil<br>> ---<br>> ...<br>> +--<br>> +-- gh-4756: PREPARE and EXECUTE statistics should be present in box.stat()<br>> +--<br>> +p = box.stat().PREPARE.total<br>> +---<br>> +...<br>> +e = box.stat().EXECUTE.total<br>> +---<br>> +...<br>> +s = box.prepare([[ SELECT ?; ]])<br>> +---<br>> +...<br>> +s:execute({42})<br>> +---<br>> +- metadata:<br>> + - name: '?'<br>> + type: integer<br>> + rows:<br>> + - [42]<br>> +...<br>> +box.stat().PREPARE.total == p + 1<br>> +---<br>> +- true<br>> +...<br>> +box.stat().EXECUTE.total == e + 1<br>> +---<br>> +- true<br>> +...<br>> -- Cleanup xlog<br>> box.snapshot()<br>> ---<br>> <br>> diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua<br>> index 4019fb7a4..1417aa32b 100644<br>> --- a/test/sql/iproto.test.lua<br>> +++ b/test/sql/iproto.test.lua<br>> @@ -242,5 +242,17 @@ box.schema.user.revoke('guest', 'read,write,execute', 'universe')<br>> box.schema.user.revoke('guest', 'create', 'space')<br>> space = nil<br>> <br>> +--<br>> +-- gh-4756: PREPARE and EXECUTE statistics should be present in box.stat()<br>> +--<br>> +p = box.stat().PREPARE.total<br>> +e = box.stat().EXECUTE.total<br>> +<br>> +s = box.prepare([[ SELECT ?; ]])<br>> +s:execute({42})<br><br>You also have to unprepare statement. Otherwise it gets stuck in<br>cache and sql/prepare.test.lua may fail since at the start of<br>execution it checks cahce size:<br><a href="https://travis-ci.org/tarantool/tarantool/jobs/652552114?utm_medium=notification&utm_source=github_status" target="_blank">https://travis-ci.org/tarantool/tarantool/jobs/652552114?utm_medium=notification&utm_source=github_status</a><br><br>It is sort of basic clean-up which is provided for any schema object<br>(spaces, triggers etc).<br><br>> +box.stat().PREPARE.total == p + 1<br>> +box.stat().EXECUTE.total == e + 1<br>> +<br><br>Nit: I'd better wrap these statements into asssertions:<br><br>assert(box.stat().PREPARE.total == p + 1)<br> </div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Maria Khaydich</div></div></div><div> </div></div></BODY></HTML>