[Tarantool-patches] [PATCH] sql: account prepared stmt cache size right after entry removal

[Tarantool-patches] [PATCH] sql: account prepared stmt cache size right after entry removal

[Nikita Pettik]

SQL prepared statement cache is implemented as two data structures: hash
table <stmt_id : pointer-to-metadata> and GC queue. The latter is
required to avoid workload spikes on session's disconnect: instead of
cleaning up memory for all session-local prepared statements, prepared
statements to be deleted are moved to GC queue. When memory limit for PS
is reached, all elements from queue are removed at once. If statement
traps to the GC queue it is assumed to be already dead. Accidentally,
change of occupied by PS cache takes place only after GC queue clean-up,
so correct size of PS cache is displayed only after GC cycles. Let's fix
this and account PS cache size change right after entry removal (i.e. at
the moment PS gets into GC queue).
---
Branch: https://github.com/tarantool/tarantool/tree/np/fix-prepared-stmt-cache-size-calc

 src/box/sql_stmt_cache.c   | 2 +-
 test/sql/prepared.result   | 9 +++++++++
 test/sql/prepared.test.lua | 3 +++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/box/sql_stmt_cache.c b/src/box/sql_stmt_cache.c
index 53966dbff..5e2fb3def 100644
--- a/src/box/sql_stmt_cache.c
+++ b/src/box/sql_stmt_cache.c
@@ -92,7 +92,6 @@ sql_stmt_cache_delete(struct stmt_cache_entry *entry)
 	if (sql_stmt_cache.last_found == entry)
 		sql_stmt_cache.last_found = NULL;
 	rlist_del(&entry->link);
-	sql_stmt_cache.mem_used -= sql_cache_entry_sizeof(entry->stmt);
 	sql_cache_entry_delete(entry);
 }
 
@@ -175,6 +174,7 @@ sql_stmt_cache_entry_unref(struct stmt_cache_entry *entry)
 		assert(i != mh_end(cache->hash));
 		mh_i32ptr_del(cache->hash, i, NULL);
 		rlist_add(&sql_stmt_cache.gc_queue, &entry->link);
+		sql_stmt_cache.mem_used -= sql_cache_entry_sizeof(entry->stmt);
 		if (sql_stmt_cache.last_found == entry)
 			sql_stmt_cache.last_found = NULL;
 	}
diff --git a/test/sql/prepared.result b/test/sql/prepared.result
index 18253283f..71ab0bb57 100644
--- a/test/sql/prepared.result
+++ b/test/sql/prepared.result
@@ -189,6 +189,15 @@ unprepare(s.stmt_id)
  | - null
  | ...
 
+assert(box.info.sql().cache.stmt_count == 0)
+ | ---
+ | - true
+ | ...
+assert(box.info.sql().cache.size == 0)
+ | ---
+ | - true
+ | ...
+
 -- Test preparation of different types of queries.
 -- Let's start from DDL. It doesn't make much sense since
 -- any prepared DDL statement can be executed once, but
diff --git a/test/sql/prepared.test.lua b/test/sql/prepared.test.lua
index c1203712e..1e3f02b09 100644
--- a/test/sql/prepared.test.lua
+++ b/test/sql/prepared.test.lua
@@ -78,6 +78,9 @@ end;
 test_run:cmd("setopt delimiter ''");
 unprepare(s.stmt_id)
 
+assert(box.info.sql().cache.stmt_count == 0)
+assert(box.info.sql().cache.size == 0)
+
 -- Test preparation of different types of queries.
 -- Let's start from DDL. It doesn't make much sense since
 -- any prepared DDL statement can be executed once, but
-- 
2.15.1

Re: [Tarantool-patches] [PATCH] sql: account prepared stmt cache size right after entry removal

[Nikita Pettik]

On 13 Jan 12:50, Nikita Pettik wrote:
> SQL prepared statement cache is implemented as two data structures: hash
> table <stmt_id : pointer-to-metadata> and GC queue. The latter is
> required to avoid workload spikes on session's disconnect: instead of
> cleaning up memory for all session-local prepared statements, prepared
> statements to be deleted are moved to GC queue. When memory limit for PS
> is reached, all elements from queue are removed at once. If statement
> traps to the GC queue it is assumed to be already dead. Accidentally,
> change of occupied by PS cache takes place only after GC queue clean-up,
> so correct size of PS cache is displayed only after GC cycles. Let's fix
> this and account PS cache size change right after entry removal (i.e. at
> the moment PS gets into GC queue).
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/fix-prepared-stmt-cache-size-calc
> 
>  src/box/sql_stmt_cache.c   | 2 +-
>  test/sql/prepared.result   | 9 +++++++++
>  test/sql/prepared.test.lua | 3 +++
>  3 files changed, 13 insertions(+), 1 deletion(-)

Pushed to master and 2.3 as obvious. Also it resolves accidental test
failure (sql/prepared.test.lua) on Travis/GitLab.