Skip to content

MDEV-38305: Expose adaptive hash index statistics in ANALYZE FORMAT=JSON#5069

Open
mariadb-TafzeelShams wants to merge 1 commit into
mainfrom
main-MDEV-38305
Open

MDEV-38305: Expose adaptive hash index statistics in ANALYZE FORMAT=JSON#5069
mariadb-TafzeelShams wants to merge 1 commit into
mainfrom
main-MDEV-38305

Conversation

@mariadb-TafzeelShams
Copy link
Copy Markdown
Contributor

  • The Jira issue number for this PR is: MDEV-38305

Description

MDEV-38305: Expose adaptive hash index statistics in ANALYZE FORMAT=JSON

Expose InnoDB's Adaptive Hash Index (AHI) statistics through ANALYZE
FORMAT=JSON output to provide query-level visibility into AHI usage
and effectiveness. This allows DBAs and developers to monitor how well
the adaptive hash index is serving their workloads on a per-query basis.

The r_ahi_stats object (nested inside r_engine_stats) now reports four
key metrics: ahi_searches (successful AHI lookups), ahi_searches_btree
(AHI misses requiring B-tree fallback), ahi_rows_added (rows inserted
into AHI), and ahi_pages_added (pages indexed by AHI).

  • btr_ahi_inc_searches(): Increment counter when AHI lookup succeeds.
  • btr_ahi_inc_searches_btree(): Increment counter when AHI lookup fails
    and falls back to B-tree search.
  • btr_ahi_inc_rows_added(): Increment counter when rows are added to
    the adaptive hash index structure.
  • btr_ahi_inc_pages_added(): Increment counter when new pages are
    indexed by AHI.
  • btr_cur_t::search_leaf(): Call btr_ahi_inc_searches() on successful
    AHI hit and btr_ahi_inc_searches_btree() on AHI miss to track search
    outcomes at the point where AHI is utilized.
  • trace_engine_stats(): Output r_ahi_stats object with all four AHI
    counters in JSON format when any AHI activity is detected during query
    execution.
  • ha_handler_stats: Added ahi_searches, ahi_searches_btree, ahi_rows_added,
    and ahi_pages_added fields to track per-query AHI statistics.
  • ahi_stats.test: Comprehensive verification of AHI statistics reporting
    across different scenarios: insufficient accesses (no AHI build),
    threshold triggering (AHI construction), heavy warmup (full AHI
    utilization), and disabled AHI (verify zero statistics).
  • check_ahi_status.inc: Reusable include file for executing queries with
    configurable warmup repetitions and extracting AHI statistics from
    ANALYZE FORMAT=JSON output using JSON path expressions.

How can this PR be tested?

Added mysql-test/suite/innodb/t/ahi_stats.test to run queries at different stages of AHI and compare the stats of that query.

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request exposes InnoDB Adaptive Hash Index (AHI) statistics, including successful lookups, B-tree fallback searches, and rows/pages added, within the ANALYZE FORMAT=JSON output. The implementation adds new counters to the handler statistics and instruments the InnoDB storage engine to populate them. Feedback suggests initializing the new statistics fields to zero to prevent reporting garbage values and ensuring they are reset between queries to avoid data leakage. Additionally, there is a concern regarding the performance overhead of performing thread-local lookups for statistics on every AHI search, suggesting a potential optimization by passing or caching the transaction pointer in the search path.

Comment thread sql/ha_handler_stats.h
Comment on lines +44 to +47
ulonglong ahi_searches; /* Successful adaptive hash lookups */
ulonglong ahi_searches_btree; /* B-tree searches (AHI miss) */
ulonglong ahi_rows_added; /* Rows added to adaptive hash index */
ulonglong ahi_pages_added; /* Pages added to adaptive hash index */
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The new fields in ha_handler_stats should be initialized to zero to prevent reporting garbage values in ANALYZE FORMAT=JSON. Additionally, ensure that these fields are cleared in the ha_handler_stats::reset() method (and initialized in the constructor if applicable) so that statistics do not leak between different queries or table accesses within the same session.

