Skip to content

Commit 6416cd4

Browse files
committed
[RP006] Reverts wrong changes for Adaptive Hash Index
Reverts BAD performance changes for AHI. The Bug #31750840 ADAPTIVE HASH INDEX(AHI) BUILDING CAUSING CONTENTION ON BTR_SEARCH_LATCHES Bug #33601434 InnoDB: AHI-related fields n_fields, n_bytes, left_side should be atomically set are basically wrong. Just intorducing performance regression of AHI... The AHI search is optimistic trial before normal b-tree search. If succeeds, can skip normal b-tree search. So, the most important for AHI performance is "turn-around time", has priority over "concurrent scale" or "search consistency", to minimize the trial overhead and basically limited concurrency of hash indexes. AHI limits concurrency for minimize each waiting time of "turn-around time", basically. But the BAD changes increased "turn-around time" much, and cannot get concurrency enogh than that. Just introducing regression in total. And AHI has enough result check at the end of search, and fail to search is acceptable. But the BAD changes introduced useless cosistency keeping and its overhead for "turn-around time". In addition, the BAD changes seem to contain bad code for some kinds of PGO build. Even non-AHI path, performance regression detected, lower throughput than normal build. Such no merit and only risky code should be eliminated. Reverts the followings whole as possible as we can. > commit 78e695d > Author: Marcin Babij <marcin.babij@oracle.com> > Date: Fri Apr 1 00:29:48 2022 +0200 > > Bug #31750840 ADAPTIVE HASH INDEX(AHI) BUILDING CAUSING CONTENTION ON BTR_SEARCH_LATCHES > commit d7114b1 > Author: Marcin Babij <marcin.babij@oracle.com> > Date: Fri Aug 12 17:01:16 2022 +0200 > > Bug #33601434 InnoDB: AHI-related fields n_fields, n_bytes, left_side should be atomically set and also reverted bug fix introduced by the above BAD changes. Because not needed now. > commit 1392643 > Author: Yasufumi Kinoshita <yasufumi.kinoshita@oracle.com> > Date: Mon Dec 12 16:26:57 2022 +0900 > > Bug#34544595: btr_search_hash_table_validate() has race condition with btr_search_disable() > commit 8c944fc > Author: Ramakrishnan Kamalakannan <ramakrishnan.kamalakannan@oracle.com> > Date: Fri Aug 18 08:49:34 2023 +0200 > > Bug#35037114 Server crash in old_index != nullptr at my_server_abort
1 parent 3e95fbb commit 6416cd4

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+1873
-2058
lines changed

mysql-test/suite/innodb/r/disable_ahi_other_blocks.result

Lines changed: 0 additions & 42 deletions
This file was deleted.

mysql-test/suite/innodb/t/disable_ahi_other_blocks-master.opt

Lines changed: 0 additions & 1 deletion
This file was deleted.

mysql-test/suite/innodb/t/disable_ahi_other_blocks.test

Lines changed: 0 additions & 78 deletions
This file was deleted.

share/messages_to_error_log.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6075,8 +6075,8 @@ ER_IB_MSG_179
60756075
ER_IB_MSG_180
60766076
eng "%s"
60776077

6078-
ER_IB_LONG_AHI_DISABLE_WAIT
6079-
eng "Waited for %u secs for hash index ref_count (%zu) to drop to 0. index: \"%s\" table: \"%s\""
6078+
ER_IB_MSG_181
6079+
eng "%s"
60806080

60816081
ER_IB_MSG_182
60826082
eng "%s"

storage/innobase/btr/btr0btr.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,7 +1552,7 @@ rec_t *btr_root_raise_and_insert(
15521552
lock_prdt_rec_move(new_block, root_block);
15531553
}
15541554

1555-
btr_search_update_hash_on_move(new_block, root_block, index);
1555+
btr_search_move_or_delete_hash_entries(new_block, root_block, index);
15561556
}
15571557

15581558
/* If this is a pessimistic insert which is actually done to
@@ -2508,7 +2508,7 @@ rec_t *btr_page_split_and_insert(
25082508
new_page + PAGE_NEW_INFIMUM);
25092509
}
25102510

2511-
btr_search_update_hash_on_move(new_block, block, cursor->index);
2511+
btr_search_move_or_delete_hash_entries(new_block, block, cursor->index);
25122512

25132513
/* Delete the records from the source page. */
25142514

@@ -2549,7 +2549,7 @@ rec_t *btr_page_split_and_insert(
25492549

25502550
ut_ad(!dict_index_is_spatial(index));
25512551

2552-
btr_search_update_hash_on_move(new_block, block, cursor->index);
2552+
btr_search_move_or_delete_hash_entries(new_block, block, cursor->index);
25532553

25542554
/* Delete the records from the source page. */
25552555

@@ -2936,7 +2936,7 @@ static buf_block_t *btr_lift_page_up(
29362936
lock_prdt_rec_move(father_block, block);
29372937
}
29382938

2939-
btr_search_update_hash_on_move(father_block, block, index);
2939+
btr_search_move_or_delete_hash_entries(father_block, block, index);
29402940
}
29412941

