List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:November 2 2010 12:44pm
Subject:bzr commit into mysql-5.5-runtime branch (jon.hauglid:3177) Bug#57663
View as plain text  
#At file:///export/home/x/mysql-5.5-runtime-bug57663/ based on revid:jon.hauglid@stripped

 3177 Jon Olav Hauglid	2010-11-02
      Bug #57663 Concurrent statement using stored function and DROP DATABASE
                 breaks SBR
      
      This is a preliminary version of the patch.
      
      The problem was that DROP DATABASE ignores any metadata locks on stored
      functions and procedures held by other connections. This makes it
      possible for DROP DATABASE to drop functions/procedures that are in use
      by other connections and therefore break statement based replication.
      (DROP DATABASE can appear in the binlog before a statement using a
      dropped function/procedure.)
      
      This problem was an issue left unresolved by the patch for Bug#30977
      where metadata locks for stored functions/procedures were introduced.
      
      This patch fixes the problem by making sure DROP DATABASE takes
      exclusive metadata locks on all stored functions/procedures to be
      dropped.
      
      Test case added to sp-lock.test.
      
      Questions to the reviewer:
      - The return value from sp_drop_db_routines() is currently ignored
        by mysql_rm_db(). Should this be addressed in the scope of this patch?

    modified:
      mysql-test/r/sp-lock.result
      mysql-test/t/sp-lock.test
      sql/sp.cc
=== modified file 'mysql-test/r/sp-lock.result'
--- a/mysql-test/r/sp-lock.result	2010-08-06 11:29:37 +0000
+++ b/mysql-test/r/sp-lock.result	2010-11-02 12:44:24 +0000
@@ -735,5 +735,91 @@ END	latin1	latin1_swedish_ci	latin1_swed
 # Connection default;
 DROP PROCEDURE p1;
 #
+# Bug#57663 Concurrent statement using stored function and DROP DATABASE
+#           breaks SBR
+#
+DROP DATABASE IF EXISTS db1;
+DROP FUNCTION IF EXISTS f1;
+# Test 1: Check that DROP DATABASE block if a function is used
+#         by an active transaction.
+# Connection default
+CREATE DATABASE db1;
+CREATE FUNCTION db1.f1() RETURNS INTEGER RETURN 1;
+START TRANSACTION;
+SELECT db1.f1();
+db1.f1()
+1
+# Connection con1
+# Sending:
+DROP DATABASE db1;
+# Connection default
+COMMIT;
+# Connection con1
+# Reaping: DROP DATABASE db1
+# Test 2: Check that DROP DATABASE blocks if a procedure is
+#         used by an active transaction.
+# Connection default
+CREATE DATABASE db1;
+CREATE PROCEDURE db1.p1() BEGIN END;
+CREATE FUNCTION f1() RETURNS INTEGER
+BEGIN
+CALL db1.p1();
+RETURN 1;
+END|
+START TRANSACTION;
+SELECT f1();
+f1()
+1
+# Connection con1
+# Sending:
+DROP DATABASE db1;
+# Connection default
+COMMIT;
+# Connection con1
+# Reaping: DROP DATABASE db1
+# Test 3: Check that DROP DATABASE is not selected as a victim if a
+#         deadlock is discovered with DML statements.
+# Connection default
+CREATE DATABASE db1;
+CREATE FUNCTION db1.f1() RETURNS INTEGER RETURN 1;
+CREATE FUNCTION db1.f2() RETURNS INTEGER RETURN 1;
+START TRANSACTION;
+SELECT db1.f2();
+db1.f2()
+1
+# Connection con1
+# Sending:
+DROP DATABASE db1;
+# Connection default
+SELECT db1.f1();
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+COMMIT;
+# Connection con1
+# Reaping: DROP DATABASE db1
+# Test 4: Check that active DROP DATABASE blocks stored routine DDL.
+# Connection default
+CREATE DATABASE db1;
+CREATE FUNCTION db1.f1() RETURNS INTEGER RETURN 1;
+CREATE FUNCTION db1.f2() RETURNS INTEGER RETURN 2;
+START TRANSACTION;
+SELECT db1.f2();
+db1.f2()
+2
+# Connection con1
+# Sending:
+DROP DATABASE db1;
+# Connection con2
+# Sending:
+ALTER FUNCTION db1.f1 COMMENT "test";
+# Connection default
+COMMIT;
+# Connection con1
+# Reaping: DROP DATABASE db1
+# Connection con2
+# Reaping: ALTER FUNCTION f1 COMMENT 'test'
+ERROR 42000: FUNCTION db1.f1 does not exist
+# Connection default
+DROP FUNCTION f1;
+#
 # End of 5.5 tests
 #

