Skip to content

Commit

Permalink
MDEV-20699 mysqldump of routines causes MariaDB to get killed by oom-…
Browse files Browse the repository at this point in the history
…killer

The reason for this behavior is that SP get cached, per connection.
The stored_program_cache is size of this cache, which amounts to 256
routines by default. A compiled stored procedure can easily be several
megabytes in size. Thus calling SHOW CREATE PROCEDURE for all stored
procedures, like mysqldump does, can require significant amount of memory.

Fixed by bypassing the cache for "SHOW CREATE". This should normally be
fine also perfomance-wise, as cache is meant to be used for repeated
execution, not repeated SHOW CREATEs.

Added a test to verify that CREATE PROCEDURE + SHOW CREATE PROCEURE do not
cache, i.e amount of allocated memory does not change.

Note, there is a change in existing behavior in an edge case :
If "SHOW CREATE PROCEDURE p1" called from p1, after p1 was altered, now
this will now return altered code. Previour behavior - relied on caching
and would return old code. The previous behavior might was not necessarily
correct.
  • Loading branch information
vaintroub committed Sep 27, 2021
1 parent d7aa81c commit 1f09941
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 21 deletions.
4 changes: 2 additions & 2 deletions mysql-test/main/sp-bugs.result
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ RETURN 0;
END latin1 latin1_swedish_ci latin1_swedish_ci
SHOW CREATE FUNCTION TESTF_bug11763507;
Function sql_mode Create Function character_set_client collation_connection Database Collation
testf_bug11763507 CREATE DEFINER=`root`@`localhost` FUNCTION `testf_bug11763507`() RETURNS int(11)
TESTF_bug11763507 CREATE DEFINER=`root`@`localhost` FUNCTION `TESTF_bug11763507`() RETURNS int(11)
BEGIN
RETURN 0;
END latin1 latin1_swedish_ci latin1_swedish_ci
Expand Down Expand Up @@ -212,7 +212,7 @@ SELECT "PROCEDURE testp_bug11763507";
END latin1 latin1_swedish_ci latin1_swedish_ci
SHOW CREATE PROCEDURE TESTP_bug11763507;
Procedure sql_mode Create Procedure character_set_client collation_connection Database Collation
testp_bug11763507 CREATE DEFINER=`root`@`localhost` PROCEDURE `testp_bug11763507`()
TESTP_bug11763507 CREATE DEFINER=`root`@`localhost` PROCEDURE `TESTP_bug11763507`()
BEGIN
SELECT "PROCEDURE testp_bug11763507";
END latin1 latin1_swedish_ci latin1_swedish_ci
Expand Down
8 changes: 1 addition & 7 deletions mysql-test/main/sp-lock.result
Original file line number Diff line number Diff line change
Expand Up @@ -703,9 +703,6 @@ connection default;
#
# SHOW CREATE PROCEDURE p1 called from p1, after p1 was altered
#
# We are just covering the existing behaviour with tests. The
# results are not necessarily correct."
#
CREATE PROCEDURE p1()
BEGIN
SELECT get_lock("test", 10);
Expand Down Expand Up @@ -736,10 +733,7 @@ get_lock("test", 10)
1
Procedure sql_mode Create Procedure character_set_client collation_connection Database Collation
p1 STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION CREATE DEFINER=`root`@`localhost` PROCEDURE `p1`()
BEGIN
SELECT get_lock("test", 10);
SHOW CREATE PROCEDURE p1;
END latin1 latin1_swedish_ci latin1_swedish_ci
BEGIN END latin1 latin1_swedish_ci latin1_swedish_ci
connection con3;
disconnect con3;
connection con2;
Expand Down
3 changes: 0 additions & 3 deletions mysql-test/main/sp-lock.test
Original file line number Diff line number Diff line change
Expand Up @@ -807,9 +807,6 @@ connection default;
--echo #
--echo # SHOW CREATE PROCEDURE p1 called from p1, after p1 was altered
--echo #
--echo # We are just covering the existing behaviour with tests. The
--echo # results are not necessarily correct."
--echo #