Suggested change
ulonglong ahi_searches; /* Successful adaptive hash lookups */
ulonglong ahi_searches_btree; /* B-tree searches (AHI miss) */
ulonglong ahi_rows_added; /* Rows added to adaptive hash index */
ulonglong ahi_pages_added; /* Pages added to adaptive hash index */
ulonglong ahi_searches = 0; /* Successful adaptive hash lookups */
ulonglong ahi_searches_btree = 0; /* B-tree searches (AHI miss) */
ulonglong ahi_rows_added = 0; /* Rows added to adaptive hash index */
ulonglong ahi_pages_added = 0; /* Pages added to adaptive hash index */

Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment on lines +86 to +118
void btr_ahi_inc_searches() noexcept
{
if (THD *thd= current_thd)
if (trx_t *trx= thd_to_trx(thd))
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_searches++;
}

void btr_ahi_inc_searches_btree() noexcept
{
if (THD *thd= current_thd)
if (trx_t *trx= thd_to_trx(thd))
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_searches_btree++;
}

void btr_ahi_inc_rows_added(ulonglong count) noexcept
{
MONITOR_INC_VALUE(MONITOR_ADAPTIVE_HASH_ROW_ADDED, count);
if (THD *thd= current_thd)
if (trx_t *trx= thd_to_trx(thd))
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_rows_added+= count;
}

void btr_ahi_inc_pages_added() noexcept
{
MONITOR_INC(MONITOR_ADAPTIVE_HASH_PAGE_ADDED);
if (THD *thd= current_thd)
if (trx_t *trx= thd_to_trx(thd))
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_pages_added++;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While using current_thd and thd_to_trx is the standard way to access per-query statistics in MariaDB, doing so on every Adaptive Hash Index (AHI) search lookup (both hits and misses) introduces overhead in a very performance-critical path. For high-concurrency workloads where AHI is most beneficial, this overhead might be measurable. Consider if the trx_t pointer can be passed down or cached in the cursor to avoid repeated thread-local lookups, although this would require a more significant refactoring of the B-tree search interface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed mtr->trx to btr_ahi_inc_searches() and btr_ahi_inc_searches_btree() to avoid calling thd_to_trx() for every record.

@mariadb-TafzeelShams mariadb-TafzeelShams force-pushed the main-MDEV-38305 branch 2 times, most recently from a3e0cae to b95b2da Compare May 15, 2026 11:57
@mariadb-TafzeelShams mariadb-TafzeelShams marked this pull request as ready for review May 18, 2026 09:24
Copy link
Copy Markdown
Contributor

@iMineLink iMineLink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the added #ifdef BTR_CUR_HASH_ADAPT in btr0sea.cc, as these checks should be redundant in the file.
Also, try repeating the added innodb.ahi_stats ~10k times in a row both for Debug and RelWithDebInfo builds, that's how I found similar logic being flaky for MDEV-37070 changes, hopefully here we're luckier and the test is stable since the results are per-query.
Finally, it seems the regex for replacing r_engine_stats might need touchup to support the nested r_ahi_stats object: this should avoid stray }, in the rowid_filter_innodb,ahi.rdiff file.
Thanks.

Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment thread mysql-test/suite/innodb/r/ahi_stats.result
Comment thread mysql-test/main/rowid_filter_innodb,ahi.rdiff Outdated
Copy link
Copy Markdown
Contributor

@iMineLink iMineLink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread storage/innobase/btr/btr0sea.cc Outdated
return prev;
}

void btr_ahi_inc_searches(trx_t *trx) noexcept
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nitpickery (also applicable to btr_ahi_inc_searches_btree() below): you can add __attribute__((nonnull))

