Commit 5754382c authored by Marko Mäkelä's avatar Marko Mäkelä

Correct some comments that were added in the fix of Bug #54358

(READ UNCOMMITTED access failure of off-page DYNAMIC or COMPRESSED columns).

Records that lack incompletely written externally stored columns may
be accessed by READ UNCOMMITTED transaction even without involving a
crash during an INSERT or UPDATE operation. I verified this as follows.

(1) added a delay after the mini-transaction for writing the clustered
index 'stub' record was committed (patch attached)
(2) started mysqld in gdb, setting breakpoints to the where the
assertions about READ UNCOMMITTED were added in the bug fix
(3) invoked ibtest3 --create-options=key_block_size=2
to create BLOBs in a COMPRESSED table
(4) invoked the following:
yes 'set transaction isolation level read uncommitted;
checksum table blobt3;select sleep(1);'|mysql -uroot test
(5) noted that one of the breakpoints was triggered
(return(NULL) in btr_rec_copy_externally_stored_field())

=== modified file 'storage/innodb_plugin/row/row0ins.c'
--- storage/innodb_plugin/row/row0ins.c	2010-06-30 08:17:25 +0000
+++ storage/innodb_plugin/row/row0ins.c	2010-06-30 08:17:25 +0000
@@ -2120,6 +2120,7 @@ function_exit:
 		rec_t*	rec;
 		ulint*	offsets;
 		mtr_start(&mtr);
