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