Comment thread mysql-test/suite/innodb/r/ahi_stats.result
Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment on lines +1149 to +1150
if (mtr->trx)
btr_ahi_inc_searches(mtr->trx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would we have !mtr->trx here? Never?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the answer is Never. I couldn't find any case yet.
But to be safe have kept the if condition.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think that it is acceptable to introduce conditional branches or dead code. It comes with a cost on modern superscalar processors with deep execution pipelines. Please omit all redundant if (mtr->trx).

Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment on lines +1157 to +1158
if (mtr->trx)
btr_ahi_inc_searches_btree(mtr->trx);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you post some stack traces where mtr->trx == nullptr would hold here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(rr) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1  0x00007fa6faaa09ff in __pthread_kill_internal (threadid=<optimized out>, signo=6) at ./nptl/pthread_kill.c:89
#2  0x00007fa6faa4bcc2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007fa6faa344ac in __GI_abort () at ./stdlib/abort.c:77
#4  0x00007fa6faa34420 in __assert_fail_base (fmt=<optimized out>, assertion=<optimized out>, file=<optimized out>, line=1154, function=<optimized out>) at ./assert/assert.c:118
#5  0x00005584052eba16 in btr_cur_t::search_leaf (this=0x77a6dfffe410, tuple=0x77a6dfffe3f0, mode=PAGE_CUR_GE, latch_mode=BTR_SEARCH_LEAF, mtr=0x77a6dfffe4b0) at /home/tafzeel/work/main_13May/storage/innobase/btr/btr0cur.cc:1154
#6  0x0000558404fa4915 in btr_pcur_open (tuple=0x77a6dfffe3f0, mode=PAGE_CUR_GE, latch_mode=BTR_SEARCH_LEAF, cursor=0x77a6dfffe410, mtr=0x77a6dfffe4b0) at /home/tafzeel/work/main_13May/storage/innobase/include/btr0pcur.h:430
#7  0x0000558404fa49e5 in btr_pcur_open_on_user_rec (tuple=0x77a6dfffe3f0, latch_mode=BTR_SEARCH_LEAF, cursor=0x77a6dfffe410, mtr=0x77a6dfffe4b0) at /home/tafzeel/work/main_13May/storage/innobase/include/btr0pcur.h:449
#8  0x000055840538bc3c in dict_load_table_on_id (table_id=18, ignore_err=DICT_ERR_IGNORE_FK_NOKEY) at /home/tafzeel/work/main_13May/storage/innobase/dict/dict0load.cc:2613
#9  0x00005584052682cc in trx_purge_table_open (table_id=18, mdl_context=0x558414c627d8, mdl=0x77a6dfffe938) at /home/tafzeel/work/main_13May/storage/innobase/trx/trx0purge.cc:1145
#10 0x0000558405268922 in trx_purge_attach_undo_recs (trx=0x7fa6ef3ff780, n_work_items=0x77a6dfffea28) at /home/tafzeel/work/main_13May/storage/innobase/trx/trx0purge.cc:1251
#11 0x000055840526924c in trx_purge (trx=0x7fa6ef3ff780, n_tasks=4, history_size=7) at /home/tafzeel/work/main_13May/storage/innobase/trx/trx0purge.cc:1364
#12 0x000055840524d5fd in purge_coordinator_state::do_purge (this=0x558406f98240 <purge_state>, trx=0x7fa6ef3ff780) at /home/tafzeel/work/main_13May/storage/innobase/srv/srv0srv.cc:1431
#13 0x000055840524c3f8 in purge_coordinator_callback () at /home/tafzeel/work/main_13May/storage/innobase/srv/srv0srv.cc:1525
#14 0x000055840548270c in tpool::task_group::execute (this=0x558406f98380 <purge_coordinator_task_group>, t=0x558406f98460 <purge_coordinator_task>) at /home/tafzeel/work/main_13May/tpool/task_group.cc:73
#15 0x0000558405482ac2 in tpool::task::execute (this=0x558406f98460 <purge_coordinator_task>) at /home/tafzeel/work/main_13May/tpool/task.cc:32
#16 0x000055840547aa50 in tpool::thread_pool_generic::worker_main (this=0x558414680c50, thread_var=0x558414ace9f0) at /home/tafzeel/work/main_13May/tpool/tpool_generic.cc:531
#17 0x00005584054824d9 in std::__invoke_impl<void, void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> (
    __f=@0x7fa6f0003578: (void (tpool::thread_pool_generic::*)(class tpool::thread_pool_generic * const, struct tpool::worker_data *)) 0x55840547a922 <tpool::thread_pool_generic::worker_main(tpool::worker_data*)>, __t=@0x7fa6f0003570: 0x558414680c50)
    at /usr/include/c++/14/bits/invoke.h:74
