diff --git a/include/heap.h b/include/heap.h index 7d326c4988762..d3eb4efa4e9c1 100644 --- a/include/heap.h +++ b/include/heap.h @@ -219,6 +219,7 @@ typedef struct st_heap_info my_bool implicit_emptied; my_bool has_zerocopy_blobs; /* Last hp_read_blobs produced zero-copy ptrs */ my_bool has_pending_blob_free; /* pending_blob_chains awaits freeing */ + my_bool lock_granted; /* thr_lock() gave `lock' to this handle */ THR_LOCK_DATA lock; LIST open_list; } HP_INFO; diff --git a/mysql-test/suite/heap/blob_delayed_insert.opt b/mysql-test/suite/heap/blob_delayed_insert.opt new file mode 100644 index 0000000000000..789275fa25e27 --- /dev/null +++ b/mysql-test/suite/heap/blob_delayed_insert.opt @@ -0,0 +1 @@ +--skip-log-bin diff --git a/mysql-test/suite/heap/blob_delayed_insert.result b/mysql-test/suite/heap/blob_delayed_insert.result new file mode 100644 index 0000000000000..b869012ce137d --- /dev/null +++ b/mysql-test/suite/heap/blob_delayed_insert.result @@ -0,0 +1,51 @@ +CREATE TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; +INSERT DELAYED INTO t1 VALUES (1, REPEAT('x', 300)), (2, REPEAT('y', 300)); +SELECT VARIABLE_VALUE > 0 AS delayed_thread_ran +FROM information_schema.global_status +WHERE VARIABLE_NAME = 'DELAYED_WRITES'; +delayed_thread_ran +1 +INSERT DELAYED INTO t1 VALUES (3, REPEAT('z', 300)), (4, REPEAT('w', 300)); +connect reader,localhost,root,,test; +connection default; +INSERT DELAYED INTO t1 VALUES (5, REPEAT('v', 300)); +connection reader; +SELECT COUNT(*) >= 0 AS reader_ran FROM t1; +reader_ran +1 +connection default; +INSERT DELAYED INTO t1 VALUES (6, REPEAT('u', 300)); +disconnect reader; +SELECT a, LENGTH(b) FROM t1; +a LENGTH(b) +1 300 +2 300 +3 300 +4 300 +5 300 +6 300 +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +INSERT INTO t1 VALUES (7, REPEAT('t', 300)); +UPDATE t1 SET b = REPEAT('s', 400) WHERE a = 7; +DELETE FROM t1 WHERE a = 7; +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +FLUSH TABLES; +INSERT INTO t1 VALUES (8, REPEAT('r', 300)); +UPDATE t1 SET b = REPEAT('q', 400) WHERE a = 8; +DELETE FROM t1 WHERE a = 8; +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +SELECT a, LENGTH(b) FROM t1; +a LENGTH(b) +1 300 +2 300 +3 300 +4 300 +5 300 +6 300 +DROP TABLE t1; diff --git a/mysql-test/suite/heap/blob_delayed_insert.test b/mysql-test/suite/heap/blob_delayed_insert.test new file mode 100644 index 0000000000000..eeb29b77c27e6 --- /dev/null +++ b/mysql-test/suite/heap/blob_delayed_insert.test @@ -0,0 +1,76 @@ +# +# The record of a THR_LOCK grant survives an INSERT DELAYED lock cycle. +# +# thr_lock() does not call the grant callback once per external_lock(): a +# delayed insert is granted TL_WRITE_DELAYED and then calls it again on the +# same request when thr_upgrade_write_delay_lock() turns that into a real +# write lock, with no external_lock() in between. A handle that counted +# grants would keep the surplus for its whole life and then report a lock it +# does not hold on every later statement. +# +# The binary log must be off: with statement binlogging INSERT DELAYED is +# downgraded to an ordinary write lock and the delayed thread never runs. +# + +CREATE TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; + +INSERT DELAYED INTO t1 VALUES (1, REPEAT('x', 300)), (2, REPEAT('y', 300)); + +--let $wait_condition= SELECT COUNT(*) = 2 FROM t1 +--source include/wait_condition.inc + +# The delayed thread really did the writing, rather than the statement being +# silently downgraded to a plain INSERT. +SELECT VARIABLE_VALUE > 0 AS delayed_thread_ran + FROM information_schema.global_status + WHERE VARIABLE_NAME = 'DELAYED_WRITES'; + +# Second cycle on the still-live delayed thread: it locks the same handle +# again, which is where a leaked grant is noticed. +INSERT DELAYED INTO t1 VALUES (3, REPEAT('z', 300)), (4, REPEAT('w', 300)); + +--let $wait_condition= SELECT COUNT(*) = 4 FROM t1 +--source include/wait_condition.inc + +# A third cycle with a concurrent reader, so the delayed thread has to give +# the lock up and take it again mid-batch. +connect (reader,localhost,root,,test); + +connection default; +INSERT DELAYED INTO t1 VALUES (5, REPEAT('v', 300)); + +connection reader; +SELECT COUNT(*) >= 0 AS reader_ran FROM t1; + +connection default; +INSERT DELAYED INTO t1 VALUES (6, REPEAT('u', 300)); + +--let $wait_condition= SELECT COUNT(*) = 6 FROM t1 +--source include/wait_condition.inc + +disconnect reader; + +--sorted_result +SELECT a, LENGTH(b) FROM t1; +CHECK TABLE t1; + +# Ordinary lock cycles on the same table afterwards. These write blobs, so +# they park and redeem chains, which a handle wrongly believing it holds the +# lock would do outside it. +INSERT INTO t1 VALUES (7, REPEAT('t', 300)); +UPDATE t1 SET b = REPEAT('s', 400) WHERE a = 7; +DELETE FROM t1 WHERE a = 7; +CHECK TABLE t1; + +# And once more after the table is reopened, so the same share is reached +# through a fresh handle. +FLUSH TABLES; +INSERT INTO t1 VALUES (8, REPEAT('r', 300)); +UPDATE t1 SET b = REPEAT('q', 400) WHERE a = 8; +DELETE FROM t1 WHERE a = 8; +CHECK TABLE t1; + +--sorted_result +SELECT a, LENGTH(b) FROM t1; + +DROP TABLE t1; diff --git a/mysql-test/suite/heap/blob_lock_twice.result b/mysql-test/suite/heap/blob_lock_twice.result new file mode 100644 index 0000000000000..a51a55bbfaf28 --- /dev/null +++ b/mysql-test/suite/heap/blob_lock_twice.result @@ -0,0 +1,59 @@ +CREATE TABLE ht (a INT, b BLOB) ENGINE=MEMORY; +CREATE TABLE mi (a INT, b BLOB) ENGINE=MyISAM; +INSERT INTO ht VALUES (1,'foo'),(2,'bar'); +INSERT INTO mi VALUES (1,'foo'),(2,'bar'); +# ===== single aliased entry, referenced as "table AS alias" ===== +# -- MyISAM +LOCK TABLE mi AS m1 WRITE; +SELECT COUNT(*) FROM mi AS m1; +COUNT(*) +2 +UPDATE mi AS m1 SET b='x' WHERE a=1; +# the unaliased name is not locked +SELECT COUNT(*) FROM mi; +ERROR HY000: Table 'mi' was not locked with LOCK TABLES +UNLOCK TABLES; +# -- MEMORY +LOCK TABLE ht AS h1 WRITE; +SELECT COUNT(*) FROM ht AS h1; +COUNT(*) +2 +UPDATE ht AS h1 SET b='x' WHERE a=1; +# the unaliased name is not locked +SELECT COUNT(*) FROM ht; +ERROR HY000: Table 'ht' was not locked with LOCK TABLES +UNLOCK TABLES; +# ===== two aliased entries, one WRITE and one READ ===== +# -- MyISAM +LOCK TABLE mi AS m1 WRITE, mi AS m2 READ; +SELECT COUNT(*) FROM mi AS m2; +COUNT(*) +2 +UPDATE mi AS m1 SET b='y' WHERE a=1; +# INSERT takes no alias, so it cannot reach an aliased lock at all +INSERT INTO mi SELECT a+10, b FROM mi AS m2; +ERROR HY000: Table 'mi' was not locked with LOCK TABLES +UNLOCK TABLES; +# -- MEMORY +LOCK TABLE ht AS h1 WRITE, ht AS h2 READ; +SELECT COUNT(*) FROM ht AS h2; +COUNT(*) +2 +UPDATE ht AS h1 SET b='y' WHERE a=1; +# INSERT takes no alias, so it cannot reach an aliased lock at all +INSERT INTO ht SELECT a+10, b FROM ht AS h2; +ERROR HY000: Table 'ht' was not locked with LOCK TABLES +UNLOCK TABLES; +# ===== blob update and delete under the double lock (MEMORY) ===== +LOCK TABLE ht AS h1 WRITE, ht AS h2 READ; +UPDATE ht AS h1, ht AS h2 SET h1.b=REPEAT('z', 900) +WHERE h1.a=h2.a AND h1.a=1; +DELETE FROM ht AS h1 WHERE a=2; +UNLOCK TABLES; +CHECK TABLE ht; +Table Op Msg_type Msg_text +test.ht check status OK +SELECT a, LENGTH(b) FROM ht; +a LENGTH(b) +1 900 +DROP TABLE ht, mi; diff --git a/mysql-test/suite/heap/blob_lock_twice.test b/mysql-test/suite/heap/blob_lock_twice.test new file mode 100644 index 0000000000000..0dd62abe47419 --- /dev/null +++ b/mysql-test/suite/heap/blob_lock_twice.test @@ -0,0 +1,69 @@ +# +# A MEMORY table with a blob locked twice in the same lock set. +# +# Two aliased entries give one share two handles, and thr_lock() grants each +# of them separately, so the grant a handle records is its own. Every row +# operation below runs with both grants live, and the blob update parks a +# chain that has to be redeemed when the handles are unlocked. MyISAM runs +# the same shapes as a control. +# +# An aliased entry can only be referenced by its alias while LOCK TABLES is +# in effect; the unaliased name is not in the lock set at all. That is +# asserted rather than avoided, because it is what keeps the two handles +# distinct. +# +CREATE TABLE ht (a INT, b BLOB) ENGINE=MEMORY; +CREATE TABLE mi (a INT, b BLOB) ENGINE=MyISAM; +INSERT INTO ht VALUES (1,'foo'),(2,'bar'); +INSERT INTO mi VALUES (1,'foo'),(2,'bar'); + +--echo # ===== single aliased entry, referenced as "table AS alias" ===== +--echo # -- MyISAM +LOCK TABLE mi AS m1 WRITE; +SELECT COUNT(*) FROM mi AS m1; +UPDATE mi AS m1 SET b='x' WHERE a=1; +--echo # the unaliased name is not locked +--error ER_TABLE_NOT_LOCKED +SELECT COUNT(*) FROM mi; +UNLOCK TABLES; + +--echo # -- MEMORY +LOCK TABLE ht AS h1 WRITE; +SELECT COUNT(*) FROM ht AS h1; +UPDATE ht AS h1 SET b='x' WHERE a=1; +--echo # the unaliased name is not locked +--error ER_TABLE_NOT_LOCKED +SELECT COUNT(*) FROM ht; +UNLOCK TABLES; + +--echo # ===== two aliased entries, one WRITE and one READ ===== +--echo # -- MyISAM +LOCK TABLE mi AS m1 WRITE, mi AS m2 READ; +SELECT COUNT(*) FROM mi AS m2; +UPDATE mi AS m1 SET b='y' WHERE a=1; +--echo # INSERT takes no alias, so it cannot reach an aliased lock at all +--error ER_TABLE_NOT_LOCKED +INSERT INTO mi SELECT a+10, b FROM mi AS m2; +UNLOCK TABLES; + +--echo # -- MEMORY +LOCK TABLE ht AS h1 WRITE, ht AS h2 READ; +SELECT COUNT(*) FROM ht AS h2; +UPDATE ht AS h1 SET b='y' WHERE a=1; +--echo # INSERT takes no alias, so it cannot reach an aliased lock at all +--error ER_TABLE_NOT_LOCKED +INSERT INTO ht SELECT a+10, b FROM ht AS h2; +UNLOCK TABLES; + +--echo # ===== blob update and delete under the double lock (MEMORY) ===== +LOCK TABLE ht AS h1 WRITE, ht AS h2 READ; +UPDATE ht AS h1, ht AS h2 SET h1.b=REPEAT('z', 900) + WHERE h1.a=h2.a AND h1.a=1; +DELETE FROM ht AS h1 WHERE a=2; +UNLOCK TABLES; + +CHECK TABLE ht; +--sorted_result +SELECT a, LENGTH(b) FROM ht; + +DROP TABLE ht, mi; diff --git a/mysql-test/suite/heap/blob_online_alter.result b/mysql-test/suite/heap/blob_online_alter.result new file mode 100644 index 0000000000000..1583e3608b121 --- /dev/null +++ b/mysql-test/suite/heap/blob_online_alter.result @@ -0,0 +1,26 @@ +CREATE TABLE t1 (a INT PRIMARY KEY, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1, REPEAT('x', 300)), (2, REPEAT('y', 300)), +(3, REPEAT('w', 300)), (4, REPEAT('v', 300)); +connect alterer,localhost,root,,test; +SET DEBUG_SYNC= 'alter_table_online_downgraded SIGNAL downgraded EXECUTE 1'; +SET DEBUG_SYNC= 'alter_table_online_progress WAIT_FOR dml_done EXECUTE 1'; +ALTER TABLE t1 ADD COLUMN c INT, ALGORITHM=COPY, LOCK=NONE; +connection default; +SET SESSION lock_wait_timeout= 20; +SET DEBUG_SYNC= 'now WAIT_FOR downgraded'; +DELETE FROM t1 WHERE a = 1; +UPDATE t1 SET b = REPEAT('z', 900) WHERE a = 2; +DELETE FROM t1 WHERE a = 3; +SET DEBUG_SYNC= 'now SIGNAL dml_done'; +connection alterer; +connection default; +disconnect alterer; +SET DEBUG_SYNC= 'RESET'; +SELECT a, LENGTH(b), c FROM t1; +a LENGTH(b) c +2 900 NULL +4 300 NULL +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +DROP TABLE t1; diff --git a/mysql-test/suite/heap/blob_online_alter.test b/mysql-test/suite/heap/blob_online_alter.test new file mode 100644 index 0000000000000..c6891493212c4 --- /dev/null +++ b/mysql-test/suite/heap/blob_online_alter.test @@ -0,0 +1,47 @@ +# +# Blob chains parked on an online-ALTER copy target. +# +# The copy target is locked with a direct handler::ha_external_lock() rather +# than through the SQL layer's lock set, so thr_lock() never grants it +# anything and the handle holds a write lock that no grant records. Rows +# deleted or updated on the source while the copy runs are replayed onto that +# target, and a blob delete/update there parks a chain that has to be redeemed +# when the copy is unlocked. +# +# The ALTER is not paused while it holds the source lock -- doing that blocks +# the very statements this test needs to run. It is released at the +# post-downgrade point and only made to wait once it is replaying. +# +--source include/have_debug_sync.inc + +CREATE TABLE t1 (a INT PRIMARY KEY, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1, REPEAT('x', 300)), (2, REPEAT('y', 300)), + (3, REPEAT('w', 300)), (4, REPEAT('v', 300)); + +connect (alterer,localhost,root,,test); +SET DEBUG_SYNC= 'alter_table_online_downgraded SIGNAL downgraded EXECUTE 1'; +SET DEBUG_SYNC= 'alter_table_online_progress WAIT_FOR dml_done EXECUTE 1'; +--send ALTER TABLE t1 ADD COLUMN c INT, ALGORITHM=COPY, LOCK=NONE + +connection default; +SET SESSION lock_wait_timeout= 20; +SET DEBUG_SYNC= 'now WAIT_FOR downgraded'; + +# Both shapes that park a chain: a delete, and an update that grows the blob +DELETE FROM t1 WHERE a = 1; +UPDATE t1 SET b = REPEAT('z', 900) WHERE a = 2; +DELETE FROM t1 WHERE a = 3; + +SET DEBUG_SYNC= 'now SIGNAL dml_done'; + +connection alterer; +--reap + +connection default; +disconnect alterer; +SET DEBUG_SYNC= 'RESET'; + +--sorted_result +SELECT a, LENGTH(b), c FROM t1; +CHECK TABLE t1; +DROP TABLE t1; diff --git a/mysql-test/suite/heap/blob_tmp_table.result b/mysql-test/suite/heap/blob_tmp_table.result new file mode 100644 index 0000000000000..77058a9ddeeea --- /dev/null +++ b/mysql-test/suite/heap/blob_tmp_table.result @@ -0,0 +1,95 @@ +# +# Blob updates and deletes on a user TEMPORARY MEMORY table. +# +# A non-transactional TEMPORARY table is left out of the lock set +# altogether, so its handle never holds a THR_LOCK. It is not an +# internal table, though, so it still defers its blob chain frees -- +# a combination no other kind of table has, and one a debug build used +# to assert on at the end of every such statement. +# +CREATE TEMPORARY TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1,'one'), (2,REPEAT('two',500)), (3,REPEAT('three',500)); +# UPDATE parks one chain per changed blob +UPDATE t1 SET b=REPEAT('x',4000) WHERE a=2; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +a LENGTH(b) LEFT(b,5) +1 3 one +2 4000 xxxxx +3 2500 three +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +# DELETE parks the whole row's chain +DELETE FROM t1 WHERE a=3; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +a LENGTH(b) LEFT(b,5) +1 3 one +2 4000 xxxxx +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +# Repeated cycles: every parked chain has to come back to the free list +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +a LENGTH(b) LEFT(b,5) +1 1500 oneon +2 8000 zzzzz +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +# Multi-table UPDATE and DELETE across two TEMPORARY tables +CREATE TEMPORARY TABLE t2 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t2 VALUES (1,REPEAT('u',4000)), (2,REPEAT('v',4000)); +UPDATE t1, t2 SET t1.b=REPEAT('p',4000), t2.b=REPEAT('q',4000) +WHERE t1.a=t2.a; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +a LENGTH(b) LEFT(b,5) +1 4000 ppppp +2 4000 ppppp +SELECT a, LENGTH(b), LEFT(b,5) FROM t2 ORDER BY a; +a LENGTH(b) LEFT(b,5) +1 4000 qqqqq +2 4000 qqqqq +CHECK TABLE t1, t2; +Table Op Msg_type Msg_text +test.t1 check status OK +test.t2 check status OK +DELETE t1, t2 FROM t1, t2 WHERE t1.a=t2.a AND t1.a=1; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +a LENGTH(b) LEFT(b,5) +2 4000 ppppp +SELECT a, LENGTH(b), LEFT(b,5) FROM t2 ORDER BY a; +a LENGTH(b) LEFT(b,5) +2 4000 qqqqq +CHECK TABLE t1, t2; +Table Op Msg_type Msg_text +test.t1 check status OK +test.t2 check status OK +DROP TEMPORARY TABLE t1, t2; +# +# The same statements on a non-temporary MEMORY table, which does hold +# the lock while it parks and redeems at unlock time. +# +CREATE TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1,'one'), (2,REPEAT('two',500)), (3,REPEAT('three',500)); +UPDATE t1 SET b=REPEAT('x',4000) WHERE a=2; +DELETE FROM t1 WHERE a=3; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +a LENGTH(b) LEFT(b,5) +1 3 one +2 4000 xxxxx +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +# Under LOCK TABLES the redemption happens in ha_heap::reset() instead, +# with the lock still held +LOCK TABLES t1 WRITE; +UPDATE t1 SET b=REPEAT('w',4000) WHERE a=2; +DELETE FROM t1 WHERE a=1; +UNLOCK TABLES; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +a LENGTH(b) LEFT(b,5) +2 4000 wwwww +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +DROP TABLE t1; diff --git a/mysql-test/suite/heap/blob_tmp_table.test b/mysql-test/suite/heap/blob_tmp_table.test new file mode 100644 index 0000000000000..009b02e45183e --- /dev/null +++ b/mysql-test/suite/heap/blob_tmp_table.test @@ -0,0 +1,78 @@ +--source include/not_embedded.inc + +--echo # +--echo # Blob updates and deletes on a user TEMPORARY MEMORY table. +--echo # +--echo # A non-transactional TEMPORARY table is left out of the lock set +--echo # altogether, so its handle never holds a THR_LOCK. It is not an +--echo # internal table, though, so it still defers its blob chain frees -- +--echo # a combination no other kind of table has, and one a debug build used +--echo # to assert on at the end of every such statement. +--echo # + +CREATE TEMPORARY TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1,'one'), (2,REPEAT('two',500)), (3,REPEAT('three',500)); + +--echo # UPDATE parks one chain per changed blob +UPDATE t1 SET b=REPEAT('x',4000) WHERE a=2; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +CHECK TABLE t1; + +--echo # DELETE parks the whole row's chain +DELETE FROM t1 WHERE a=3; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +CHECK TABLE t1; + +--echo # Repeated cycles: every parked chain has to come back to the free list +--disable_query_log +let $i= 20; +while ($i) +{ + UPDATE t1 SET b=REPEAT('y',4000) WHERE a=2; + UPDATE t1 SET b=REPEAT('z',8000) WHERE a=2; + DELETE FROM t1 WHERE a=1; + INSERT INTO t1 VALUES (1,REPEAT('one',500)); + dec $i; +} +--enable_query_log +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +CHECK TABLE t1; + +--echo # Multi-table UPDATE and DELETE across two TEMPORARY tables +CREATE TEMPORARY TABLE t2 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t2 VALUES (1,REPEAT('u',4000)), (2,REPEAT('v',4000)); +UPDATE t1, t2 SET t1.b=REPEAT('p',4000), t2.b=REPEAT('q',4000) + WHERE t1.a=t2.a; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +SELECT a, LENGTH(b), LEFT(b,5) FROM t2 ORDER BY a; +CHECK TABLE t1, t2; + +DELETE t1, t2 FROM t1, t2 WHERE t1.a=t2.a AND t1.a=1; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +SELECT a, LENGTH(b), LEFT(b,5) FROM t2 ORDER BY a; +CHECK TABLE t1, t2; + +DROP TEMPORARY TABLE t1, t2; + +--echo # +--echo # The same statements on a non-temporary MEMORY table, which does hold +--echo # the lock while it parks and redeems at unlock time. +--echo # + +CREATE TABLE t1 (a INT, b BLOB) ENGINE=MEMORY; +INSERT INTO t1 VALUES (1,'one'), (2,REPEAT('two',500)), (3,REPEAT('three',500)); +UPDATE t1 SET b=REPEAT('x',4000) WHERE a=2; +DELETE FROM t1 WHERE a=3; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +CHECK TABLE t1; + +--echo # Under LOCK TABLES the redemption happens in ha_heap::reset() instead, +--echo # with the lock still held +LOCK TABLES t1 WRITE; +UPDATE t1 SET b=REPEAT('w',4000) WHERE a=2; +DELETE FROM t1 WHERE a=1; +UNLOCK TABLES; +SELECT a, LENGTH(b), LEFT(b,5) FROM t1 ORDER BY a; +CHECK TABLE t1; + +DROP TABLE t1; diff --git a/storage/heap/CMakeLists.txt b/storage/heap/CMakeLists.txt index d0dc06a6c5823..4e75796778d8e 100644 --- a/storage/heap/CMakeLists.txt +++ b/storage/heap/CMakeLists.txt @@ -34,7 +34,7 @@ IF(WITH_UNIT_TESTS) TARGET_LINK_LIBRARIES(hp_test2 heap mysys dbug strings) MY_ADD_TESTS(hp_test_hash hp_test_scan hp_test_freelist hp_test_concurrent hp_test_block_size hp_test_blob_alias hp_test_update - hp_test_write_dup + hp_test_write_dup hp_test_unlock_check LINK_LIBRARIES heap mysys dbug strings) INCLUDE_DIRECTORIES(${CMAKE_SOURCE_DIR}/sql diff --git a/storage/heap/ha_heap.cc b/storage/heap/ha_heap.cc index f6694ba1ae3b8..6433d69883e72 100644 --- a/storage/heap/ha_heap.cc +++ b/storage/heap/ha_heap.cc @@ -499,6 +499,31 @@ int ha_heap::extra(enum ha_extra_function operation) int ha_heap::reset() { + /* + heap_reset() redeems the blob chains parked by a deferred free, which puts + records back on the shared free list. That is a write to HP_SHARE, so it + needs the THR_LOCK -- unless no other connection can reach the share at + all, which is the case for a TEMPORARY table. + + Only heap_delete() and heap_update() park, and only for a table that is + not internal, an internal one being never binlogged and so free to drop + its chains outright. `internal' is HA_OPEN_INTERNAL_TABLE, the + optimizer's own temporary table, which is not the set the server locks: + per sql/lock.cc, a non-transactional TEMPORARY table is never passed to + thr_multi_lock() and never reaches external_lock(F_UNLCK), so a user + TEMPORARY table parks its chains while holding nothing. It owns them + alone -- no other connection can reach the share -- and this call, reached + from mark_tmp_table_as_free_for_reuse() at the end of every statement, is + what redeems them. + + Every other table does hold the lock while it parks, and keeps holding it + until the chains are back: external_lock(F_UNLCK) redeems them for an + ordinary statement, and this call does for a statement under LOCK TABLES, + reached from mark_used_tables_as_free_for_reuse() with the lock still + held. By the time close_thread_table() gets here there is nothing left. + */ + DBUG_ASSERT(!file->has_pending_blob_free || hp_lock_is_held(file) || + table->s->tmp_table != NO_TMP_TABLE); return heap_reset(file); } @@ -528,22 +553,54 @@ int ha_heap::reset_auto_increment(ulonglong value) int ha_heap::external_lock(THD *thd, int lock_type) { #if !defined(DBUG_OFF) && defined(EXTRA_HEAP_DEBUG) - /* - A table already marked crashed is knowingly inconsistent; every data - access on it fails with HA_ERR_CRASHED, so re-detecting the damage - here would only raise a second error into a diagnostics area that - can already be OK (e.g. after UNLOCK TABLES) and fire the - Diagnostics_area assertion. - */ - if (lock_type == F_UNLCK && file->s->changed && - !heap_is_crashed(file->s) && heap_check_heap(file, 0)) + /* See hp_may_check_heap_on_unlock() for when this is safe to run at all */ + if (lock_type == F_UNLCK && hp_may_check_heap_on_unlock(file) && + heap_check_heap(file, 0)) return HA_ERR_CRASHED; #endif if (lock_type != F_UNLCK && heap_is_crashed(file->s)) return HA_ERR_CRASHED; if (lock_type == F_UNLCK) + { + /* + Redeeming the parked chains puts records back on the shared free list, + which needs the THR_LOCK -- see hp_test_concurrent-t.c, which reproduces + the del_link corruption that flushing without it causes -- unless no + other connection can reach the share at all, the same escape + ha_heap::reset() takes for a temporary table. + + A chain is parked only by heap_delete()/heap_update(), so only by a + handle that could write the share, and the description in sql/lock.cc + has the unlock order -- external_lock(F_UNLCK) first, thr_multi_unlock() + after -- so a handle that took the lock through the SQL layer still + holds it here. The two paths that reach F_UNLCK holding nothing -- + lock_external() unwinding what it had locked, and mysql_lock_tables() + balancing it after thr_multi_lock() failed -- never ran a row operation, + so they have nothing parked and this is a no-op for them. + + The temporary table is the ALTER TABLE copy target, which + copy_data_between_tables() locks with a direct ha_external_lock() + instead of through the lock set. thr_lock() never grants that handle + anything, so hp_lock_is_held() is false on it even while it writes; an + online ALTER replays concurrent deletes onto it, which park. Its share + is private to the ALTER, so redeeming without the lock is safe. + */ + DBUG_ASSERT(!file->has_pending_blob_free || hp_lock_is_held(file) || + table->s->tmp_table != NO_TMP_TABLE); hp_flush_pending_blob_free(file); + hp_lock_released(file); + } + else + { + /* + Every grant is released above before the next request is made, so a + request can never find one left over from the request before it. Were + that to happen, hp_lock_is_held() would report a lock this handle does + not hold, and the verification above would scan HP_SHARE unlocked. + */ + DBUG_ASSERT(!hp_lock_is_held(file)); + } return 0; // No external locking } diff --git a/storage/heap/heapdef.h b/storage/heap/heapdef.h index f157bcc77e9b9..e58e0cccbf2ef 100644 --- a/storage/heap/heapdef.h +++ b/storage/heap/heapdef.h @@ -391,6 +391,80 @@ static inline void hp_flush_pending_blob_free(HP_INFO *info) hp_flush_pending_blob_free_impl(info); } +/* + A grant recorded by hp_lock_granted() is over. + + Called from ha_heap::external_lock(F_UNLCK), which per the description in + sql/lock.cc the SQL layer reaches while the THR_LOCK is still held, just + before thr_multi_unlock(). + + F_UNLCK is also reached on paths where this handle was never granted + anything (see hp_lock_is_held()), where clearing an already clear record is + simply a no-op. +*/ + +static inline void hp_lock_released(HP_INFO *info) +{ + info->lock_granted= 0; +} + +/* + Does this handle hold its table's THR_LOCK? + + ha_heap::external_lock() is reached with F_UNLCK on three paths that look + identical to the handler. Only the first of them still holds the lock: + + - mysql_unlock_tables() calls unlock_external() before thr_multi_unlock(); + - mysql_lock_tables() calls unlock_external() to balance the external + locks it already took, because thr_multi_lock() failed; + - lock_external() unwinds the tables it has already locked, because a + later table refused -- all before thr_multi_lock() runs at all. + ha_partition::external_lock() unwinds its partitions the same way. + + The description in sql/lock.cc covers the first and the third; the second is + only in its code, where a non-zero thr_multi_lock() return sends + mysql_lock_tables() through unlock_external(). + + The requested lock type cannot tell them apart on its own, because + ha_heap::store_lock() records it at get_lock_data() time, before anything is + locked: on the last path it is set while nothing is held. Only an actual + grant can, which is what hp_lock_granted() records. The type is still + needed as well, for the lock thr_multi_lock() takes and then rolls back when + a later table times out: thr_unlock() resets the type but cannot reach the + record, which stands until ha_heap::external_lock(F_UNLCK) clears it. + + A HEAP table has no row-level concurrency control: everything shared through + HP_SHARE is protected by the THR_LOCK alone. Anything that reads or writes + the share therefore has to ask this first. +*/ + +static inline my_bool hp_lock_is_held(const HP_INFO *info) +{ + return info->lock_granted && info->lock.type != TL_UNLOCK; +} + +/* + May ha_heap::external_lock(F_UNLCK) verify the table with heap_check_heap()? + + A scan taken outside the THR_LOCK sees a writer's intermediate state: + hp_alloc_from_tail() publishes total_records at allocation time, before the + slot is written. The scan then either counts a slot the writer has not + filled in yet or races the counters it compares against, and reports damage + that is not there. Since heap_check_heap() marks the share crashed, that + false positive poisons a healthy table for every connection using it. + + A table already marked crashed is knowingly inconsistent; every data access + on it fails with HA_ERR_CRASHED, so re-detecting the damage here would only + raise a second error into a diagnostics area that can already be OK (e.g. + after UNLOCK TABLES) and fire the Diagnostics_area assertion. +*/ + +static inline my_bool hp_may_check_heap_on_unlock(const HP_INFO *info) +{ + return (hp_lock_is_held(info) && info->s->changed && + !heap_is_crashed(info->s)); +} + /* Does a record's blob data live in `chain`? diff --git a/storage/heap/hp_create.c b/storage/heap/hp_create.c index 8bdc08bb36908..bb3871089f6b8 100644 --- a/storage/heap/hp_create.c +++ b/storage/heap/hp_create.c @@ -22,6 +22,31 @@ static void init_block(HP_BLOCK *block, size_t reclength, ulong min_records, ulong max_records); +/* + THR_LOCK grant callback. + + The only thing HEAP wants from it is the fact that it was called: a handle + may not touch HP_SHARE until thr_lock() has actually given it the lock, and + the requested lock type says nothing about that (see hp_lock_is_held()). + Never fails, so a grant is never turned into THR_LOCK_ABORTED. + + Set rather than counted, because thr_lock() does not call this once per + ha_heap::external_lock(): a delayed insert is granted TL_WRITE_DELAYED and + then calls here a second time on the same THR_LOCK_DATA when + thr_upgrade_write_delay_lock() turns it into a real write lock, with no + external_lock() in between. A count would keep the surplus for the life of + the handle; setting is idempotent, and hp_lock_released() clears it from + ha_heap::external_lock(F_UNLCK). +*/ + +static my_bool hp_lock_granted(void *param, + my_bool concurrent_insert __attribute__((unused))) +{ + ((HP_INFO*) param)->lock_granted= 1; + return 0; +} + + /* In how many parts are we going to do allocations of memory and indexes If we assign 1M to the heap table memory, we will allocate roughly @@ -305,6 +330,7 @@ int heap_create(const char *name, HP_CREATE_INFO *create_info, if (!create_info->internal_table) { thr_lock_init(&share->lock); + share->lock.get_status= hp_lock_granted; share->open_list.data= (void*) share; heap_share_list= list_add(heap_share_list,&share->open_list); } diff --git a/storage/heap/hp_open.c b/storage/heap/hp_open.c index c512b88efd369..fe06d463af1ef 100644 --- a/storage/heap/hp_open.c +++ b/storage/heap/hp_open.c @@ -39,7 +39,7 @@ HP_INFO *heap_open_from_share(HP_SHARE *share, int mode) DBUG_RETURN(0); } share->open_count++; - thr_lock_data_init(&share->lock,&info->lock,NULL); + thr_lock_data_init(&share->lock,&info->lock,info); info->s= share; info->lastkey= (uchar*) (info + 1); info->recbuf= (uchar*) (info->lastkey + share->max_key_length); diff --git a/storage/heap/hp_test_unlock_check-t.c b/storage/heap/hp_test_unlock_check-t.c new file mode 100644 index 0000000000000..0b1d683e100df --- /dev/null +++ b/storage/heap/hp_test_unlock_check-t.c @@ -0,0 +1,342 @@ +/* + Unit test: the unlock-time table verification must not run when the + THR_LOCK is not held. + + ha_heap::external_lock(F_UNLCK) verifies the table with heap_check_heap(). + That is safe on the ordinary unlock path, which sql/lock.cc describes as + external_lock(F_UNLCK) followed by thr_multi_unlock(), so the THR_LOCK is + still held. It is not safe on either path that unlocks after a failed lock + attempt: when mysql_lock_tables() balances the external locks it took + because thr_multi_lock() timed out, and when lock_external() itself unwinds + the tables it already locked because a later table refused -- sql/lock.cc + spells out the second of those, the first is only in its code. In both the + caller holds nothing, while another connection is writing. + + The test drives the real lock protocol (thr_multi_lock/thr_multi_unlock, + exactly what sql/lock.cc calls) rather than reproducing a race by repetition. + A holder thread takes TL_WRITE and keeps it until told to let go, so the + contending request is guaranteed to time out; the holder also parks the share + in the state a writer passes through mid-row, so the verification has + something to (wrongly) find. Every outcome here is forced, not raced. +*/ + +#include "hp_test_helpers.h" +#include + +/* Holder-thread handshake. Both flags are set under state_mutex. */ +static pthread_mutex_t state_mutex; +static pthread_cond_t state_cond; +static int holder_ready; /* lock taken, share parked */ +static int holder_release; /* main is done, let go */ + +static HP_INFO *holder_info; + +/* + The slot the holder allocated but has not written yet. Handed back to main + so it can undo the parked state after the holder is gone. +*/ +static uchar *parked_slot; + + +static void set_flag(int *flag) +{ + pthread_mutex_lock(&state_mutex); + *flag= 1; + pthread_cond_broadcast(&state_cond); + pthread_mutex_unlock(&state_mutex); +} + + +static void wait_flag(int *flag) +{ + pthread_mutex_lock(&state_mutex); + while (!*flag) + pthread_cond_wait(&state_cond, &state_mutex); + pthread_mutex_unlock(&state_mutex); +} + + +/* + Put a handle in the state the SQL layer leaves it in just before the + THR_LOCK is attempted: ha_heap::store_lock() has recorded the requested type + (at get_lock_data() time, well before any locking), and lock_external() has + reached ha_heap::external_lock() with that type. Nothing is held yet. +*/ + +static void request_lock(HP_INFO *info, enum thr_lock_type type) +{ + info->lock.type= type; /* ha_heap::store_lock() */ +} + + +/* + Release the handle in the order sql/lock.cc documents: mysql_unlock_tables() + reaches ha_heap::external_lock(F_UNLCK) while the THR_LOCK is still held, and + only then calls thr_multi_unlock(). Call this before thr_multi_unlock(), + never after, or the test stops mirroring the protocol it is here to police. +*/ + +static void release_lock(HP_INFO *info) +{ + hp_lock_released(info); /* ha_heap::external_lock() */ +} + + +/* + Reproduce the state heap_write() is in between allocating a slot and marking + it visible: next_free_record_pos() has already published the slot in + total_records, but the record has not been stored yet. + + The slot is zeroed rather than left as it comes from my_malloc() so that the + test asserts on a defined outcome. A zero flags byte is what a scan of a + half-written row legitimately sees; leaving the malloc garbage in place is + what makes the same access an uninitialised read under MSAN. +*/ + +static uchar *park_mid_write(HP_SHARE *share) +{ + uchar *pos= next_free_record_pos(share); + if (pos) + memset(pos, 0, share->block.recbuffer); + return pos; +} + + +static void unpark_mid_write(HP_SHARE *share, uchar *pos) +{ + hp_push_free_record(share, pos); + hp_shrink_tail(share); +} + + +static void *holder_thread(void *arg __attribute__((unused))) +{ + THR_LOCK_INFO owner; + THR_LOCK_DATA *lock_data[1]; + + my_thread_init(); + thr_lock_info_init(&owner, my_thread_var); + + request_lock(holder_info, TL_WRITE); + lock_data[0]= &holder_info->lock; + + if (thr_multi_lock(lock_data, 1, &owner, 0) != THR_LOCK_SUCCESS) + { + /* Nothing else holds the lock yet, so this cannot fail */ + set_flag(&holder_ready); + my_thread_end(); + return NULL; + } + + parked_slot= park_mid_write(holder_info->s); + + set_flag(&holder_ready); + wait_flag(&holder_release); + + release_lock(holder_info); + thr_multi_unlock(lock_data, 1, 0); + my_thread_end(); + return NULL; +} + + +int main(int argc __attribute__((unused)), + char **argv __attribute__((unused))) +{ + HP_SHARE *share, *share2; + HP_INFO *info1, *info2, *other; + const char *contended; + THR_LOCK_INFO waiter; + THR_LOCK_DATA *waiter_lock_data[2]; + enum enum_thr_lock_result lock_result; + pthread_t holder; + uchar rec[REC_LENGTH]; + uchar blob_data[200]; + int i; + + plan(17); + MY_INIT("hp_test_unlock_check-t"); + pthread_mutex_init(&state_mutex, NULL); + pthread_cond_init(&state_cond, NULL); + + if (create_and_open("test_unlock_check", &share, &info1) || + create_and_open("test_unlock_check2", &share2, &other)) + { + ok(0, "setup failed"); + return exit_status(); + } + + /* + thr_multi_lock() sorts the request array before locking anything, and + LOCK_CMP compares the THR_LOCK address first. Both requests in the + two-table scenario below are TL_WRITE on different shares, so nothing + breaks that tie and the order is decided by where my_malloc() put the two + shares -- which arm of the scenario runs would otherwise vary per run. + + Contend the share that sorts LAST. Then the other table is always + reached and granted before the contended one times out, which is the + granted-then-rolled-back arm. It is the only thing anywhere that + exercises the lock.type half of hp_lock_is_held(): a handle whose grant + is recorded while it holds nothing. The arm this avoids leaves both + halves false and so passes whichever half is deleted. + */ + if (&share->lock > &share2->lock) + contended= "test_unlock_check"; + else + { + HP_SHARE *swap_share= share; + HP_INFO *swap_info= info1; + share= share2; + info1= other; + share2= swap_share; + other= swap_info; + contended= "test_unlock_check2"; + } + + info2= heap_open(contended, 2); + if (!info2) + { + ok(0, "second open failed"); + heap_close(info1); + return exit_status(); + } + heap_extra(info2, HA_EXTRA_NO_READCHECK); + + /* Populate, so that share->changed is set and the scan has work to do */ + for (i= 0; i < 5; i++) + { + memset(blob_data, 'a' + i, sizeof(blob_data)); + build_record(rec, 100 + i, blob_data, (uint16) sizeof(blob_data)); + if (heap_write(info1, rec)) + { + ok(0, "populate failed"); + heap_close(info2); + heap_drop_table(other); + heap_drop_table(info1); + return exit_status(); + } + } + + ok(heap_check_heap(info1, 0) == 0, "table is consistent before the test"); + ok(share->changed != 0, "share is marked changed by the writes"); + + holder_info= info1; + pthread_create(&holder, NULL, holder_thread, NULL); + wait_flag(&holder_ready); + + ok(parked_slot != NULL, "holder parked the share mid-write under TL_WRITE"); + + /* + What sql/lock.cc does: thr_multi_lock() with a timeout, which cannot + succeed because the holder keeps TL_WRITE until we say so. + */ + thr_lock_info_init(&waiter, my_thread_var); + request_lock(info2, TL_WRITE); + waiter_lock_data[0]= &info2->lock; + lock_result= thr_multi_lock(waiter_lock_data, 1, &waiter, 1); + + ok(lock_result == THR_LOCK_WAIT_TIMEOUT, "contending lock request times out"); + + /* + mysql_lock_tables() now calls unlock_external() to balance the external + locks it took, reaching ha_heap::external_lock(F_UNLCK) with no THR_LOCK. + thr_multi_lock() left the request marked TL_UNLOCK, which is how the + handler tells this path from an ordinary unlock. + */ + ok(info2->lock.type == TL_UNLOCK, + "failed lock request is left marked TL_UNLOCK"); + ok(!hp_lock_is_held(info2), "the failed requester knows it holds no lock"); + ok(hp_lock_is_held(info1), "the holder knows it does hold the lock"); + ok(!hp_may_check_heap_on_unlock(info2), + "unlock-time verification is skipped when the lock is not held"); + + /* + Show what running it there would have cost. The holder is still parked + mid-write, so the verification finds the half-written slot and marks the + share crashed -- which is what makes a later INSERT fail with + ER_NOT_KEYFILE even though nothing is wrong with the table. + */ + ok(heap_check_heap(info2, 0) != 0 && heap_is_crashed(share), + "running it anyway reports damage and marks the share crashed"); + + heap_clear_state(share); + + /* Finish the unlock_external() this path performs */ + release_lock(info2); + + /* + A real statement locks more than one table, and a failed request leaves + its tables in two different states. The setup above pinned which is + which: thr_multi_lock() reaches the uncontended table first and grants + it, then times out on the contended one and rolls that grant back. + + The rollback is the shape the predicate exists for. thr_unlock() resets + the request's type but cannot reach the handler's grant record, so the + record stands on a handle that holds nothing, and only the type half of + hp_lock_is_held() answers no. The contended table is the other shape -- + never attempted, and normalized to TL_UNLOCK by the loop in + thr_multi_lock() -- which leaves both halves false. + */ + request_lock(info2, TL_WRITE); + request_lock(other, TL_WRITE); + waiter_lock_data[0]= &info2->lock; + waiter_lock_data[1]= &other->lock; + lock_result= thr_multi_lock(waiter_lock_data, 2, &waiter, 1); + + ok(lock_result == THR_LOCK_WAIT_TIMEOUT, + "multi-table request times out on the contended table"); + ok(other->lock_granted && other->lock.type == TL_UNLOCK, + "the uncontended table was granted and then rolled back"); + ok(!info2->lock_granted, + "the contended table timed out without ever being granted"); + ok(!hp_lock_is_held(info2) && !hp_lock_is_held(other), + "neither table of a failed multi-table request claims the lock"); + + /* + unlock_external() balances every table of the failed request, including + the one thr_multi_lock() granted and then rolled back: thr_unlock() reset + that request's type but left its grant recorded. + */ + release_lock(info2); + release_lock(other); + + /* An uncontended grant, to show the predicate does say yes when it should */ + request_lock(other, TL_WRITE); + waiter_lock_data[0]= &other->lock; + lock_result= thr_multi_lock(waiter_lock_data, 1, &waiter, 1); + + ok(lock_result == THR_LOCK_SUCCESS && hp_lock_is_held(other), + "an uncontended grant is reported as held"); + + release_lock(other); + thr_multi_unlock(waiter_lock_data, 1, 0); + ok(!hp_lock_is_held(other), "the lock is not reported as held once released"); + + /* + The third F_UNLCK path: lock_external() locked this table, a later table + refused, and lock_external() unwinds what it had already locked -- + entirely before thr_multi_lock() runs. The requested type is set and + nothing normalized it, so the type alone cannot tell this apart from an + ordinary unlock. Only the fact that no grant ever arrived can. + */ + request_lock(other, TL_WRITE); + ok(!hp_lock_is_held(other), + "a request unwound before the lock is attempted claims nothing"); + + /* Let the holder finish its row and release the lock */ + set_flag(&holder_release); + pthread_join(holder, NULL); + + unpark_mid_write(share, parked_slot); + + ok(heap_check_heap(info1, 0) == 0, + "the table was consistent all along: the report was a false positive"); + + heap_close(info2); + heap_drop_table(other); + heap_drop_table(info1); + pthread_cond_destroy(&state_cond); + pthread_mutex_destroy(&state_mutex); + my_end(0); + return exit_status(); +}