delimiter |;
CREATE PROCEDURE p1()
Expand Down
22 changes: 22 additions & 0 deletions mysql-test/main/sp.result
Original file line number Diff line number Diff line change
Expand Up @@ -8893,4 +8893,26 @@ ERROR 42000: You have an error in your SQL syntax; check the manual that corresp
BEGIN
RETURN '';
END' at line 2
SELECT VARIABLE_VALUE into @global_mem_used FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
SELECT VARIABLE_VALUE into @local_mem_used FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
CREATE PROCEDURE sp0() SELECT 1;
SHOW CREATE PROCEDURE sp0;
Procedure sql_mode Create Procedure character_set_client collation_connection Database Collation
sp0 STRICT_ALL_TABLES CREATE DEFINER=`root`@`localhost` PROCEDURE `sp0`()
SELECT 1 latin1 latin1_swedish_ci latin1_swedish_ci
DROP PROCEDURE sp0;
SELECT VARIABLE_VALUE into @global_mem_used FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
SELECT VARIABLE_VALUE into @local_mem_used FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
CREATE PROCEDURE sp1() SELECT 1;
SHOW CREATE PROCEDURE sp1;
Procedure sql_mode Create Procedure character_set_client collation_connection Database Collation
sp1 STRICT_ALL_TABLES CREATE DEFINER=`root`@`localhost` PROCEDURE `sp1`()
SELECT 1 latin1 latin1_swedish_ci latin1_swedish_ci
SELECT VARIABLE_VALUE-@local_mem_used FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
VARIABLE_VALUE-@local_mem_used
0
SELECT VARIABLE_VALUE-@global_mem_used FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
VARIABLE_VALUE-@global_mem_used
0
DROP PROCEDURE sp1;
# End of 10.3 tests
17 changes: 17 additions & 0 deletions mysql-test/main/sp.test
Original file line number Diff line number Diff line change
Expand Up @@ -10431,4 +10431,21 @@ END;
$$
DELIMITER ;$$

# MDEV-20699 do not cache SP in SHOW CREATE
# Warmup round, this might allocate some memory for session variable
# and the output
SELECT VARIABLE_VALUE into @global_mem_used FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
SELECT VARIABLE_VALUE into @local_mem_used FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
CREATE PROCEDURE sp0() SELECT 1;
SHOW CREATE PROCEDURE sp0;
DROP PROCEDURE sp0;

#Check that CREATE/SHOW does not use memory in caches.
SELECT VARIABLE_VALUE into @global_mem_used FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
SELECT VARIABLE_VALUE into @local_mem_used FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
CREATE PROCEDURE sp1() SELECT 1;
SHOW CREATE PROCEDURE sp1;
SELECT VARIABLE_VALUE-@local_mem_used FROM INFORMATION_SCHEMA.SESSION_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
SELECT VARIABLE_VALUE-@global_mem_used FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='MEMORY_USED';
DROP PROCEDURE sp1;
--echo # End of 10.3 tests
16 changes: 7 additions & 9 deletions sql/sp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1890,8 +1890,6 @@ bool
Sp_handler::sp_show_create_routine(THD *thd,
const Database_qualified_name *name) const
{
sp_head *sp;

DBUG_ENTER("sp_show_create_routine");
DBUG_PRINT("enter", ("type: %s name: %.*s",
type_str(),
Expand All @@ -1904,20 +1902,20 @@ Sp_handler::sp_show_create_routine(THD *thd,
It is "safe" to do as long as it doesn't affect the results
of the binary log or the query cache, which currently it does not.
*/
if (sp_cache_routine(thd, name, false, &sp))
DBUG_RETURN(TRUE);

if (sp == NULL || sp->show_create_routine(thd, this))
sp_head *sp= 0;
bool free_sp= db_find_routine(thd, name, &sp) == SP_OK;
bool ret= !sp || sp->show_create_routine(thd, this);
if (ret)
{
/*
If we have insufficient privileges, pretend the routine
does not exist.
*/
my_error(ER_SP_DOES_NOT_EXIST, MYF(0), type_str(), name->m_name.str);
DBUG_RETURN(TRUE);
}

DBUG_RETURN(FALSE);
if (free_sp)
sp_head::destroy(sp);
DBUG_RETURN(ret);
}


Expand Down

0 comments on commit 1f09941

Please sign in to comment.