#18 0x00005584054823fe in std::__invoke<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> (
    __fn=@0x7fa6f0003578: (void (tpool::thread_pool_generic::*)(class tpool::thread_pool_generic * const, struct tpool::worker_data *)) 0x55840547a922 <tpool::thread_pool_generic::worker_main(tpool::worker_data*)>) at /usr/include/c++/14/bits/invoke.h:96
#19 0x0000558405482331 in std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> >::_M_invoke<0ul, 1ul, 2ul> (this=0x7fa6f0003568) at /usr/include/c++/14/bits/std_thread.h:301
#20 0x00005584054822ce in std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> >::operator() (this=0x7fa6f0003568) at /usr/include/c++/14/bits/std_thread.h:308
#21 0x00005584054822b2 in std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (tpool::thread_pool_generic::*)(tpool::worker_data*), tpool::thread_pool_generic*, tpool::worker_data*> > >::_M_run (this=0x7fa6f0003560) at /usr/include/c++/14/bits/std_thread.h:253
#22 0x00007fa6face1224 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#23 0x00007fa6faa9eb7b in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:448
#24 0x00007fa6fab1c630 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
(rr) f 5
#5  0x00005584052eba16 in btr_cur_t::search_leaf (this=0x77a6dfffe410, tuple=0x77a6dfffe3f0, mode=PAGE_CUR_GE, latch_mode=BTR_SEARCH_LEAF, mtr=0x77a6dfffe4b0) at /home/tafzeel/work/main_13May/storage/innobase/btr/btr0cur.cc:1154
1154	    ut_ad(mtr->trx!=nullptr);
(rr) p mtr->trx
$1 = (trx_t * const) 0x0
(rr) list
1149	    ++btr_cur_n_sea;
1150	
1151	    return DB_SUCCESS;
1152	  }
1153	  else {
1154	    ut_ad(mtr->trx!=nullptr);
1155	    ++btr_cur_n_non_sea;
1156	  }
1157	# endif
1158	#endif
(rr) 

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(rr) bt
#0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44
#1  0x00007f8e112a09ff in __pthread_kill_internal (threadid=<optimized out>, signo=6) at ./nptl/pthread_kill.c:89
#2  0x00007f8e1124bcc2 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
#3  0x00007f8e112344ac in __GI_abort () at ./stdlib/abort.c:77
#4  0x00007f8e11234420 in __assert_fail_base (fmt=<optimized out>, assertion=<optimized out>, file=<optimized out>, line=1155, function=<optimized out>) at ./assert/assert.c:118
#5  0x00005566d035c4e2 in btr_cur_t::search_leaf (this=0x7f8e0a6819c0, tuple=0x7f8e0a6819a0, mode=PAGE_CUR_GE, latch_mode=BTR_SEARCH_LEAF, mtr=0x7f8e0a681a60)
    at /home/tafzeel/work/main_before_37070/storage/innobase/btr/btr0cur.cc:1155
#6  0x00005566d00131d1 in btr_pcur_open (tuple=0x7f8e0a6819a0, mode=PAGE_CUR_GE, latch_mode=BTR_SEARCH_LEAF, cursor=0x7f8e0a6819c0, mtr=0x7f8e0a681a60)
    at /home/tafzeel/work/main_before_37070/storage/innobase/include/btr0pcur.h:430