29422942
if (!dict_table_is_locking_disabled(index->table)) {

storage/innobase/btr/btr0cur.cc

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -688,6 +688,7 @@ void btr_cur_search_to_nth_level(
688688

689689
DBUG_TRACE;
690690

691+
btr_search_t *info;
691692
mem_heap_t *heap = nullptr;
692693
ulint offsets_[REC_OFFS_NORMAL_SIZE];
693694
ulint *offsets = offsets_;
@@ -770,13 +771,15 @@ void btr_cur_search_to_nth_level(
770771
cursor->flag = BTR_CUR_BINARY;
771772
cursor->index = index;
772773

774+
info = btr_search_get_info(index);
775+
773776
#ifdef UNIV_SEARCH_PERF_STAT
774777
info->n_searches++;
775778
#endif
776779
/* Use of AHI is disabled for intrinsic table as these tables re-use
777780
the index-id and AHI validation is based on index-id. */
778781
if (rw_lock_get_writer(btr_get_search_latch(index)) == RW_LOCK_NOT_LOCKED &&
779-
latch_mode <= BTR_MODIFY_LEAF && index->search_info->last_hash_succ &&
782+
latch_mode <= BTR_MODIFY_LEAF && info->last_hash_succ &&
780783
!index->disable_ahi && !estimate
781784
#ifdef PAGE_CUR_LE_OR_EXTENDS
782785
&& mode != PAGE_CUR_LE_OR_EXTENDS
@@ -786,7 +789,7 @@ void btr_cur_search_to_nth_level(
786789
btr_search_enabled below, and btr_search_guess_on_hash()
787790
will have to check it again. */
788791
&& UNIV_LIKELY(btr_search_enabled) && !modify_external &&
789-
btr_search_guess_on_hash(tuple, mode, latch_mode, cursor,
792+
btr_search_guess_on_hash(index, info, tuple, mode, latch_mode, cursor,
790793
has_search_latch, mtr)) {
791794

792795
/* Search using the hash index succeeded */
@@ -958,10 +961,10 @@ void btr_cur_search_to_nth_level(
958961
retry_page_get:
959962
ut_ad(n_blocks < BTR_MAX_LEVELS);
960963
tree_savepoints[n_blocks] = mtr_set_savepoint(mtr);
961-
block = buf_page_get_gen(
962-
page_id, page_size, rw_latch,
963-
(height == ULINT_UNDEFINED ? index->search_info->root_guess : nullptr),
964-
fetch, {file, line}, mtr);
964+
block =
965+
buf_page_get_gen(page_id, page_size, rw_latch,
966+
(height == ULINT_UNDEFINED ? info->root_guess : nullptr),
967+
fetch, {file, line}, mtr);
965968

966969
tree_blocks[n_blocks] = block;
967970

@@ -1130,7 +1133,7 @@ void btr_cur_search_to_nth_level(
11301133
rtr_get_mbr_from_tuple(tuple, &cursor->rtr_info->mbr);
11311134
}
11321135

1133-
index->search_info->root_guess = block;
1136+
info->root_guess = block;
11341137
}
11351138

11361139
if (height == 0) {
@@ -1657,7 +1660,7 @@ void btr_cur_search_to_nth_level(
16571660
btr_search_build_page_hash_index() before building a
16581661
page hash index, while holding search latch. */
16591662
if (btr_search_enabled && !index->disable_ahi) {
1660-
btr_search_info_update(cursor);
1663+
btr_search_info_update(index, cursor);
16611664
}
16621665
ut_ad(cursor->up_match != ULINT_UNDEFINED || mode != PAGE_CUR_GE);
16631666
ut_ad(cursor->up_match != ULINT_UNDEFINED || mode != PAGE_CUR_LE);
@@ -3337,6 +3340,7 @@ dberr_t btr_cur_update_in_place(ulint flags, btr_cur_t *cursor, ulint *offsets,
33373340
rec_t *rec;
33383341
roll_ptr_t roll_ptr = 0;
33393342
bool was_delete_marked;
3343+
bool is_hashed;
33403344

33413345
rec = btr_cur_get_rec(cursor);
33423346
index = cursor->index;
@@ -3394,7 +3398,9 @@ dberr_t btr_cur_update_in_place(ulint flags, btr_cur_t *cursor, ulint *offsets,
33943398
was_delete_marked =
33953399
rec_get_deleted_flag(rec, page_is_comp(buf_block_get_frame(block)));
33963400

3397-
if (block->ahi.index.load() != nullptr) {
3401+
is_hashed = (block->index != nullptr);
3402+
3403+
if (is_hashed) {
33983404
/* TO DO: Can we skip this if none of the fields
33993405
index->search_info->curr_n_fields
34003406
are being updated? */
@@ -3409,11 +3415,17 @@ dberr_t btr_cur_update_in_place(ulint flags, btr_cur_t *cursor, ulint *offsets,
34093415
/* Remove possible hash index pointer to this record */
34103416
btr_search_update_hash_on_delete(cursor);
34113417
}
3418+
3419+
rw_lock_x_lock(btr_get_search_latch(index), UT_LOCATION_HERE);
34123420
}
34133421

3414-
block->ahi.validate();
3422+
assert_block_ahi_valid(block);
34153423
row_upd_rec_in_place(rec, index, offsets, update, page_zip);
34163424

3425+
if (is_hashed) {
3426+
rw_lock_x_unlock(btr_get_search_latch(index));
3427+
}
3428+
34173429
btr_cur_update_in_place_log(flags, rec, index, update, trx_id, roll_ptr, mtr);
34183430

34193431
if (was_delete_marked &&

0 commit comments

Comments
 (0)