List:Commits« Previous MessageNext Message »
From:marko.makela Date:December 10 2012 8:35am
Subject:bzr push into mysql-5.6 branch (marko.makela:4739 to 4740) Bug#15925768
View as plain text  
 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#15925768marko.makela10 Dec