Unnecessary AHI removal in buf_LRU_mark_space_was_deleted

Bug #1076215 reported by yinfeng
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona Server moved to https://jira.percona.com/projects/PS
Fix Released
Low
Laurynas Biveinis
5.1
Fix Released
Low
Laurynas Biveinis
5.5
Fix Released
Low
Laurynas Biveinis

Bug Description

Recently I am trying to understand the code path of "drop table".

buf_LRU_mark_space_was_deleted was called if innodb_lazy_drop_table was set to 1. and the code bellow may be executed(PS 5.5.28):

        chunk = buf_pool->chunks;
        for (j = buf_pool->n_chunks; j--; chunk++) {
            buf_block_t* block = chunk->blocks;
            for (k = chunk->size; k--; block++) {
                if (buf_block_get_state(block)
                    != BUF_BLOCK_FILE_PAGE
                    || !block->index
                    || buf_page_get_space(&block->page) != id) {
                    continue;
                }

                btr_search_s_unlock_all();

                rw_lock_x_lock(&block->lock);
                btr_search_drop_page_hash_index(block);
                rw_lock_x_unlock(&block->lock);

                btr_search_s_lock_all();
            }
        }

btr_search_drop_page_hash_index was called to drop page hash index entries.

But before fil_delete_tablespace was called . All page hash index entries of the deleted table should have been freed up(fseg_free_extent ->btr_search_drop_page_hash_when_freed)

I am not sure if the code quoted above is redundant.

Related branches

Revision history for this message
Raghavendra D Prabhu (raghavendra-prabhu) wrote :

@yinfeng,

I noticed that both the lazy / non-lazy implementations call btr_search_drop_page_hash_index

 if (srv_lazy_drop_table) {
  buf_LRU_mark_space_was_deleted(id);
 } else {
 buf_LRU_flush_or_remove_pages(
  id, evict_all
  ? BUF_REMOVE_ALL_NO_WRITE
  : BUF_REMOVE_FLUSH_NO_WRITE);

 }

buf_LRU_flush_or_remove_pages also calls it.

Regarding btr_search_drop_page_hash_when_freed from fseg_free_extent I noticed that only dict_truncate_index_tree makes call to it in row_truncate_table_for_mysql, though that is done after fil_discard_tablespace

Can you tell us the call-path where you found duplicate calls to btr_search_drop_page_hash_when_freed / btr_search_drop_page_hash_index (gdb bt with drop table should do)?

Revision history for this message
yinfeng (yinfeng-zwx) wrote :
Download full text (3.2 KiB)

buf_LRU_flush_or_remove_pages won't call buf_LRU_drop_page_hash_for_tablespace if buf_remove=BUF_REMOVE_FLUSH_NO_WRITE. Bellow is the quited code from buf_LRU_flush_or_remove_pages

        case BUF_REMOVE_FLUSH_NO_WRITE:
            /* A DROP table case. AHI entries are already
            removed. No need to evict all pages from LRU
            list. Just evict pages from flush list without
            writing. */
            buf_flush_dirty_pages(buf_pool, id);
            break;

backtrace of the first call to free page hash index entries:

#0 btr_search_drop_page_hash_when_freed (space=17296, zip_size=4096, page_no=622)
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/btr/btr0sea.c:1447
#1 0x0000000000855c29 in fseg_free_extent (seg_inode=<value optimized out>, space=17296, zip_size=4096, page=576, mtr=0x7fe183a5ffd0)
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/fsp/fsp0fsp.c:3498
#2 0x0000000000856a24 in fseg_free_step (header=<value optimized out>, mtr=0x7fe183a5ffd0)
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/fsp/fsp0fsp.c:3598
#3 0x00000000007f7441 in btr_free_but_not_root (space=17296, zip_size=4096, root_page_no=3)
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/btr/btr0btr.c:1544
#4 0x0000000000826e7e in dict_drop_index_tree (rec=0x7fe43dc076c6 "", mtr=0x7fe183a60580)
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/dict/dict0crea.c:795
#5 0x00000000007be8fd in row_upd_clust_step (node=0x7fe17802e088, thr=0x7fe17802fd68)
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/row/row0upd.c:2210
#6 0x00000000007bf2be in row_upd (thr=0x7fe17802fd68) at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/row/row0upd.c:2356
#7 row_upd_step (thr=0x7fe17802fd68) at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/row/row0upd.c:2496
#8 0x00000000008a4b08 in que_thr_step (thr=0x7fe17802fd68) at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/que/que0que.c:1238
#9 que_run_threads_low (thr=0x7fe17802fd68) at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/que/que0que.c:1319
#10 que_run_threads (thr=0x7fe17802fd68) at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/que/que0que.c:1356
#11 0x00000000008a5206 in que_eval_sql (info=<value optimized out>, sql=<value optimized out>, reserve_dict_mutex=<value optimized out>,
    trx=0x7fe178013bf8) at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/que/que0que.c:1445
#12 0x00000000007ac98a in row_drop_table_for_mysql (name=0x7fe183a60ff0 "sbtest/sbtest6", trx=0x7fe178013bf8, drop_db=<value optimized out>)
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/row/row0mysql.c:3433
#13 0x00000000007974c0 in ha_innobase::delete_table (this=<value optimized out>, name=0x7fe183a62500 "./sbtest/sbtest6")
    at /u01/project/Percona-Server-5.5.28-rel29.1/storage/innobase/handler/ha_innodb.cc:8361
#14 0x0000000000699f0e in ha_delete_table (thd=0x14870730, table_type=<value optimized out>, path=0x7fe183a62500 "./sbtest/sbtest6",
    db=0x7fe178004ee8 "sbtest", alias=0x7fe178004988 "sbtest6", generate_war...

Read more...

Revision history for this message
Alexey Kopytov (akopytov) wrote :

Yes, the code dropping page hash indexes in buf_LRU_mark_space_was_deleted() appears to be redundant for the DROP TABLE case (unlike the DISCARD TABLESPACE case).

It was introduced as a fix for https://bugs.launchpad.net/percona-server/+bug/798371, but that fix contained other changes, so it is unclear whether this specific code is actually needed. I'm fairly sure it is not, because indeed AHI entries are removed when freeing up index extents and that is done before calling fil_delete_tablespace(). And the upstream fix for http://bugs.mysql.com/bug.php?id=64284 relies on this fact.

summary: - some confusing code in buf_LRU_mark_space_was_deleted
+ Unnecessary AHI scan in buf_LRU_mark_space_was_deleted
Revision history for this message
Alexey Kopytov (akopytov) wrote : Re: Unnecessary AHI scan in buf_LRU_mark_space_was_deleted

Is it really an AHI scan as the bug title says now? It is actually a buffer pool scan.

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Hm, right, thanks.

summary: - Unnecessary AHI scan in buf_LRU_mark_space_was_deleted
+ Unnecessary AHI removal in buf_LRU_mark_space_was_deleted
Revision history for this message
Shahriyar Rzayev (rzayev-sehriyar) wrote :

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PS-1945

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.