=== modified file 'mysql-test/t/sp-lock.test'
--- a/mysql-test/t/sp-lock.test	2010-08-06 11:29:37 +0000
+++ b/mysql-test/t/sp-lock.test	2010-11-02 12:44:24 +0000
@@ -972,5 +972,165 @@ DROP PROCEDURE p1;
 
 
 --echo #
+--echo # Bug#57663 Concurrent statement using stored function and DROP DATABASE
+--echo #           breaks SBR
+--echo #
+
+--disable_warnings
+DROP DATABASE IF EXISTS db1;
+DROP FUNCTION IF EXISTS f1;
+--enable_warnings
+
+connect(con1, localhost, root);
+connect(con2, localhost, root);
+
+--echo # Test 1: Check that DROP DATABASE block if a function is used
+--echo #         by an active transaction.
+
+--echo # Connection default
+connection default;
+CREATE DATABASE db1;
+CREATE FUNCTION db1.f1() RETURNS INTEGER RETURN 1;
+START TRANSACTION;
+SELECT db1.f1();
+
+--echo # Connection con1
+connection con1;
+--echo # Sending:
+--send DROP DATABASE db1
+
+--echo # Connection default
+connection default;
+let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
+  WHERE state= 'Waiting for stored function metadata lock'
+  AND info='DROP DATABASE db1';
+--source include/wait_condition.inc
+COMMIT;
+
+--echo # Connection con1
+connection con1;
+--echo # Reaping: DROP DATABASE db1
+--reap
+
+--echo # Test 2: Check that DROP DATABASE blocks if a procedure is
+--echo #         used by an active transaction.
+
+--echo # Connection default
+connection default;
+CREATE DATABASE db1;
+CREATE PROCEDURE db1.p1() BEGIN END;
+delimiter |;
+CREATE FUNCTION f1() RETURNS INTEGER
+BEGIN
+  CALL db1.p1();
+  RETURN 1;
+END|
+delimiter ;|
+START TRANSACTION;
+SELECT f1();
+
+--echo # Connection con1
+connection con1;
+--echo # Sending:
+--send DROP DATABASE db1
+
+--echo # Connection default
+connection default;
+let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
+  WHERE state= 'Waiting for stored procedure metadata lock'
+  AND info='DROP DATABASE db1';
+--source include/wait_condition.inc
+COMMIT;
+
+--echo # Connection con1
+connection con1;
+--echo # Reaping: DROP DATABASE db1
+--reap
+
+--echo # Test 3: Check that DROP DATABASE is not selected as a victim if a
+--echo #         deadlock is discovered with DML statements.
+
+--echo # Connection default
+connection default;
+CREATE DATABASE db1;
+CREATE FUNCTION db1.f1() RETURNS INTEGER RETURN 1;
+CREATE FUNCTION db1.f2() RETURNS INTEGER RETURN 1;
+START TRANSACTION;
+# DROP DATABASE will try to lock f2 before f1
+SELECT db1.f2();
+
+--echo # Connection con1
+connection con1;
+--echo # Sending:
+--send DROP DATABASE db1
+
+--echo # Connection default
+connection default;
+let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
+  WHERE state= 'Waiting for stored function metadata lock'
+  AND info='DROP DATABASE db1';
+--source include/wait_condition.inc
+--error ER_LOCK_DEADLOCK
+SELECT db1.f1();
+COMMIT;
+
+--echo # Connection con1
+connection con1;
+--echo # Reaping: DROP DATABASE db1
+--reap
+
+--echo # Test 4: Check that active DROP DATABASE blocks stored routine DDL.
+
+--echo # Connection default
+connection default;
+CREATE DATABASE db1;
+CREATE FUNCTION db1.f1() RETURNS INTEGER RETURN 1;
+CREATE FUNCTION db1.f2() RETURNS INTEGER RETURN 2;
+START TRANSACTION;
+SELECT db1.f2();
+
+--echo # Connection con1
+connection con1;
+--echo # Sending:
+--send DROP DATABASE db1
+
+--echo # Connection con2
+connection con2;
+let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
+  WHERE state= 'Waiting for stored function metadata lock'
+  AND info='DROP DATABASE db1';
+--source include/wait_condition.inc
+--echo # Sending:
+--send ALTER FUNCTION db1.f1 COMMENT "test"
+
+--echo # Connection default
+connection default;
+let $wait_condition= SELECT COUNT(*)= 1 FROM information_schema.processlist
+  WHERE state= 'Waiting for schema metadata lock'
+  AND info='ALTER FUNCTION db1.f1 COMMENT "test"';
+--source include/wait_condition.inc
+COMMIT;
+
+--echo # Connection con1
+connection con1;
+--echo # Reaping: DROP DATABASE db1
+--reap
+disconnect con1;
+--source include/wait_until_disconnected.inc
+
+--echo # Connection con2
+connection con2;
+--echo # Reaping: ALTER FUNCTION f1 COMMENT 'test'
+--error ER_SP_DOES_NOT_EXIST
+--reap
+disconnect con2;
+--source include/wait_until_disconnected.inc
+
+--echo # Connection default
+connection default;
+DROP FUNCTION f1;
+
+
+--echo #
 --echo # End of 5.5 tests
 --echo #

