MDEV-38833 Suboptimal or dead code at server startup#4647
Open
Conversation
|
|
The InnoDB bootstrap is creating unnecessary log checkpoints and even invalidating the buffer pool for no reason, making it necessary to read pages into the buffer pool during bootstrap. Furthermore, the page cleaner thread attempts to invoke recv_sys.apply(true), even though most calls were unreachable starting with commit 685d958 (MDEV-14425) if not earlier. buf_flush_sync(): Replaced with buf_flush_sync_batch(LSN_MAX), possibly preceded by recv_sys.apply(true). buf_pool_t::assert_all_freed(): Assert that buf_pool.mutex is being held by the caller. buf_dblwr_t::create(): Evict each doublewrite buffer pool page immediately from the buffer pool, so that there will be no need to invoke buf_pool_invalidate(). Also, remove the unnecessary call to buf_flush_wait_flushed(); there is nothing special about the doublewrite buffer creation ever since commit e2c63a7 (MDEV-38595) made the redo logging actually crash-safe. buf_pool_invalidate(): Moved to the same compilation unit with the only remaining caller, recv_sys_t::apply(). srv_start(): Do not invoke buf_flush_sync(). We already created an initial checkpoint in log_t::create(lsn_t), and we will create the final one during shutdown. Together with the simplification of buf_dblwr_t::create(), this should noticeably speed up the server bootstrap. Also, simplify the log file rebuild by extending the critical section of log_sys.latch and buf_pool.flush_list_mutex. create_log_file(): Assert and assume that both log_sys.latch and buf_pool.flush_list_mutex were acquired by the caller. This prevents any race condition with buf_flush_page_cleaner() when the log is being rebuilt. srv_prepare_to_delete_redo_log_file(): Invoke recv_sys.apply(true) and mark the recovery as completed, before checking for one more precondition and invoking buf_flush_sync_batch(LSN_MAX). Remove some duplicated assertions as well as a redundant call to log_write_up_to() from the end. If we crash while rebuilding a latest_format log file, the subsequent startup will perform crash recovery normally. This avoids a redundant write to the old log file during log file resizing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The InnoDB bootstrap is creating unnecessary log checkpoints and even invalidating the buffer pool for no reason, making it necessary to read pages into the buffer pool during bootstrap.
Furthermore, the page cleaner thread attempts to invoke
recv_sys.apply(true), even though most calls were unreachable starting with 685d958 (MDEV-14425) if not earlier.buf_flush_sync(): Replaced withbuf_flush_sync_batch(LSN_MAX), possibly preceded byrecv_sys.apply(true).buf_pool_t::assert_all_freed(): Assert thatbuf_pool.mutexis being held by the caller.buf_dblwr_t::create(): Evict each doublewrite buffer pool page immediately from the buffer pool, so that there will be no need to invokebuf_pool_invalidate(). Also, remove the unnecessary call tobuf_flush_wait_flushed(); there is nothing special about the doublewrite buffer creation ever since e2c63a7 (MDEV-38595) made the redo logging actually crash-safe.buf_pool_invalidate(): Moved to the same compilation unit with the only remaining caller,recv_sys_t::apply().srv_start(): Do not invokebuf_flush_sync(). We already created an initial checkpoint inlog_t::create(lsn_t), and we will create the final one during shutdown. Together with the simplification ofbuf_dblwr_t::create(), this should noticeably speed up the server bootstrap. Also, simplify the log file rebuild by extending the critical section oflog_sys.latchandbuf_pool.flush_list_mutex.create_log_file(): Assert and assume that bothlog_sys.latchandbuf_pool.flush_list_mutexwere acquired by the caller. This prevents any race condition withbuf_flush_page_cleaner()when the log is being rebuilt.srv_prepare_to_delete_redo_log_file(): Invokerecv_sys.apply(true)and mark the recovery as completed, before checking for one more precondition and invokingbuf_flush_sync_batch(LSN_MAX). Remove some duplicated assertions as well as a redundant call tolog_write_up_to()from the end. If we crash while rebuilding a latest_format log file, the subsequent startup will perform crash recovery normally. This avoids a redundant write to the old log file during log file resizing.