From 317166540f3c90058680aca9a6fc70599cf7e590 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 16 Jan 2019 11:22:26 +0200 Subject: [PATCH 1/3] MXS-2266: Close prepared statements with internal ID The ID used to store the prepared statements uses the internal ID and using the external ID caused unwanted memory use and a false warning. --- server/core/queryclassifier.cc | 14 ++++++++++++-- .../routing/readwritesplit/rwsplit_route_stmt.cc | 1 + 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/server/core/queryclassifier.cc b/server/core/queryclassifier.cc index 802a2fe5f..18d87d929 100644 --- a/server/core/queryclassifier.cc +++ b/server/core/queryclassifier.cc @@ -390,8 +390,18 @@ uint32_t QueryClassifier::ps_get_type(std::string id) const void QueryClassifier::ps_erase(GWBUF* buffer) { - m_ps_handles.erase(qc_mysql_extract_ps_id(buffer)); - m_sPs_manager->erase(buffer); + if (qc_mysql_is_ps_command(mxs_mysql_get_command(buffer))) + { + // Erase the type of the statement stored with the internal ID + m_sPs_manager->erase(ps_id_internal_get(buffer)); + // ... and then erase the external to internal ID mapping + m_ps_handles.erase(qc_mysql_extract_ps_id(buffer)); + } + else + { + // Not a PS command, we don't need the ID mapping + m_sPs_manager->erase(buffer); + } } bool QueryClassifier::query_type_is_read_only(uint32_t qtype) const diff --git a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc index 65cf6092f..56fc1c2fb 100644 --- a/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc +++ b/server/modules/routing/readwritesplit/rwsplit_route_stmt.cc @@ -451,6 +451,7 @@ bool RWSplitSession::route_session_write(GWBUF* querybuf, uint8_t command, uint3 } else if (qc_query_is_type(type, QUERY_TYPE_DEALLOC_PREPARE)) { + mxb_assert(!mxs_mysql_is_ps_command(m_qc.current_route_info().command())); m_qc.ps_erase(querybuf); } From 68d33a3ae4b5c45d3ccda7b90b9b6d1908f74482 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 16 Jan 2019 11:30:52 +0200 Subject: [PATCH 2/3] MXS-2266: Add test case Extended the existing tests to cover MXS-2266. --- maxscale-system-test/CMakeLists.txt | 2 +- maxscale-system-test/binary_ps.cpp | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/maxscale-system-test/CMakeLists.txt b/maxscale-system-test/CMakeLists.txt index da04de531..cfce1f6e3 100644 --- a/maxscale-system-test/CMakeLists.txt +++ b/maxscale-system-test/CMakeLists.txt @@ -455,7 +455,7 @@ add_test_executable(mxs359_read_only.cpp mxs359_read_only mxs359_read_only LABEL # Test master_failure_mode=error_on_write and master replacement add_test_executable(mxs359_error_on_write.cpp mxs359_error_on_write mxs359_error_on_write LABELS readwritesplit REPL_BACKEND) -# Binary protocol prepared statement tests +# Binary protocol prepared statement tests (also tests MXS-2266) add_test_executable(binary_ps.cpp binary_ps replication LABELS readwritesplit LIGHT REPL_BACKEND) add_test_executable(binary_ps_cursor.cpp binary_ps_cursor replication LABELS readwritesplit LIGHT REPL_BACKEND) diff --git a/maxscale-system-test/binary_ps.cpp b/maxscale-system-test/binary_ps.cpp index 137f903f9..bd513263e 100644 --- a/maxscale-system-test/binary_ps.cpp +++ b/maxscale-system-test/binary_ps.cpp @@ -46,6 +46,7 @@ int main(int argc, char** argv) test.add_result(strcmp(buffer, server_id[0]), "Expected server_id '%s', got '%s'", server_id[0], buffer); mysql_stmt_close(stmt); + stmt = mysql_stmt_init(test.maxscales->conn_rwsplit[0]); // Execute read, should return a slave server ID @@ -66,5 +67,8 @@ int main(int argc, char** argv) test.maxscales->close_maxscale_connections(0); + // MXS-2266: COM_STMT_CLOSE causes a warning to be logged + test.log_excludes(0, "Closing unknown prepared statement"); + return test.global_result; } From 519cd7d889225c2de5e8dec9ffdfa5e2902040f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Wed, 16 Jan 2019 16:00:32 +0200 Subject: [PATCH 3/3] Don't call python3 directly Calling it directly from the script adds a dependency on it and we don't what that. --- script/maxscale_generate_support_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/maxscale_generate_support_info.py b/script/maxscale_generate_support_info.py index a99ca1523..3eb73d22c 100755 --- a/script/maxscale_generate_support_info.py +++ b/script/maxscale_generate_support_info.py @@ -1,4 +1,4 @@ -#!/usr/bin/python3 +#!/usr/bin/env python3 # Copyright (c) 2019 MariaDB Corporation Ab #