4740 Marko Mäkelä 2012-12-10
Bug#15925768 PERFORMANCE REGRESSION IN PURGE, ROLLBACK, UPDATE
The fix of the race condition Bug#14756963 introduced a performance
regression. When a secondary index is being created online, it may
transition from ONLINE_INDEX_COMPLETE to ONLINE_INDEX_ABORTED or
ONLINE_INDEX_ABORTED_DROPPED when the ALTER TABLE operation is rolled
back later.
Between prepare_inplace_alter_table and commit_inplace_alter_table,
the secondary index name will start with TEMP_INDEX_PREFIX. Thus, the
index->online_status must always be ONLINE_INDEX_COMPLETE for
secondary indexes whose name does not start with TEMP_INDEX_PREFIX.
(For clustered index, online_status!=ONLINE_INDEX_COMPLETE would
indicate the existence of a table modification log.)
row_purge_remove_sec_if_poss_tree(),
row_purge_remove_sec_if_poss_leaf(),
row_undo_ins_remove_sec_low(),
row_undo_mod_del_mark_or_remove_sec_low(),
row_undo_mod_del_unmark_sec_and_undo_update(),
row_upd_sec_index_entry():
Do not check index->online_status unless index->name starts with
TEMP_INDEX_PREFIX. This should remove the performance regression
without a risk of reintroducing any race condition.
rb:1682 approved by Jimmy Yang
modified:
storage/innobase/row/row0purge.cc
storage/innobase/row/row0umod.cc
storage/innobase/row/row0upd.cc
4739 Shivji Kumar Jha 2012-12-09 [merge]
BUG#12359942 - REPLICATION TEST FROM ENGINE SUITE
RPL_ROW_UNTIL TIMES OUT
patch to fix post push falures in pb2
BUG#15872504 - REMOVE MYSQL-TEST/INCLUDE/GET_BINLOG_DUMP_THREAD_ID.INC
bzr merge 5.5->5.6
removed:
mysql-test/include/get_binlog_dump_thread_id.inc
modified:
mysql-test/suite/engines/funcs/r/rpl_row_until.result
=== modified file 'storage/innobase/row/row0purge.cc'
--- a/storage/innobase/row/row0purge.cc revid:shivji.jha@stripped
+++ b/storage/innobase/row/row0purge.cc revid:marko.makela@oracle.com-20121210081441-8exn0ltn6ykb6scg
@@ -303,14 +303,26 @@ row_purge_remove_sec_if_poss_tree(
log_free_check();
mtr_start(&mtr);
- mtr_x_lock(dict_index_get_lock(index), &mtr);
-
- if (dict_index_is_online_ddl(index)) {
- /* Online secondary index creation will not copy any
- delete-marked records. Therefore there is nothing to
- be purged. We must also skip the purge when a completed
- index is dropped by rollback_inplace_alter_table(). */
- goto func_exit_no_pcur;
+ if (*index->name == TEMP_INDEX_PREFIX) {
+ /* The index->online_status may change if the
+ index->name starts with TEMP_INDEX_PREFIX (meaning
+ that the index is or was being created online). It is
+ protected by index->lock. */
+ mtr_x_lock(dict_index_get_lock(index), &mtr);
+
+ if (dict_index_is_online_ddl(index)) {
+ /* Online secondary index creation will not
+ copy any delete-marked records. Therefore
+ there is nothing to be purged. We must also
+ skip the purge when a completed index is
+ dropped by rollback_inplace_alter_table(). */
+ goto func_exit_no_pcur;
+ }
+ } else {
+ /* For secondary indexes,
+ index->online_status==ONLINE_INDEX_CREATION unless
+ index->name starts with TEMP_INDEX_PREFIX. */
+ ut_ad(!dict_index_is_online_ddl(index));
}
search_result = row_search_index_entry(index, entry, BTR_MODIFY_TREE,
@@ -390,20 +402,38 @@ row_purge_remove_sec_if_poss_leaf(
{
mtr_t mtr;
btr_pcur_t pcur;
+ ulint mode;
enum row_search_result search_result;
bool success = true;
log_free_check();
mtr_start(&mtr);
- mtr_s_lock(dict_index_get_lock(index), &mtr);
- if (dict_index_is_online_ddl(index)) {
- /* Online secondary index creation will not copy any
- delete-marked records. Therefore there is nothing to
- be purged. We must also skip the purge when a completed
- index is dropped by rollback_inplace_alter_table(). */
- goto func_exit_no_pcur;
+ if (*index->name == TEMP_INDEX_PREFIX) {
+ /* The index->online_status may change if the
+ index->name starts with TEMP_INDEX_PREFIX (meaning
+ that the index is or was being created online). It is
+ protected by index->lock. */
+ mtr_s_lock(dict_index_get_lock(index), &mtr);
+
+ if (dict_index_is_online_ddl(index)) {
+ /* Online secondary index creation will not
+ copy any delete-marked records. Therefore
+ there is nothing to be purged. We must also
+ skip the purge when a completed index is
+ dropped by rollback_inplace_alter_table(). */
+ goto func_exit_no_pcur;
+ }
+
+ mode = BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED | BTR_DELETE;
+ } else {
+ /* For secondary indexes,
+ index->online_status==ONLINE_INDEX_CREATION unless
+ index->name starts with TEMP_INDEX_PREFIX. */
+ ut_ad(!dict_index_is_online_ddl(index));
+
+ mode = BTR_MODIFY_LEAF | BTR_DELETE;
}
/* Set the purge node for the call to row_purge_poss_sec(). */
@@ -413,9 +443,7 @@ row_purge_remove_sec_if_poss_leaf(
pcur.btr_cur.thr = static_cast<que_thr_t*>(que_node_get_parent(node));
search_result = row_search_index_entry(
- index, entry,
- BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED | BTR_DELETE,
- &pcur, &mtr);
+ index, entry, mode, &pcur, &mtr);
switch (search_result) {
case ROW_FOUND:
=== modified file 'storage/innobase/row/row0umod.cc'
--- a/storage/innobase/row/row0umod.cc revid:shivji.jha@stripped6-52md7sd5y1zt6k6z
+++ b/storage/innobase/row/row0umod.cc revid:marko.makela@strippedb6scg
@@ -367,16 +367,27 @@ row_undo_mod_del_mark_or_remove_sec_low(
log_free_check();
mtr_start(&mtr);
- if (mode == BTR_MODIFY_LEAF) {
- mode = BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED;
- mtr_s_lock(dict_index_get_lock(index), &mtr);
- } else {
- ut_ad(mode == BTR_MODIFY_TREE);
- mtr_x_lock(dict_index_get_lock(index), &mtr);
- }
+ if (*index->name == TEMP_INDEX_PREFIX) {
+ /* The index->online_status may change if the
+ index->name starts with TEMP_INDEX_PREFIX (meaning
+ that the index is or was being created online). It is
+ protected by index->lock. */
+ if (mode == BTR_MODIFY_LEAF) {
+ mode = BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED;
+ mtr_s_lock(dict_index_get_lock(index), &mtr);
+ } else {
+ ut_ad(mode == BTR_MODIFY_TREE);
+ mtr_x_lock(dict_index_get_lock(index), &mtr);
+ }
- if (row_log_online_op_try(index, entry, 0)) {
- goto func_exit_no_pcur;
+ if (row_log_online_op_try(index, entry, 0)) {
+ goto func_exit_no_pcur;
+ }
+ } else {
+ /* For secondary indexes,
+ index->online_status==ONLINE_INDEX_CREATION unless
+ index->name starts with TEMP_INDEX_PREFIX. */
+ ut_ad(!dict_index_is_online_ddl(index));
}
btr_cur = btr_pcur_get_btr_cur(&pcur);
@@ -528,16 +539,27 @@ row_undo_mod_del_unmark_sec_and_undo_upd
log_free_check();
mtr_start(&mtr);
- if (mode == BTR_MODIFY_LEAF) {
- mode = BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED;
- mtr_s_lock(dict_index_get_lock(index), &mtr);
- } else {
- ut_ad(mode == BTR_MODIFY_TREE);
- mtr_x_lock(dict_index_get_lock(index), &mtr);
- }
+ if (*index->name == TEMP_INDEX_PREFIX) {
+ /* The index->online_status may change if the
+ index->name starts with TEMP_INDEX_PREFIX (meaning
+ that the index is or was being created online). It is
+ protected by index->lock. */
+ if (mode == BTR_MODIFY_LEAF) {
+ mode = BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED;
+ mtr_s_lock(dict_index_get_lock(index), &mtr);
+ } else {
+ ut_ad(mode == BTR_MODIFY_TREE);
+ mtr_x_lock(dict_index_get_lock(index), &mtr);
+ }
- if (row_log_online_op_try(index, entry, trx->id)) {
- goto func_exit_no_pcur;
+ if (row_log_online_op_try(index, entry, trx->id)) {
+ goto func_exit_no_pcur;
+ }
+ } else {
+ /* For secondary indexes,
+ index->online_status==ONLINE_INDEX_CREATION unless
+ index->name starts with TEMP_INDEX_PREFIX. */
+ ut_ad(!dict_index_is_online_ddl(index));
}
search_result = row_search_index_entry(index, entry, mode,
=== modified file 'storage/innobase/row/row0upd.cc'
--- a/storage/innobase/row/row0upd.cc revid:shivji.jha@stripped121209120206-52md7sd5y1zt6k6z
+++ b/storage/innobase/row/row0upd.cc revid:marko.makela@strippedexn0ltn6ykb6scg
@@ -1692,45 +1692,62 @@ row_upd_sec_index_entry(
#endif /* UNIV_DEBUG */
mtr_start(&mtr);
- mtr_s_lock(dict_index_get_lock(index), &mtr);
- switch (dict_index_get_online_status(index)) {
- case ONLINE_INDEX_COMPLETE:
- /* This is a normal index. Do not log anything.
- Perform the update on the index tree directly. */
- break;
- case ONLINE_INDEX_CREATION:
- /* Log a DELETE and optionally INSERT. */
- row_log_online_op(index, entry, 0);
-
- if (!node->is_delete) {
- mem_heap_empty(heap);
- entry = row_build_index_entry(
- node->upd_row, node->upd_ext, index, heap);
- ut_a(entry);
- row_log_online_op(index, entry, trx->id);
+ if (*index->name == TEMP_INDEX_PREFIX) {
+ /* The index->online_status may change if the
+ index->name starts with TEMP_INDEX_PREFIX (meaning
+ that the index is or was being created online). It is
+ protected by index->lock. */
+
+ mtr_s_lock(dict_index_get_lock(index), &mtr);
+
+ switch (dict_index_get_online_status(index)) {
+ case ONLINE_INDEX_COMPLETE:
+ /* This is a normal index. Do not log anything.
+ Perform the update on the index tree directly. */
+ break;
+ case ONLINE_INDEX_CREATION:
+ /* Log a DELETE and optionally INSERT. */
+ row_log_online_op(index, entry, 0);
+
+ if (!node->is_delete) {
+ mem_heap_empty(heap);
+ entry = row_build_index_entry(
+ node->upd_row, node->upd_ext,
+ index, heap);
+ ut_a(entry);
+ row_log_online_op(index, entry, trx->id);
+ }
+ /* fall through */
+ case ONLINE_INDEX_ABORTED:
+ case ONLINE_INDEX_ABORTED_DROPPED:
+ mtr_commit(&mtr);
+ goto func_exit;
}
- /* fall through */
- case ONLINE_INDEX_ABORTED:
- case ONLINE_INDEX_ABORTED_DROPPED:
- mtr_commit(&mtr);
- goto func_exit;
+
+ /* We can only buffer delete-mark operations if there
+ are no foreign key constraints referring to the index. */
+ mode = referenced
+ ? BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED
+ : BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED
+ | BTR_DELETE_MARK;
+ } else {
+ /* For secondary indexes,
+ index->online_status==ONLINE_INDEX_CREATION unless
+ index->name starts with TEMP_INDEX_PREFIX. */
+ ut_ad(!dict_index_is_online_ddl(index));
+
+ /* We can only buffer delete-mark operations if there
+ are no foreign key constraints referring to the index. */
+ mode = referenced
+ ? BTR_MODIFY_LEAF
+ : BTR_MODIFY_LEAF | BTR_DELETE_MARK;
}
/* Set the query thread, so that ibuf_insert_low() will be
able to invoke thd_get_trx(). */
btr_pcur_get_btr_cur(&pcur)->thr = thr;
- /* We can only try to use the insert/delete buffer to buffer
- delete-mark operations if the index we're modifying has no foreign
- key constraints referring to it. */
- if (referenced) {
- mode = BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED;
- } else {
- mode = BTR_MODIFY_LEAF | BTR_ALREADY_S_LATCHED
- | BTR_DELETE_MARK;
- }
-
search_result = row_search_index_entry(index, entry, mode,
&pcur, &mtr);
No bundle (reason: useless for push emails).| Thread |
|---|
| • bzr push into mysql-5.6 branch (marko.makela:4739 to 4740) Bug#15925768 | marko.makela | 10 Dec |
