Skip to content

Commit

Permalink
[upstream] PS-8174, Bug #107069 - Assertion failure: buf0flu.cc:3567:…
Browse files Browse the repository at this point in the history
…UT_LIST_GET_LEN(buf_pool->flush_list) == 0

https://jira.percona.com/browse/PS-8174

Problem:
There is a possibility that at shutdown by the time we do the last
sweep on flushing the buffer pool there are still pages in the flush
list. Those pages are still marked as io_fix->BUF_IO_READ thus they are
not eligible for flushing from flush_list.

Where is the workflow:

1. ibuf_merge_in_background requested those pages to be read in order to
merge the ibuf changes. This will mark the page as BUF_IO_READ and
increment buf_pool->n_pend_reads by 1.
2. When IO threads pick them up, it will start to merge the insert
bugger changes.
3. On the first change, it will add the page to flush_list.
4. If there are more changes to apply, it will and continue on applying
the changes until it is done.
5. Once the io thread finishes applying ibuf records to this page, it
will mark the page as BUF_IO_NONE
6. the io thread decreases buf_pool->n_pend_reads by 1.

The last sweep on flushing buffer pool considers the round of flushes
completed when n_flushed == 0 which is not correct, if it runs when we
are at step 4.

Also, there is a still another race condition that by the time we tried
to flush a page it was still marked as BUF_IO_READ (step 5), but when
the page cleaner code checks buf_get_n_pending_read_ios, the IO thread
has already decremented leaving one last page on flush_list.

Fix:

For the last round flushing pages at shutdown, only consider the flush
completed if there are no pending read io operations and flush_list size
is 0. Added a new function to get flush_list lenght. For this last sweep
from PC we do not need mutex as there won't be other threads reading nor
writting to it at this stage.
  • Loading branch information
altmannmarcelo authored and oleksandr-kachan committed Jan 19, 2024
1 parent 93b2f0c commit 18e77ef
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 1 deletion.
16 changes: 16 additions & 0 deletions storage/innobase/buf/buf0buf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,22 @@ static void buf_pool_register_chunk(buf_chunk_t *chunk) {
buf_pool_chunk_map_t::value_type(chunk->blocks->frame, chunk));
}

ulint buf_get_flush_list_len(const buf_pool_t *buf_pool) {
ulint pages = 0;
if (buf_pool == nullptr) {
for (ulint i = 0; i < srv_buf_pool_instances; i++) {
buf_pool_t *buf_pool_instance;

buf_pool_instance = buf_pool_from_array(i);

pages += UT_LIST_GET_LEN(buf_pool_instance->flush_list);
}
} else {
pages = UT_LIST_GET_LEN(buf_pool->flush_list);
}
return (pages);
}

lsn_t buf_pool_get_oldest_modification_approx(void) {
lsn_t lsn = 0;
lsn_t oldest_lsn = 0;
Expand Down
3 changes: 2 additions & 1 deletion storage/innobase/buf/buf0flu.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3518,7 +3518,8 @@ static void buf_flush_page_coordinator_thread() {
buf_flush_await_no_flushing(nullptr, BUF_FLUSH_LIST);
buf_flush_await_no_flushing(nullptr, BUF_FLUSH_LRU);

} while (!success || n_flushed > 0 || are_any_read_ios_still_underway);
} while (!success || n_flushed > 0 || are_any_read_ios_still_underway ||
buf_get_flush_list_len(nullptr) > 0);

for (ulint i = 0; i < srv_buf_pool_instances; i++) {
buf_pool_t *buf_pool = buf_pool_from_array(i);
Expand Down
5 changes: 5 additions & 0 deletions storage/innobase/include/buf0buf.h
Original file line number Diff line number Diff line change
Expand Up @@ -1083,6 +1083,11 @@ has returned NULL and before invoking buf_pool_watch_unset(same_page_id).
@return false if the given page was not read in, true if it was */
[[nodiscard]] bool buf_pool_watch_occurred(const page_id_t &page_id);

/** Get the current length of the flush list.
@param[in] buf_pool buffer pool instance or nullptr to all instances
@return number of pages in flush list */
ulint buf_get_flush_list_len(const buf_pool_t *buf_pool);

/** Get total buffer pool statistics.
@param[out] LRU_len Length of all lru lists
@param[out] free_len Length of all free lists
Expand Down

0 comments on commit 18e77ef

Please sign in to comment.