#7  0x00005566d00132a1 in btr_pcur_open_on_user_rec (tuple=0x7f8e0a6819a0, latch_mode=BTR_SEARCH_LEAF, cursor=0x7f8e0a6819c0, mtr=0x7f8e0a681a60)
    at /home/tafzeel/work/main_before_37070/storage/innobase/include/btr0pcur.h:449
#8  0x00005566d03fc628 in dict_load_table_on_id (table_id=42, ignore_err=DICT_ERR_IGNORE_FK_NOKEY) at /home/tafzeel/work/main_before_37070/storage/innobase/dict/dict0load.cc:2613
#9  0x00005566d03de3a3 in dict_table_open_on_id (table_id=42, dict_locked=true, table_op=DICT_TABLE_OP_NORMAL, thd=0x0, mdl=0x0) at /home/tafzeel/work/main_before_37070/storage/innobase/dict/dict0dict.cc:841
#10 0x00005566d004e1ba in innobase_reload_table (thd=0x778de8001470, table=0x778de823b7c0, table_name=..., ctx=...) at /home/tafzeel/work/main_before_37070/storage/innobase/handler/handler0alter.cc:10627
#11 0x00005566d0051315 in ha_innobase::commit_inplace_alter_table (this=0x778de80821e0, altered_table=0x7f8e0a682ca0, ha_alter_info=0x7f8e0a682bf0, commit=true)
    at /home/tafzeel/work/main_before_37070/storage/innobase/handler/handler0alter.cc:11779
#12 0x00005566cfa78377 in handler::ha_commit_inplace_alter_table (this=0x778de80821e0, altered_table=0x7f8e0a682ca0, ha_alter_info=0x7f8e0a682bf0, commit=true)
    at /home/tafzeel/work/main_before_37070/sql/handler.cc:6014
#13 0x00005566cf6ceaf0 in mysql_inplace_alter_table (thd=0x778de8001470, table_list=0x778de8018ea0, table=0x778de81ce490, altered_table=0x7f8e0a682ca0, ha_alter_info=0x7f8e0a682bf0, 
    target_mdl_request=0x7f8e0a6834f0, ddl_log_state=0x7f8e0a682b70, trigger_param=0x7f8e0a6830a0, alter_ctx=0x7f8e0a684140, partial_alter=@0x7f8e0a682a3c: false, start_alter_id=@0x7f8e0a682a80: 0, 
    if_exists=false) at /home/tafzeel/work/main_before_37070/sql/sql_table.cc:8320
#14 0x00005566cf6d99f5 in mysql_alter_table (thd=0x778de8001470, new_db=0x778de80064b0, new_name=0x778de8006948, create_info=0x7f8e0a685030, table_list=0x778de8018ea0, recreate_info=0x7f8e0a6852a0, 
    alter_info=0x7f8e0a684eb0, order_num=0, order=0x0, ignore=false, if_exists=false) at /home/tafzeel/work/main_before_37070/sql/sql_table.cc:11755
#15 0x00005566cf7c716a in Sql_cmd_alter_table::execute (this=0x778de8019630, thd=0x778de8001470) at /home/tafzeel/work/main_before_37070/sql/sql_alter.cc:697
#16 0x00005566cf57b257 in mysql_execute_command (thd=0x778de8001470, is_called_from_prepared_stmt=false) at /home/tafzeel/work/main_before_37070/sql/sql_parse.cc:5905
#17 0x00005566cf581943 in mysql_parse (thd=0x778de8001470, rawbuf=0x778de8018d90 "ALTER TABLE t1 DROP INDEX f_idx", length=31, parser_state=0x7f8e0a6862b0)
    at /home/tafzeel/work/main_before_37070/sql/sql_parse.cc:7945
#18 0x00005566cf56cdec in dispatch_command (command=COM_QUERY, thd=0x778de8001470, packet=0x778de800cb81 "ALTER TABLE t1 DROP INDEX f_idx", packet_length=31, blocking=true)
    at /home/tafzeel/work/main_before_37070/sql/sql_parse.cc:1903