=== modified file 'sql/sp.cc'
--- a/sql/sp.cc	2010-10-21 08:41:13 +0000
+++ b/sql/sp.cc	2010-11-02 12:44:24 +0000
@@ -1370,7 +1370,10 @@ sp_drop_db_routines(THD *thd, char *db)
   TABLE *table;
   int ret;
   uint key_len;
-  MDL_ticket *mdl_savepoint= thd->mdl_context.mdl_savepoint();
+  longlong type;
+  char *name;
+  char buff[65];
+  String str(buff, sizeof(buff), &my_charset_bin);
   DBUG_ENTER("sp_drop_db_routines");
   DBUG_PRINT("enter", ("db: %s", db));
 
@@ -1392,14 +1395,35 @@ sp_drop_db_routines(THD *thd, char *db)
 
     do
     {
-      if (! table->file->ha_delete_row(table->record[0]))
-	deleted= TRUE;		/* We deleted something */
-      else
+      /*
+        Take an exclusive MDL lock on the routine to be dropped.
+        These locks (as well as the lock on mysql.proc) are held until the
+        end of the statement.
+
+        Taking these locks one by one won't give a higher risk for
+        DROP DATABASE to abort with ER_LOCK_DEADLOCK because:
+        - We already have an MDL_EXCLUSIVE lock on the database which will
+          prevent conflicting DDL statements.
+        - DML statements that use stored routines can cause deadlock, but in
+          this case, the conflicting DML statement will be selected as victim.
+      */
+      table->field[MYSQL_PROC_FIELD_NAME]->val_str(&str, &str);
+      name= thd->strmake(str.ptr(), str.length());
+      type= table->field[MYSQL_PROC_MYSQL_TYPE]->val_int();
+      MDL_request mdl_request;
+      mdl_request.init(type == TYPE_ENUM_FUNCTION ?
+                       MDL_key::FUNCTION : MDL_key::PROCEDURE,
+                       db, name, MDL_EXCLUSIVE);
+      if (thd->mdl_context.acquire_lock(&mdl_request,
+                                        thd->variables.lock_wait_timeout) ||
+          table->file->ha_delete_row(table->record[0]))
       {
-	ret= SP_DELETE_ROW_FAILED;
-	nxtres= 0;
-	break;
+        ret= SP_DELETE_ROW_FAILED;
+        nxtres= 0;
+        break;
       }
+      else
+	deleted= TRUE;		/* We deleted something */
     } while (! (nxtres= table->file->index_next_same(table->record[0],
                                 (uchar *)table->field[MYSQL_PROC_FIELD_DB]->ptr,
 						     key_len)));
@@ -1411,11 +1435,6 @@ sp_drop_db_routines(THD *thd, char *db)
   table->file->ha_index_end();
 
   close_thread_tables(thd);
-  /*
-    Make sure to only release the MDL lock on mysql.proc, not other
-    metadata locks DROP DATABASE might have acquired.
-  */
-  thd->mdl_context.rollback_to_savepoint(mdl_savepoint);
 
 err:
   DBUG_RETURN(ret);


Attachment: [text/bzr-bundle] bzr/jon.hauglid@oracle.com-20101102124424-h47t140nne3zswbw.bundle
Thread
bzr commit into mysql-5.5-runtime branch (jon.hauglid:3177) Bug#57663Jon Olav Hauglid2 Nov