From: Jon Olav Hauglid Date: November 2 2010 12:44pm Subject: bzr commit into mysql-5.5-runtime branch (jon.hauglid:3177) Bug#57663 List-Archive: http://lists.mysql.com/commits/122528 X-Bug: 57663 Message-Id: <201011021245.oA29Kupf031688@acsinet15.oracle.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2550909873457129845==" --===============2550909873457129845== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #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); --===============2550909873457129845== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/jon.hauglid@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: jon.hauglid@stripped # target_branch: file:///export/home/x/mysql-5.5-runtime-bug57663/ # testament_sha1: 312977c024cf248f1dca86b05f7460345492d7b0 # timestamp: 2010-11-02 13:44:33 +0100 # source_branch: file:///export/home/x/mysql-5.5-bugfixing/ # base_revision_id: jon.hauglid@stripped\ # ygnikn79r4g0i09t # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWY/6waEAB4l/gFeQCABd9/// ////6r////5gD63w97UKT3dqjuw0pUs6FjQ7lm7FDp0BQA6SDo0A68JJI0aTTKTaj8p6k8SbKZoT aanlPSNM1DQ000Mg0B6gAlCmEyYhGo0IantNKbRGjTARkZBkAYE0aMBE1JkDTQANDQNA0Bk0AAAA AGgGgiSJMo01PUDUYPUGmkYJ6TEYARnqaAQNGmBBwNBpkNNGhhAyGhgjQ0yaNAMgxAAGgkSCAjIE wEJkxNNRkmU8moNNNqBkPUaBoNlFamQBs1UTxAeqR4OpiUfWnrjarDhbFL0OKGb6/O2bWkGso4o1 cSIi4ExJZGAWY7H92X8Was8EiBiaB6IHMoFBjsOUU1WofZ3BAbQwgnMGwhqfDsxeOOOP2USC9How gHGrEP24ool0CYSyO6fnmbsC57FwlDbY0l5a8QkptZZwfhODNiOZRHaxTGKcAayqCYpqr+Bvt66K X8zlj5yx61p91ItRXK2vskIU0I2iYMjREDlPSAT3XyTcO6mY4Wn3dHeeW7rJBkJPkl6w11hN6G/s cfQqR4OnOT+DA3CEp4PvgQFG5hh0C2GNukFdXTtlHvOKPMr2ks0ckVA1KgkH7gA6gConHUXwessm gPGCYFyXWm7Iuiv5BihQ31ZpEQMkfxguPxBY9vjdFS4NzF2AsxHiu8UCPlXiBOYG+lER7zizK6CI PLiZBMbPzB0kmnppzPqcW1c/lh5X7RPrvwP/ddJFcdDyqMBSaG3W4pFQDkI3gwYQf5DBcylyQrDt 2TnhF4Xil5/7k2srawwV2xXZp1xQ+pS1YyjHQkOH5xlrWQH7kf5ohW87y42qdOBlFN+aNM4T4W50 peqqcO/5qxTzrLQ2OxarFKg7ohwWKsigVuKE3KLxz068kKYRTG9QtomCdFTIg6q7aZCVCKgo6U71 dnxtm4u1UVqQqz74PueHYwk7ONzRFBr9Q7WKQnw30cM7XXFvcHh2TK8zZMjociufnPC+hnuSWAOF mZmZmGZmAgEIRCISRIhBkkhORUIO/Cc5MTMPC1FUdWV7KnxGA2NdOk85lDBogfxRlf4QlIr1x0XB f7IVMJWM/S6dhRdp2zRNED2DQjyOR56YVstz4bt3ZYZW2fs7G0JLx/Me4tDzAPgYnjUvD0b4jOzg kTaBF12ZSGDcJUbzI32SrJNBdtqfFlpIpaHurDrVcpas88MNS7C6wS7ECgSxGiuWFAs9kT7VuYe/ w8FhTQowXopJc6pBq06yUvDaz8FU7QkE7YAJiDodKWzgYiG4XyPgUkKjCYXW/JTtRVKf9Q58FIw6 6aEhDEckA3HOjQAVEoEFg4CNruCRJkYhRzSCSEhjmLovW2gDZe9VCfjX3HjtotNiFCa2Gc62WgF9 pb68PHLpUyMVyGRVHvERJ4kKMjdlx5jK2NZZmZd82S8qkSzOJsB8QG4FUGDFrDQdxlqWRLjI6Xlw zldpy0pFJdBI7LZFVqBiGdN3EoImI18pFtTX5SszZBvgTEuFie8h7pG8PsWNM16Ek4rRD7ZGvTA4 iO8pM4qzeBFFxvykThkfd2GzIasMfmX7Tn45WCL4NfYCzWBTI6MAyoRu8OkrGbRDNT3PxNAjM7x/ IRQqP1D9xlQicAPBqAyaS9+Sbxxmc8YrdzcNSJS1xl7txCu0LdZtJHeucSdIOu03rLIntP1jpvLz svngUNj4HQutrcoHbb4DwGdfSzu0h6aupsN5x4oux1lhyNcuibLR0W5lRaxumC8quLUGJHNwkMAh MyjYuGStnksbEWmspGrQYJkCJT18tehwRM1SLDTBUecxz2epdHkulNi0cWNKkYSKhY1CjA86ETYr zeMyMCTY6VMTdPS/AKMk0iTJ8vICaGZI1jgHSzhSR7G/VY3PFky/dDtRlCKQh2OYXira6fcHzA1Q 6Wx37dnimWQTcj4ZoiGtJWW0FhNeegtC5itOaSwYTC0dH2dYJoTsYp0Ex7+BUM8hd3bMFuIm1dC8 e2IjzYyAlu2JJHKBoObrJr2s4usjmA2AN5VNSqbIptmUhGH8qpDI0OHRsF/evzLJYCJBTBXQA5AZ jBAbJeFCD9Xf2nEpLheORB1wHhBYKDXCg2XXbOaXhgkIRU4hFfnSUFVpIKcTIA23EHbDTMkoTlBc Rc/BMhhGlDyRV0agCgow6rTWqAoURxiBI2gH5Ijmw5ZhAK4AOaMQNhhuDM20TbNABk8SRkQiwkfx TWxGlfwf+OCLWBUKoX4F/5aDeCoQMSU5gK1GkJVBSAkl0zA1d0hDYiaUcAApFNVkrgZ1E+pBr0Gx HLotJaNJbwMyEPESLwsaQSAXNHJGpIRkaLjYA1aI1AsQYEKaSQ0YCMCLNS10AYXUBarkZIxAYiEV AwSSKAMSJAtiLagBaBUJALCWruIAxHEQyWYCPQzTMBKFi9B/Ig1ESBsTd96ndefh6DhLqk2Guewd q60pB7s3Kwj05OnHEOzvW+o4DknhHFGuJsvrlgiUT2DjjLoogLbha7kNMBzyE/L0F7mY54P/1CGU v6xmeH9Df/URP2vQiIswGMxNfjOLnIyE8yCcyFp5csWRJfrOZtyJbhWyRQZg0NDsST4EZSNiC7SV uvqTGfrw7m8rUh/FBgF4BKrR/TUS2pC2HlJgfxXrZ7aRyEiJhLqukZeY2E/ENZX0hOOTmO3XDNWI 77LjPBXd+LqLeacA5gnwtIHghuLSPX00wbX7zvOX7ddDYZHUud5pDc8fVW7z0S/eWoXQxqehhugj E6bhaQEX2sCAGJVaRIStS/0MkBsvAObQm1gIZgcBjSCCUsWXwgxRGdsgsSghRGljmsF7AUnQgNSQ JUOIPJblCL7tZqLrUaaoj6CaEbsTSBpy/tgEXMQxiF0n4mpWHBLl5HcXc2kK7AtzO3Z7BtEeFhaX ExHu9gtMAdMXZaRv7zoBb06v0zyPidWOlPpn9dtbILLJ9wc4PNJqK1JF2EwLXS6gRF+6WCIkyAaI m3lbb5iYgxz9js0kvSLnsPkOfqtegzdlFSfCkKMlaLOkj3m379CqkfcI0ZG/wARaBXaGCaElGseh QcJCvZynDa0MnN1IA9Pd6Tf1dbkhZ2mpp3Mll5sOlxUbZUqKGJuihSiuJDWQR4OQRIsF22oQ2whQ EI8IjzGGGCemsaSdo7MdBPrww3uPhH1snnG3EjU+5H6UGWoTaYylJNWo8o1UAnK0wIAw9vc8AMy6 iWYeh4ncbi77ExmEEJNpiYj4d2fiSDqeBeQY1LEheZX2oLipGoOvvPyT1LUHEvGKEGKZ85Igl8Cc kFWtOixb2JQrlxiQI6d/XWlfrdiSs5fWqG7xofSPZ3rimyghnuA4QppHcJaYSPEYcf6AJlRedDyi LHwlWRNN1ARYbtVN8CXGmO5dHwvRVzKNVZknCTtctPx6otoJ3F6qrBGxQleqI9PXklJLeKqNshEG toa10O6eQjzEZF/YaN326PTtzxEAZjENiYMFn42LsGmMaZmlwFSW5HtGvyQt5wUIuESMTR3uA5QG H6feaDYvxPtBqOz3eN7QZtJDYzJnI+fZApHqvVWSTA7S7WFXrRiExS7JBcRUuhWR+FerVVeDeAlA oJLnCuPdHcilt54cjokqLUimMnDtt7TQtGIyANkecDY5ZMoNMrolhjeJLWbdaVGfR3bHfdrKQMAh 75CeIwPb8vWeNrRYkR2uYDDs66cfC60NHhPRP8zE4mi93wwYMaBJsbFDHpeEoYuxoRsGID4piVbv 4bIVoF7SuQsXG7Eha42EuJRJOOohKLFFKMgIbIx7FoJh8RFUAEy9O6s0r0XCLWiVAgJDQNcT+SuX Z8QEawPv4zRoC4aExAM5cF0X3MNQhp/7GkAJI9Sh/QWLgIEF7DQmB06Nkg3AIBAhBCKYLNhJA0MG LqJahTkuckJwB7/hNPcUXI0j8p27EtuhGI+KLaC8w4QjkdxSSNHg00MKh9VGagjPARpn6you+hQt u/NrJr5NqEaLbCWJu9Tfv1ge+2DnmHDXslARAxwJ3I9mQvtTmCOk0akpNqwpJN3d1iOSPotErFp6 q8G0cNIVxDRt8q716Rg+PWA06HXDpPq51gLOvO7RAIG2RLIeqZjgcSCpNaKzbOsYk4hvnwHZ7glV aicmwnDIhtkJpk5ViClCRImpw0410mjy66880VCGXc9WLkOCwhFsrFqRv2OGsFxstCuIoKK2Xz1I 7LUacUv0Frga8JaqkturDA40TOsQKIhiATgjSOibxPs79Fi9La9D1EVFxEGi1HCYDEc48iM/7+Nl ZlF7rYRzpg0SvML7iiKm2wl8H94j7Yqg06HgItVLROSog/Rs+jhcBKwQVDI9iEX11InNIwO/t8Gb yFqNWkUWLjJIzf/zBbxrY/59M+OC1J8eh2ImaUVgSAtkiGiEQb2Iuu5mIFEgkqR5NE0cfvlUmXuA waJctlOfqZ+ZwAwtPu9g/f9S/rB70lPf9fCYgvTQOEKeZyN1K0SjICfhyC2xtn7BhrNN9Bo7/tJT PpNx7FzyOYWNGFeixLf1FPaSql06FIoMu+jEixwxA1BD3TMcLAd03wpyyFxrhf4CQEvUxDt7r5UY TbRU+z1P5TQqeColzOmaMb/QQdOW95AggX/F3JFOFCQj/rBoQA== --===============2550909873457129845==--