#19 0x00005566cf56b611 in do_command (thd=0x778de8001470, blocking=true) at /home/tafzeel/work/main_before_37070/sql/sql_parse.cc:1437
#20 0x00005566cf7b6a8d in do_handle_one_connection (connect=0x5566d9f30b60, put_in_cache=true) at /home/tafzeel/work/main_before_37070/sql/sql_connect.cc:1503
#21 0x00005566cf7b67da in handle_one_connection (arg=0x5566da009080) at /home/tafzeel/work/main_before_37070/sql/sql_connect.cc:1415
#22 0x00005566cfe7caca in pfs_spawn_thread (arg=0x5566d9f30810) at /home/tafzeel/work/main_before_37070/storage/perfschema/pfs.cc:2198
#23 0x00007f8e1129eb7b in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:448
#24 0x00007f8e1131c630 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:100
(rr) f 5
#5  0x00005566d035c4e2 in btr_cur_t::search_leaf (this=0x7f8e0a6819c0, tuple=0x7f8e0a6819a0, mode=PAGE_CUR_GE, latch_mode=BTR_SEARCH_LEAF, mtr=0x7f8e0a681a60)
    at /home/tafzeel/work/main_before_37070/storage/innobase/btr/btr0cur.cc:1155
1155        ut_ad(mtr->trx);
(rr) p mtr->trx
$1 = (trx_t * const) 0x0

Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment on lines +85 to +95
void btr_ahi_inc_searches(trx_t *trx) noexcept
{
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_searches++;
}

