From 32cd5b16bad4af85cfb93f28274ff14df1e95bee Mon Sep 17 00:00:00 2001 From: Alessandro Vetere Date: Wed, 15 Jul 2026 18:48:36 +0200 Subject: [PATCH] MDEV-40408 btr_page_reorganize_low() uses the buffer pool just to obtain a scratch block Add a pool of scratch blocks, btr_scratch_pool_t, to be used instead of the global buffer pool in the page-reorganization operations, when a scratch block is needed. Removes the needs of obtaining the buf_pool.mutex in such functions, and any delay while holding at least page X-latch that it might have caused. --- extra/mariabackup/xtrabackup.cc | 1 + storage/innobase/btr/btr0btr.cc | 256 ++++++++++++++++++++++++++++- storage/innobase/include/btr0btr.h | 15 ++ storage/innobase/include/buf0buf.h | 5 +- storage/innobase/page/page0zip.cc | 4 +- storage/innobase/srv/srv0srv.cc | 1 + storage/innobase/srv/srv0start.cc | 1 + 7 files changed, 278 insertions(+), 5 deletions(-) diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index dec69a0905ecd..a5d9ede991780 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -5532,6 +5532,7 @@ static bool xtrabackup_backup_func() srv_thread_pool_init(); /* Reset the system variables in the recovery module. */ trx_pool_init(); + btr_scratch_pool_init(); btr_search_sys_create(); recv_sys.create(); diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/btr0btr.cc index f340d3a4891fe..a113691f2bc98 100644 --- a/storage/innobase/btr/btr0btr.cc +++ b/storage/innobase/btr/btr0btr.cc @@ -1288,6 +1288,251 @@ void btr_write_autoinc(trx_t *trx, dict_index_t *index, uint64_t autoinc, mtr.commit(); } +/** A buffer-pool-independent pool of scratch buf_block_t objects. + +Such pool is designed to be used instead of the global buffer pool +for the page-reorganization operations done by btr_page_reorganize() +and page_zip_reorganize(), where a scratch block is required to +hold a temporary, pre-rebuild copy of the page being reorganized. + +Using this pool removes the need of obtaining the global buf_pool.mutex +in such functions, avoiding any related delay while holding at least the +page X-latch. +This is traded however with some on-demand calls to aligned_malloc(), +whose number is bounded by the MAX_SLABS parameter. + +Allocation is lazy and organized in slabs, each containing BLOCKS_PER_SLAB +blocks. +The first slab is eagerly allocated upon pool opening. +Each slab is linked to the next one in a singly-linked list fashion, +to be traversed when the pool is destroyed. +The pool never shrinks during usage. +Each block in a slab is wrapped in a node, which contains the block +and a pointer to the next-free node in a singly-linked list fashion. +The head of the singly-linked list is the top of the free stack. + +When there is no free scratch block, a slab is allocated if possible, +otherwise MAX_SLABS slabs are already allocated and the caller waits +for a block to be released. +This wait has two properties: first, it is intended to be rare, +since MAX_SLABS * BLOCKS_PER_SLAB is designed to cover reasonable amount +of concurrent page-reorganizing threads, and second, it is bounded to +a rather short operation, that is reorganizing a page. +There is at least one caveat which might prolong the wait length: +lock_sys.latch contention, which might delay lock_move_reorganize_page(). +The current setting of BLOCKS_PER_SLAB and MAX_SLABS should be generous +enough, but if this ever becomes a bottleneck, it could be suggested +to add user-facing knobs to control the size of this scratch pool. */ +class btr_scratch_pool_t +{ + /** Number of scratch blocks per allocated slab. + At default 16KiB page size: ~1MiB per slab. + Worst case 64KiB page size: ~4MiB per slab. */ + static constexpr size_t BLOCKS_PER_SLAB= 64; + static_assert(BLOCKS_PER_SLAB > 0); + + /** Maximum number of slabs. + At default 16KiB page size: ~16MiB max. + Worst case 64KiB page size: ~64MiB max. + MAX_SLABS * BLOCKS_PER_SLAB currently 1024 determines the absolute + maximum number of concurrent reorganizations. */ + static constexpr size_t MAX_SLABS= 16; + static_assert(MAX_SLABS > 0); + + /** Block wrapped with intrusive free-list linkage. */ + struct alignas(CPU_LEVEL1_DCACHE_LINESIZE) node_t + { + buf_block_t block; + node_t *next_free; + }; + static_assert(!(sizeof(node_t) % CPU_LEVEL1_DCACHE_LINESIZE), + "node_t must be a whole number of cache lines to " + "avoid false sharing in slab_t::nodes"); + + /** Slab of BLOCKS_PER_SLAB nodes. */ + struct slab_t + { + slab_t *next; + /** BLOCKS_PER_SLAB * srv_page_size bytes, aligned to srv_page_size. */ + byte *frames; + node_t nodes[BLOCKS_PER_SLAB]; + }; + + mysql_mutex_t mutex; + /** Signaled by put() whenever a block is returned. + Protected by mutex. */ + pthread_cond_t block_freed; + slab_t *slabs= nullptr; + size_t n_slabs= 0; + /** Singly-linked stack of unused nodes. */ + node_t *free= nullptr; + /** Number of blocks currently handed out to callers, that is obtained + by get() and not yet returned by put(). Protected by mutex. Debug only. */ + ut_d(size_t n_active= 0); + + /** Allocate one more slab and push its nodes onto the free stack, + unless MAX_SLABS was already reached. + @tparam must_own_mutex whether the caller must own mutex. + @return whether a slab was allocated. */ + template + bool add_slab() noexcept + { + if constexpr (must_own_mutex) + mysql_mutex_assert_owner(&mutex); + + if (n_slabs >= MAX_SLABS) + return false; + + slab_t *s= static_cast + (aligned_malloc(sizeof(slab_t), alignof(slab_t))); + if (UNIV_UNLIKELY(!s)) + return false; + memset(static_cast(s), 0, sizeof(slab_t)); + + s->frames= static_cast + (aligned_malloc(BLOCKS_PER_SLAB * srv_page_size, srv_page_size)); + if (UNIV_UNLIKELY(!s->frames)) + { + aligned_free(s); + return false; + } + + for (size_t i= 0; i < BLOCKS_PER_SLAB; i++) + { + node_t *node= &s->nodes[i]; + node->block.page.frame= s->frames + i * srv_page_size; + node->block.page.lock.init(); + node->block.page.set_state(buf_page_t::MEMORY); + node->block.page.set_os_unused(); + node->next_free= free; + free= node; + } + + s->next= slabs; + slabs= s; + ++n_slabs; + return true; + } + +public: + /** Create the pool, initializing the mutex and the condition variable, + then eagerly allocate the first slab, retrying with a sleep in between + on aligned_malloc() failure. Abort on impossibility of allocating the + first slab. + + Note: safe to call only after srv_page_size is set. */ + void create() noexcept + { + mysql_mutex_init(PSI_NOT_INSTRUMENTED, &mutex, nullptr); + pthread_cond_init(&block_freed, nullptr); + ut_ad(srv_page_size >= UNIV_PAGE_SIZE_MIN && + srv_page_size <= UNIV_PAGE_SIZE_MAX); + ut_ad(!slabs); + ut_ad(!n_slabs); + ut_ad(!free); + /* Try allocating the first slab, abort on failure after few retries */ + size_t failures= 0; + while (!add_slab()) + { + if (!failures) + ib::warn() << "Could not allocate the first scratch pool slab, retrying..."; + ++failures; + if (failures < 100) + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + else + ib::fatal() << "Cannot allocate the first scratch pool slab."; + } + ut_ad(slabs); + ut_ad(n_slabs == 1); + ut_ad(free); + } + + /** Close the pool, traversing the list of slabs and freeing each + slabs's block, frames and the slab itself. Finally condition + variable and mutex are destroyed. + No-op if create() was never called. */ + void close() noexcept + { + if (!slabs) + { + ut_ad(!n_slabs); + ut_ad(!free); + ut_ad(!n_active); + return; + } + ut_ad(n_slabs >= 1 && n_slabs <= MAX_SLABS); + ut_ad(free); + ut_ad(!n_active); + for (slab_t *s= slabs; s; ) + { + slab_t *next= s->next; + for (node_t &node : s->nodes) + node.block.page.lock.free(); + aligned_free(s->frames); + aligned_free(s); + s= next; + } + slabs= nullptr; + n_slabs= 0; + free= nullptr; + pthread_cond_destroy(&block_freed); + mysql_mutex_destroy(&mutex); + } + + /** Get a block from the free stack, protected by mutex. + If no free block is available, try allocating a new slab. + If no slab can be allocated (MAX_SLABS reached, or allocation + errors), then wait on the condition variable for a block + to be freed. + @return a scratch block; never nullptr. */ + buf_block_t *get() noexcept + { + mysql_mutex_lock(&mutex); + ut_ad(slabs); + ut_ad(n_slabs >= 1); + while (!free && !add_slab()) + my_cond_wait(&block_freed, &mutex.m_mutex); + ut_ad(free); + node_t *node= free; + free= node->next_free; + ut_d(++n_active); + mysql_mutex_unlock(&mutex); + node->block.page.set_os_used(); + return &node->block; + } + + /** Return a block obtained from get(), protected by mutex. + Condition variable is signalled once the block is put in + the free stack. + @param block the block to be returned. */ + void put(buf_block_t *block) noexcept + { + ut_ad(block); + node_t *node= reinterpret_cast(block); + ut_ad(reinterpret_cast(&node->block) == + reinterpret_cast(node)); + node->block.page.set_os_unused(); + mysql_mutex_lock(&mutex); + node->next_free= free; + free= node; + ut_ad(free); + ut_ad(n_active > 0); + ut_d(--n_active); + pthread_cond_signal(&block_freed); + mysql_mutex_unlock(&mutex); + } +}; + +/** The scratch pool. */ +static btr_scratch_pool_t btr_scratch_pool; + +void btr_scratch_pool_init() noexcept { btr_scratch_pool.create(); } +void btr_scratch_pool_close() noexcept { btr_scratch_pool.close(); } + +buf_block_t *btr_scratch_block_get() noexcept { return btr_scratch_pool.get(); } +void btr_scratch_block_put(buf_block_t *block) noexcept +{ btr_scratch_pool.put(block); } + /** Reorganize an index page. @param cursor index page cursor @param mtr mini-transaction */ @@ -1312,7 +1557,7 @@ static dberr_t btr_page_reorganize_low(page_cur_t *cursor, mtr_t *mtr) btr_search_drop_page_hash_index(block, nullptr); - buf_block_t *old= buf_block_alloc(); + buf_block_t *old= btr_scratch_block_get(); /* Copy the old page to temporary space */ memcpy_aligned(old->page.frame, block->page.frame, srv_page_size); @@ -1336,7 +1581,10 @@ static dberr_t btr_page_reorganize_low(page_cur_t *cursor, mtr_t *mtr) mtr->set_log_mode(log_mode); if (UNIV_UNLIKELY(err != DB_SUCCESS)) + { + btr_scratch_block_put(old); return err; + } /* Copy the PAGE_MAX_TRX_ID or PAGE_ROOT_AUTO_INC. */ ut_ad(!page_get_max_trx_id(block->page.frame)); @@ -1372,6 +1620,7 @@ static dberr_t btr_page_reorganize_low(page_cur_t *cursor, mtr_t *mtr) sql_print_error("InnoDB: Page old data size %u new data size %u" ", page old max ins size %zu new max ins size %zu", data_size1, data_size2, max1, max2); + btr_scratch_block_put(old); return DB_CORRUPTION; } @@ -1379,7 +1628,10 @@ static dberr_t btr_page_reorganize_low(page_cur_t *cursor, mtr_t *mtr) if (!pos) ut_ad(cursor->rec == page_get_infimum_rec(block->page.frame)); else if (!(cursor->rec= page_rec_get_nth(block->page.frame, pos))) + { + btr_scratch_block_put(old); return DB_CORRUPTION; + } if (block->page.id().page_no() != cursor->index->page || fil_page_get_type(old->page.frame) != FIL_PAGE_TYPE_INSTANT) @@ -1550,7 +1802,7 @@ static dberr_t btr_page_reorganize_low(page_cur_t *cursor, mtr_t *mtr) } } - buf_block_free(old); + btr_scratch_block_put(old); MONITOR_INC(MONITOR_INDEX_REORG_ATTEMPTS); MONITOR_INC(MONITOR_INDEX_REORG_SUCCESSFUL); diff --git a/storage/innobase/include/btr0btr.h b/storage/innobase/include/btr0btr.h index 55acef7e58c73..9c1d105202829 100644 --- a/storage/innobase/include/btr0btr.h +++ b/storage/innobase/include/btr0btr.h @@ -87,6 +87,21 @@ bool btr_root_fseg_validate(ulint offset, const buf_block_t &block, @param index the index that is being accessed */ ATTRIBUTE_COLD void btr_read_failed(dberr_t err, const dict_index_t &index); +/** Create the pool of scratch blocks used by btr_page_reorganize_low() +and page_zip_reorganize(). */ +void btr_scratch_pool_init() noexcept; +/** Release the pool of scratch blocks used by btr_page_reorganize_low() +and page_zip_reorganize(). */ +void btr_scratch_pool_close() noexcept; + +/** Obtain a scratch block for reorganizing a page. It is never linked +into buf_pool.page_hash, buf_pool.LRU or buf_pool.free, and obtaining one +never takes buf_pool.mutex. +@return a scratch block; never nullptr */ +buf_block_t *btr_scratch_block_get() noexcept; +/** Return a block obtained from btr_scratch_block_get(). */ +void btr_scratch_block_put(buf_block_t *block) noexcept; + /** Get an index page and declare its latching order level. @param index index tree @param page page number diff --git a/storage/innobase/include/buf0buf.h b/storage/innobase/include/buf0buf.h index a2dc7e8d42c04..70687048ecc84 100644 --- a/storage/innobase/include/buf0buf.h +++ b/storage/innobase/include/buf0buf.h @@ -680,6 +680,7 @@ class buf_page_t ut_ad(!((prev_state ^ s) & LRU_MASK)); } + template inline void set_state(uint32_t s) noexcept; inline void set_corrupt_id() noexcept; @@ -1796,9 +1797,11 @@ inline void page_hash_latch::lock() noexcept } #endif /* SUX_LOCK_GENERIC */ +template inline void buf_page_t::set_state(uint32_t s) noexcept { - mysql_mutex_assert_owner(&buf_pool.mutex); + if constexpr (must_own_buf_pool_mutex) + mysql_mutex_assert_owner(&buf_pool.mutex); ut_ad(s <= REMOVE_HASH || s >= UNFIXED); ut_ad(s < WRITE_FIX); ut_ad(s <= READ_FIX + 1 || zip.fix == READ_FIX + 1); diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc index 45d1228a20f14..5f06dd98d2b01 100644 --- a/storage/innobase/page/page0zip.cc +++ b/storage/innobase/page/page0zip.cc @@ -4397,8 +4397,8 @@ page_zip_reorganize( /* Disable logging */ mtr_log_t log_mode = mtr_set_log_mode(mtr, MTR_LOG_NONE); - temp_block = buf_block_alloc(); btr_search_drop_page_hash_index(block, nullptr); + temp_block = btr_scratch_block_get(); temp_page = temp_block->page.frame; /* Copy the old page to temporary space */ @@ -4472,7 +4472,7 @@ page_zip_reorganize( lock_move_reorganize_page(block, temp_block); } - buf_block_free(temp_block); + btr_scratch_block_put(temp_block); return err; } diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc index 5c8772d0dac7a..7995aa3839a30 100644 --- a/storage/innobase/srv/srv0srv.cc +++ b/storage/innobase/srv/srv0srv.cc @@ -604,6 +604,7 @@ void srv_boot() buf_dblwr.init(); srv_thread_pool_init(); trx_pool_init(); + btr_scratch_pool_init(); btr_search_sys_create(); srv_init(); } diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index 106a3f5552d6d..8071f64e2f485 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -2091,6 +2091,7 @@ void innodb_shutdown() buf_dblwr.close(); lock_sys.close(); trx_pool_close(); + btr_scratch_pool_close(); if (!srv_read_only_mode) { mysql_mutex_destroy(&srv_monitor_file_mutex);