[Tarantool-patches] [PATCH] box: sql prepare and execute statistics should be reflected in box.stat()
Maria Khaydich
maria.khaydich at tarantool.org
Wed Feb 19 19:37:33 MSK 2020
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
---
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
src/box/execute.c | 4 ++++
test/sql/iproto.result | 28 ++++++++++++++++++++++++++++
test/sql/iproto.test.lua | 12 ++++++++++++
3 files changed, 44 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..b4a05e4f8 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -788,6 +788,34 @@ 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]
+...
+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})
+
+box.stat().PREPARE.total == p + 1
+box.stat().EXECUTE.total == e + 1
+
-- Cleanup xlog
box.snapshot()
--
2.24.0
>Пятница, 14 февраля 2020, 0:44 +03:00 от Vladislav Shpilevoy <v.shpilevoy at tarantool.org>:
>
>Hi! Thanks for the patch!
>
>On 10/02/2020 19:35, Maria Khaydich wrote:
>> 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 < https://github.com/tarantool/tarantool/issues/4756Branch:https://github.com/tarantool/tarantool/compare/eljashm/gh-4756-box-stat-execute-and-prepare-not-updated >
>
>Something is broken with the links. When I click on them, all the links are
>pasted into the address string of my browser concatenated. And this is what
>I see in the source:
>
>https://github.com/tarantool/tarantool/compare/eljashm/gh-4756-box-stat-execute-and-prepare-not-updated < https://github.com/tarantool/tarantool/issues/4756Branch:https://github.com/tarantool/tarantool/compare/eljashm/gh-4756-box-stat-execute-and-prepare-not-updated >
>
>So everything is a one huge line. This does not happen with other emails
>and links.
>
>See 2 comments below.
>
>> src/box/execute.c | 4 ++++
>> test/box-tap/cfg.test.lua | 16 +++++++++++++---
>> 2 files changed, 17 insertions(+), 3 deletions(-)
>> diff --git a/src/box/execute.c b/src/box/execute.c
>> index dc8dce81c..e775055b4 100644
>> --- a/src/box/execute.c
>> +++ b/src/box/execute.c
>> @@ -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_PREPARE, 1);
>
>1. Nikita is right, this is called when IPROTO_EXECUTE arrives. You can
>see that in iproto.cc. This is a separate question, why do we collect
>IPROTO_* statistics out of iproto. For now lets just use IPROTO_EXECUTE
>here.
>
>> 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/box-tap/cfg.test.lua b/test/box-tap/cfg.test.lua
>> index d529447bb..d367aab07 100755
>> --- a/test/box-tap/cfg.test.lua
>> +++ b/test/box-tap/cfg.test.lua
>> @@ -6,7 +6,7 @@ local socket = require('socket')
>> local fio = require('fio')
>> local uuid = require('uuid')
>> local msgpack = require('msgpack')
>> -test:plan(104)
>> +test:plan(106)
>
>2. Here Nikita is also right. This file is for box.cfg() function
>tests. For iproto statistics, indeed, sql/iproto.test.lua would
>fit well.
--
Maria Khaydich
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200219/5fdd5cde/attachment.html>
More information about the Tarantool-patches
mailing list