Details
-
Task
-
Status: Closed (View Workflow)
-
Minor
-
Resolution: Fixed
-
None
Description
Currently, btr_cur_n_non_sea is used to track the search that missed
adaptive hash index. adaptive hash index is turned off by default
but the said variable is updated always though the value of it makes sense
only when an adaptive index is enabled. It is meant to check how many
searches didn't go through an adaptive hash index.
Given a global variable that is updated on each search path it causes
a contention with a multi-threaded workload.
Patch moves the said variables inside a loop that is now updated
only when the adaptive hash index is enabled and that in theory should
also, reduce the update frequency of the said variable as the majority of
the request should be serviced through the adaptive hash index.
Attachments
Issue Links
- relates to
-
MDEV-25911 Document innodb_adaptive_hash_index=OFF changes
-
- Closed
-
-
-
- Closed
-
Activity
Marko,
Based on our discussion and your suggestion I have revamped the complete patch. Updated patch uploaded to PR and uploading the benchmarking result here.
| Field | Original Value | New Value |
|---|---|---|
| Attachment | updatedpatch.png [ 57998 ] |
| Summary | Using distributed counter to track number of b-tree (non-adaptive) searches | Statistics used to track b-tree (non-adaptive) searches should be updated only when adaptive hashing is turned-on |
| Description |
Currently, btr_cur_n_non_sea is used to track the search that missed adaptive hash.
The count is meant for statistics only and no decision is taken based on the said count. Originally it was ulint that was eventually converted to Atomic_counter<ulint> probably to make it cache aligned but while experimenting with 10.6 it was discovered that the said counter continues to act as a bottleneck due to multi-core sharing. Switching to distributed counter ib_counter_t<ulint, ib_counter_element_t> helped reduced the contention and gains of 4-40% are observed for write workload. Check the attached graph for more performance details. |
Currently, btr_cur_n_non_sea is used to track the search that missed
adaptive hash index. adaptive hash index is turned off by default but the said variable is updated always though the value of it makes sense only when an adaptive index is enabled. It is meant to check how many searches didn't go through an adaptive hash index. Given a global variable that is updated on each search path it causes a contention with a multi-threaded workload. Patch moves the said variables inside a loop that is now updated only when the adaptive hash index is enabled and that in theory should also, reduce the update frequency of the said variable as the majority of the request should be serviced through the adaptive hash index. |
| Attachment | updatedpatch.png [ 57998 ] |
| Attachment | btr_cur_n_non_sea.png [ 57995 ] |
| Attachment | arm-x86-btr_cur_n_non_sea.png [ 58025 ] |
Marko,
Based on our discussion and taking the approach further we have now moved the said btr_cur_n_non_sea to make it specific only when ahi is enabled.
Performance: with AHI enabled effect is clearly seen with 3-9% performance improvement.
With AHI disabled (cmake -DWITH_INNODB_AHI=OFF) effect is more wide spread since there is a global variable is not being updated anymore.
I have attached the graph.
greenman, please document the user-visible change:
When innodb_adaptive_hash_index=OFF (the default), the following counters (which reflect btr_cur_n_non_sea) will no longer be updated:
- adaptive_hash_index in INFORMATION_SCHEMA.INNODB_METRICS
- Innodb_adaptive_hash_non_hash_searches in INFORMATION_SCHEMA.GLOBAL_STATUS
| issue.field.resolutiondate | 2021-06-11 09:34:51.0 | 2021-06-11 09:34:51.22 |
| Fix Version/s | 10.6.2 [ 25800 ] | |
| Assignee | Marko Mäkelä [ marko ] | |
| Resolution | Fixed [ 1 ] | |
| Status | Open [ 1 ] | Closed [ 6 ] |
| Link |
This issue relates to |
| Workflow | MariaDB v3 [ 122549 ] | MariaDB v4 [ 134448 ] |
| Link |
This issue relates to |
That counter is exported as adaptive_hash_non_hash_searches, unless the server is built with cmake -DWITH_INNODB_AHI=OFF to disable all code related to the adaptive hash index:
#ifdef BTR_CUR_HASH_ADAPTMaybe we could disable the updates of this counter when the adaptive hash index is disabled? I would think that the difference of these two counters could be interesting, in determining whether the adaptive hash index is useful at all. Do we expect that this counter would be used for any other purpose? Could we do something like this (along with declaring the variable only in cmake -DWITH_INNODB_AHI=ON builds)?
diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.ccindex 723096bd08c..81b5f4b4a74 100644--- a/storage/innobase/btr/btr0cur.cc+++ b/storage/innobase/btr/btr0cur.cc@@ -1411,7 +1411,8 @@ btr_cur_search_to_nth_level_func(# ifdef UNIV_SEARCH_PERF_STATinfo->n_searches++;# endif- if (autoinc == 0+ if (!btr_search_enabled) {+ } else if (autoinc == 0&& latch_mode <= BTR_MODIFY_LEAF&& info->last_hash_succ# ifdef MYSQL_INDEX_DISABLE_AHI@@ -1425,7 +1426,6 @@ btr_cur_search_to_nth_level_func(/* If !ahi_latch, we do a dirty read ofbtr_search_enabled below, and btr_search_guess_on_hash()will have to check it again. */- && btr_search_enabled&& !modify_external&& !(tuple->info_bits & REC_INFO_MIN_REC_FLAG)&& btr_search_guess_on_hash(index, info, tuple, mode,@@ -1443,10 +1443,11 @@ btr_cur_search_to_nth_level_func(btr_cur_n_sea++;DBUG_RETURN(err);+ } else {+ btr_cur_n_non_sea++;}# endif /* BTR_CUR_HASH_ADAPT */#endif /* BTR_CUR_ADAPT */- btr_cur_n_non_sea++;/* If the hash search did not succeed, do binary search down theNote: btr_cur_search_to_nth_level_func() is rather low-level function that can be invoked not only during query execution, but also by the purge of history, and by some low-level operations such as restoring a persistent cursor position.
The adaptive hash index was disabled by default in
MDEV-20487, because often, it is actually reducing performance. But, we do set cmake -DWITH_INNODB_AHI=ON by default at build time.