void btr_ahi_inc_searches_btree(trx_t *trx) noexcept
{
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_searches_btree++;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could these simple functions be defined inline?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment on lines +628 to +658
MONITOR_INC(MONITOR_ADAPTIVE_HASH_ROW_ADDED);
btr_ahi_inc_rows_added();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we pass const mtr_t& to all these functions so that calls to current_thd will be avoided?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, we don't have any mtr in the calling function to pass..

Comment thread storage/innobase/include/btr0sea.h Outdated
@param cursor cursor positioned on the to-be-deleted record */
void btr_search_update_hash_on_delete(btr_cur_t *cursor) noexcept;

#ifdef BTR_CUR_HASH_ADAPT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: this can be removed as well, should be already checked on top of the file. Can you please try building with BTR_CUR_HASH_ADAPT undefined, as I've seen there are some stubs at the end of the file? Should be controllable by setting to OFF with CMake OPTION(WITH_INNODB_AHI "Include innodb_adaptive_hash_index" ON). Thanks.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it could have been removed as well.
In current patch I have moved the function definition to storage/innobase/btr/btr0cur.cc

Also, I see that we have existing issue with -DWITH_INNODB_AHI=OFF
For example
cmake -DCMAKE_BUILD_TYPE=Debug -DWITH_INNODB_AHI=OFF causes

server/storage/innobase/buf/buf0lru.cc:1306:53: error: _struct buf_block_t_ has no member named _index_
 1306 |       ut_ad(!reinterpret_cast<buf_block_t*>(bpage)->index);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I opened https://jira.mariadb.org/browse/MDEV-39769 to track the compilation issues.

Copy link
Copy Markdown
Contributor

@dr-m dr-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not introduce calls to current_thd or always-holding conditions if (mtr->trx).

Comment thread storage/innobase/btr/btr0sea.cc Outdated
Comment on lines +85 to +101
static void btr_ahi_inc_rows_added(ulonglong count= 1) noexcept
{
MONITOR_INC_VALUE(MONITOR_ADAPTIVE_HASH_ROW_ADDED, count);
if (THD *thd= current_thd)
if (trx_t *trx= thd_to_trx(thd))
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_rows_added+= count;
}

static void btr_ahi_inc_pages_added() noexcept
{
MONITOR_INC(MONITOR_ADAPTIVE_HASH_PAGE_ADDED);
if (THD *thd= current_thd)
if (trx_t *trx= thd_to_trx(thd))
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_pages_added++;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My previous comment about these frequent calls to an expensive function _current_thd() was not addressed yet.

@mariadb-TafzeelShams mariadb-TafzeelShams force-pushed the main-MDEV-38305 branch 2 times, most recently from efcbc39 to 6f614b8 Compare June 3, 2026 01:13
@mariadb-TafzeelShams mariadb-TafzeelShams requested a review from dr-m June 3, 2026 02:43
Copy link
Copy Markdown
Contributor

@dr-m dr-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we are introducing per-thread counters that will be potentially frequently updated in low-level operations, I think that we should be consistent with #4252 and update any global counters less frequently, from the cache-friendly local counters.

Comment thread sql/sql_explain.cc Outdated
Comment on lines +1955 to +1956
if (hs->ahi_searches || hs->ahi_searches_btree ||
hs->ahi_rows_added || hs->ahi_pages_added)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rewrite this as a single conditional branch with a hint:

  if (unlikely(hs->ahi_searches | hs->ahi_searches_btree |
               hs->ahi_rows_added | hs->ahi_pages_added))

The result of the bitwise or will be nonzero if any of the 4 fields is nonzero. In this way, only a single conditional branch will be emitted. It should be an unlikely one, because the adaptive hash index is disabled by default.

Comment thread storage/innobase/btr/btr0cur.cc Outdated
Comment on lines +1166 to +1175
++btr_cur_n_sea;
btr_ahi_inc_searches(*mtr);

return DB_SUCCESS;
}
else
{
++btr_cur_n_non_sea;
btr_ahi_inc_searches_btree(*mtr);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please eliminate the sharded counters btr_cur_n_sea and btr_cur_n_non_sea and implement an aggregation on trx_t::commit_cleanup() and trx_t::free(), like it was done for buf_pool.stat.n_page_gets in #4252?

Comment on lines +85 to +98
static void btr_ahi_inc_rows_added(const mtr_t &mtr, ulonglong count= 1)
noexcept
{
MONITOR_INC_VALUE(MONITOR_ADAPTIVE_HASH_ROW_ADDED, count);
if (ha_handler_stats *stats= mtr.trx->active_handler_stats)
stats->ahi_rows_added+= count;
}

static void btr_ahi_inc_pages_added(const mtr_t &mtr) noexcept
{
MONITOR_INC(MONITOR_ADAPTIVE_HASH_PAGE_ADDED);
if (ha_handler_stats *stats= mtr.trx->active_handler_stats)
stats->ahi_pages_added++;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make the monitor counters "overload" a global counter that would be updated from the stats counters in trx_t::commit_cleanup() and trx_t::free()? I see that currently there is no global counter exposed via innodb_status_variables. These counters need to be there, because a goal that was set in MDEV-15706 is to deprecate or replace the srv0mon.cc interface.


#ifdef BTR_CUR_HASH_ADAPT
void search_info_update() const noexcept;
void search_info_update(const mtr_t &mtr) const noexcept;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the added parameter mtr is not documented.

Comment thread storage/innobase/include/btr0sea.h Outdated
Comment on lines +67 to +68
@param block source page (subject to deletion later)
@param mtr mini-transaction */
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the added parameter mtr is not documented. It for updating access statistics.

Comment thread storage/innobase/include/btr0sea.h Outdated
Comment on lines +90 to +92
@param mtr mini-transaction */
void btr_search_update_hash_on_insert(btr_cur_t *cursor, bool reorg,
const mtr_t &mtr) noexcept;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of mtr is not documented.

Comment on lines +45 to +46
THD *_current_thd() { return nullptr; }
trx_t *thd_to_trx(const THD*) noexcept { return nullptr; }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these really still needed? These are the only additions of current_thd or thd_to_trx in this revision.

Comment on lines +1089 to +1095
/** Increment adaptive hash index misses (B-tree fallback) */
static inline void btr_ahi_inc_searches_btree(const mtr_t &mtr) noexcept
{
if (trx_t *trx= mtr.trx)
if (ha_handler_stats *stats= trx->active_handler_stats)
stats->ahi_searches_btree++;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When would this code be invoked with mtr.trx=nullptr? Could that caller be adjusted so that the data member will be set? My best guess would be the purge subsystem, but even there 4369a38 made an effort to ensure that mtr.trx points to a dummy transaction object that is associated with the purge coordinator.

Copy link
Copy Markdown
Contributor Author

@mariadb-TafzeelShams mariadb-TafzeelShams Jun 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mtr.trx=nullptr at this place was happening before MDEV-37070 (2 examples)

With MDEV-37070 we don't have this issue for dict_load_table_on_id() because for SYS_TABLES.ID_IND we have enabled_fixed_mask=0 (AHI_INDEX_FORCE_DISABLED)

Expose InnoDB's Adaptive Hash Index (AHI) statistics through ANALYZE
FORMAT=JSON output and global status variables to provide query-level
and system-level visibility into AHI usage and effectiveness.
This allows DBAs and developers to monitor how well
the adaptive hash index is serving their workloads on a per-query basis.

The r_ahi_stats object (nested inside r_engine_stats) now reports four
key metrics: ahi_searches (successful AHI lookups), ahi_searches_btree
(AHI misses requiring B-tree fallback), ahi_rows_added (rows inserted
into AHI), and ahi_pages_added (pages indexed by AHI). These same
metrics are exposed globally via innodb_status_variables.

Transaction-local tracking eliminates contention by accumulating
statistics in trx_t fields (n_sea, n_non_sea, n_ahi_rows_added,
n_ahi_pages_added) and aggregating them into global atomic counters
(btr_cur_n_sea, btr_cur_n_non_sea, btr_ahi_n_rows_added,
btr_ahi_n_pages_added) during trx_t::commit_cleanup() and trx_t::free(),
following the pattern established for buf_pool.stat.n_page_gets.

- btr_ahi_inc_searches(): Increment trx->n_sea when AHI lookup succeeds.
- btr_ahi_inc_searches_btree(): Increment trx->n_non_sea when AHI lookup
  fails and falls back to B-tree search.
- btr_ahi_inc_rows_added(): Increment trx->n_ahi_rows_added when rows
  are added to the adaptive hash index structure.
- btr_ahi_inc_pages_added(): Increment trx->n_ahi_pages_added when new
  pages are indexed by AHI.
- btr_cur_t::search_leaf(): Call btr_ahi_inc_searches() on successful
  AHI hit and btr_ahi_inc_searches_btree() on AHI miss.
- btr_cur_n_sea, btr_cur_n_non_sea: Replace sharded ib_counter_t with simple
  atomic counters.
- MONITOR_OVLD_ADAPTIVE_HASH_ROW_ADDED, MONITOR_OVLD_ADAPTIVE_HASH_PAGE_ADDED:
  Convert monitor counters to "overload" type that read from global atomic
  counters btr_ahi_n_rows_added and btr_ahi_n_pages_added respectively instead
  of maintaining separate statistics.
- trace_engine_stats(): Output r_ahi_stats object with all four AHI
  counters in JSON format when any AHI activity is detected during query
  execution.
- ha_handler_stats: Added ahi_searches, ahi_searches_btree, ahi_rows_added,
  and ahi_pages_added fields to track per-query AHI statistics.
- Add mtr parameter to btr_search_move_or_delete_hash_entries(),
  btr_cur_t::search_info_update(), btr_search_update_hash_on_insert(),
  btr_search_update_hash_ref(), btr_search_info_update_hash(), and
  btr_search_build_page_hash_index() to allow updating AHI statistics.
- ahi_stats.test: Comprehensive verification of AHI statistics reporting
  across different scenarios: insufficient accesses (no AHI build),
  threshold triggering (AHI construction), heavy warmup (full AHI
  utilization), and disabled AHI (verify zero statistics).
- check_ahi_status.inc: Reusable include file for executing queries with
  configurable warmup repetitions and extracting AHI statistics from
  ANALYZE FORMAT=JSON output using JSON path expressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants