Re: [Tarantool-patches] [PATCH] box: sql prepare and execute statistics should be reflected in box.stat()

["Maria Khaydich" <maria.khaydich@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 5536 bytes --]


> Please send smth like 'hotfix' for the last commit, I guess it is too late to force push to master.
 
If I understood you correctly:
----------------------------------------------------------------------
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.

There was a bug in previous commit resulting in collecting statistics on
box.execute twice in some cases.
 
Closes #4756
---
Issue:
https://github.com/tarantool/tarantool/issues/4756
Branch:
https://github.com/tarantool/tarantool/tree/eljashm/gh-4756-hotfix-box-stat-execute-prepare  

 src/box/execute.c        |  1 -
 test/sql/iproto.result   | 16 ++++++++--------
 test/sql/iproto.test.lua |  5 +++--
 3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/box/execute.c b/src/box/execute.c
index 3daa09205..24f8529ec 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -735,7 +735,6 @@ 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 a391307d1..0e046577d 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -800,22 +800,22 @@ e = box.stat().EXECUTE.total
 s = box.prepare([[ SELECT ?; ]])
 ---
 ...
-s:execute({42})
+res, err = box.unprepare(s)
 ---
-- metadata:
-  - name: '?'
-    type: integer
-  rows:
-  - [42]
 ...
-res, err = box.unprepare(s)
+box.execute('create table test (id integer primary key, a integer)')
 ---
+- row_count: 1
+...
+box.execute('DROP TABLE test')
+---
+- row_count: 1
 ...
 assert(box.stat().PREPARE.total == p + 1)
 ---
 - true
 ...
-assert(box.stat().EXECUTE.total == e + 1)
+assert(box.stat().EXECUTE.total == e + 2)
 ---
 - true
 ...
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 9eac91d2c..ec1f37035 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -249,11 +249,12 @@ p = box.stat().PREPARE.total
 e = box.stat().EXECUTE.total
 
 s = box.prepare([[ SELECT ?; ]])
-s:execute({42})
 res, err = box.unprepare(s)
+box.execute('create table test (id integer primary key, a integer)')
+box.execute('DROP TABLE test')
 
 assert(box.stat().PREPARE.total == p + 1)
-assert(box.stat().EXECUTE.total == e + 1)
+assert(box.stat().EXECUTE.total == e + 2)
 
 -- Cleanup xlog
 box.snapshot()
-- 
2.24.0
 
----------------------------------------------------------------------
Also that was delegated.
>  Kirill, it also should be backported to 2.2 skipping part related to  'prepare' feature.
 
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.
 
Version for 2.2 only includes box.execute.
 
Closes #4756
---
Branch:
https://github.com/tarantool/tarantool/tree/eljashm/gh-4756-box-stat-execute-prepare-2.2-version  
 
 src/box/execute.c        |  2 ++
 test/sql/iproto.result   | 18 ++++++++++++++++++
 test/sql/iproto.test.lua |  9 +++++++++
 3 files changed, 29 insertions(+)
diff --git a/src/box/execute.c b/src/box/execute.c
index 68e94e442..cf4d8afe6 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -45,6 +45,7 @@
 #include "tuple.h"
 #include "sql/vdbe.h"
 #include "box/lua/execute.h"
+#include "rmean.h"
 
 const char *sql_info_key_strs[] = {
     "row_count",
@@ -417,6 +418,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) {
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index f43e87117..8a9bfa405 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -820,6 +820,24 @@ box.schema.user.revoke('guest', 'create', 'space')
 space = nil
 ---
 ...
+--
+-- gh-4756: EXECUTE statistics should be present in box.stat()
+--
+e = box.stat().EXECUTE.total
+---
+...
+box.execute('create table test3 (id int primary key, a NUMBER, b text)')
+---
+- row_count: 1
+...
+box.execute('drop table test3')
+---
+- row_count: 1
+...
+assert(box.stat().EXECUTE.total == e + 2)
+---
+- true
+...
 -- Cleanup xlog
 box.snapshot()
 ---
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index dd60afe79..d0e979133 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -250,5 +250,14 @@ 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()
+--
+e = box.stat().EXECUTE.total
+box.execute('create table test3 (id int primary key, a NUMBER, b text)')
+box.execute('drop table test3')
+
+assert(box.stat().EXECUTE.total == e + 2)
+
 -- Cleanup xlog
 box.snapshot()
-- 
2.24.0
 
--
Maria Khaydich
 
 

[-- Attachment #2: Type: text/html, Size: 8563 bytes --]