+		os_thread_sleep(5000000);
 
 		btr_cur_search_to_nth_level(index, 0, entry, PAGE_CUR_LE,
 					    BTR_MODIFY_TREE, &cursor, 0,

=== modified file 'storage/innodb_plugin/row/row0upd.c'
--- storage/innodb_plugin/row/row0upd.c	2010-06-30 08:11:55 +0000
+++ storage/innodb_plugin/row/row0upd.c	2010-06-30 08:11:55 +0000
@@ -1763,6 +1763,7 @@ row_upd_clust_rec(
 		rec_offs_init(offsets_);
 
 		mtr_start(mtr);
+		os_thread_sleep(5000000);
 
 		ut_a(btr_pcur_restore_position(BTR_MODIFY_TREE, pcur, mtr));
 		rec = btr_cur_get_rec(btr_cur);
parent 5a25f408
...@@ -4849,20 +4849,10 @@ btr_rec_copy_externally_stored_field( ...@@ -4849,20 +4849,10 @@ btr_rec_copy_externally_stored_field(
if (UNIV_UNLIKELY if (UNIV_UNLIKELY
(!memcmp(data + local_len - BTR_EXTERN_FIELD_REF_SIZE, (!memcmp(data + local_len - BTR_EXTERN_FIELD_REF_SIZE,
field_ref_zero, BTR_EXTERN_FIELD_REF_SIZE))) { field_ref_zero, BTR_EXTERN_FIELD_REF_SIZE))) {
/* The externally stored field was not written /* The externally stored field was not written yet.
yet. This is only a valid condition when the server This record should only be seen by
crashed after the time a record stub was freshly
inserted but before all its columns were written. This
record should only be seen by
recv_recovery_rollback_active() or any recv_recovery_rollback_active() or any
TRX_ISO_READ_UNCOMMITTED transactions. */ TRX_ISO_READ_UNCOMMITTED transactions. */
/* TODO: assert that there is an owner_trx with
owner_trx->id == DB_TRX_ID and owner_trx->is_recovered */
/* TODO: assert that for the current transaction trx,
either (trx == owner_trx && trx_is_recv(trx)) or
trx->isolation_level == TRX_ISO_READ_UNCOMMITTED. */
return(NULL); return(NULL);
} }
......
...@@ -427,11 +427,7 @@ row_sel_fetch_columns( ...@@ -427,11 +427,7 @@ row_sel_fetch_columns(
/* data == NULL means that the /* data == NULL means that the
externally stored field was not externally stored field was not
written yet. This is only a valid written yet. This record
condition when the server crashed
after the time a record stub was
freshly inserted but before all its
columns were written. This record
should only be seen by should only be seen by
recv_recovery_rollback_active() or any recv_recovery_rollback_active() or any
TRX_ISO_READ_UNCOMMITTED TRX_ISO_READ_UNCOMMITTED
...@@ -2744,12 +2740,7 @@ row_sel_store_mysql_rec( ...@@ -2744,12 +2740,7 @@ row_sel_store_mysql_rec(
if (UNIV_UNLIKELY(!data)) { if (UNIV_UNLIKELY(!data)) {
/* The externally stored field /* The externally stored field
was not written yet. This is was not written yet. This
only a valid condition when
the server crashed after the
time a record stub was freshly
inserted but before all its
columns were written. This
record should only be seen by record should only be seen by
recv_recovery_rollback_active() recv_recovery_rollback_active()
or any TRX_ISO_READ_UNCOMMITTED or any TRX_ISO_READ_UNCOMMITTED
...@@ -3623,8 +3614,7 @@ row_search_for_mysql( ...@@ -3623,8 +3614,7 @@ row_search_for_mysql(
if (!row_sel_store_mysql_rec(buf, prebuilt, if (!row_sel_store_mysql_rec(buf, prebuilt,
rec, offsets)) { rec, offsets)) {
/* Only fresh inserts at /* Only fresh inserts may contain
server crash time may contain
incomplete externally stored incomplete externally stored
columns. Pretend that such columns. Pretend that such
records do not exist. Such records do not exist. Such
...@@ -3636,10 +3626,6 @@ row_search_for_mysql( ...@@ -3636,10 +3626,6 @@ row_search_for_mysql(
at a lower level, not here. */ at a lower level, not here. */
ut_a(trx->isolation_level ut_a(trx->isolation_level
== TRX_ISO_READ_UNCOMMITTED); == TRX_ISO_READ_UNCOMMITTED);
/* TODO: assert that there is
an owner_trx with
owner_trx->id == DB_TRX_ID and
owner_trx->is_recovered */
err = DB_TOO_BIG_RECORD; err = DB_TOO_BIG_RECORD;
...@@ -4422,16 +4408,14 @@ row_search_for_mysql( ...@@ -4422,16 +4408,14 @@ row_search_for_mysql(
if (!row_sel_push_cache_row_for_mysql(prebuilt, result_rec, if (!row_sel_push_cache_row_for_mysql(prebuilt, result_rec,
offsets)) { offsets)) {
/* Only fresh inserts at server crash time may contain /* Only fresh inserts may contain incomplete
incomplete externally stored columns. Pretend that externally stored columns. Pretend that such
such records do not exist. Such records may only be records do not exist. Such records may only be
accessed at the READ UNCOMMITTED isolation level or accessed at the READ UNCOMMITTED isolation
when rolling back a recovered transaction. Rollback level or when rolling back a recovered
happens at a lower level, not here. */ transaction. Rollback happens at a lower
level, not here. */
ut_a(trx->isolation_level == TRX_ISO_READ_UNCOMMITTED); ut_a(trx->isolation_level == TRX_ISO_READ_UNCOMMITTED);
/* TODO: assert that there is an owner_trx
with owner_trx->id == DB_TRX_ID and
owner_trx->is_recovered */
} else if (prebuilt->n_fetch_cached } else if (prebuilt->n_fetch_cached
== MYSQL_FETCH_CACHE_SIZE) { == MYSQL_FETCH_CACHE_SIZE) {
...@@ -4449,19 +4433,16 @@ row_search_for_mysql( ...@@ -4449,19 +4433,16 @@ row_search_for_mysql(
} else { } else {
if (!row_sel_store_mysql_rec(buf, prebuilt, if (!row_sel_store_mysql_rec(buf, prebuilt,
result_rec, offsets)) { result_rec, offsets)) {
/* Only fresh inserts at server crash /* Only fresh inserts may contain
time may contain incomplete externally incomplete externally stored
stored columns. Pretend that such columns. Pretend that such records do
records do not exist. Such records may not exist. Such records may only be
only be accessed at the READ UNCOMMITTED accessed at the READ UNCOMMITTED
isolation level or when rolling back a isolation level or when rolling back a
recovered transaction. Rollback happens recovered transaction. Rollback
at a lower level, not here. */ happens at a lower level, not here. */
ut_a(trx->isolation_level ut_a(trx->isolation_level
== TRX_ISO_READ_UNCOMMITTED); == TRX_ISO_READ_UNCOMMITTED);
/* TODO: assert that there is an owner_trx
with owner_trx->id == DB_TRX_ID and
owner_trx->is_recovered */
goto next_rec; goto next_rec;
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment