MDEV-18115: Remove the only async write (of redo log)

TODO: do not use fil_* functions for redo log files. log_t::checkpoint_lock: remove this lock which was used to wait for async I/O completion. checkpoint_lock_key checkpoint_lock: remove now unneeded globals log_write_checkpoint_info(): remove sync argument because all checkpoint writes are synchronous now log_write_checkpoint_info(): remove sync argument log_group_checkpoint(): merge with the only caller log_complete_checkpoint(): merge with the only caller log_t::complete_checkpoint(): remove by merging with the only caller.
author: Eugene Kosov <claprix@yandex.ru> 2019-10-22 18:52:56 +0200
committer: Eugene Kosov <claprix@yandex.ru> 2019-10-29 22:03:02 +0300
commit: 7440e61a64489a7eba9d0384366ab7aad99efe30 (patch)
tree: 3dcbe2024ef165f21f719cbda6fa0b88ad6758df /storage
parent: 2b710090aa51914e47d5066b7ce49e6cff3ea32e (diff)
download: mariadb-git-7440e61a64489a7eba9d0384366ab7aad99efe30.tar.gz
9 files changed, 82 insertions, 193 deletions
diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 4c512335112..8b2d5183702 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -4376,77 +4376,47 @@ fil_aio_wait(
 
 	ut_ad(fil_validate_skip());
 
+	ut_ad(purpose != FIL_TYPE_LOG);
+
 	/* Do the i/o handling */
 	/* IMPORTANT: since i/o handling for reads will read also the insert
 	buffer in tablespace 0, you have to be very careful not to introduce
 	deadlocks in the i/o system. We keep tablespace 0 data files always
 	open, and use a special i/o thread to serve insert buffer requests. */
 
-	switch (purpose) {
-	case FIL_TYPE_LOG:
-		srv_set_io_thread_op_info(segment, "complete io for log");
-		/* We use synchronous writing of the logs
-		and can only end up here when writing a log checkpoint! */
-		ut_a(ptrdiff_t(message) == 1);
-		/* It was a checkpoint write */
-		switch (srv_flush_t(srv_file_flush_method)) {
-		case SRV_O_DSYNC:
-		case SRV_NOSYNC:
-			break;
-		case SRV_FSYNC:
-		case SRV_LITTLESYNC:
-		case SRV_O_DIRECT:
-		case SRV_O_DIRECT_NO_FSYNC:
-#ifdef _WIN32
-		case SRV_ALL_O_DIRECT_FSYNC:
-#endif
-			fil_flush(SRV_LOG_SPACE_FIRST_ID);
-		}
+	srv_set_io_thread_op_info(segment, "complete io for buf page");
 
-		DBUG_PRINT("ib_log", ("checkpoint info written"));
-		log_sys.complete_checkpoint();
+	/* async single page writes from the dblwr buffer don't have
+	access to the page */
+	buf_page_t* bpage = static_cast<buf_page_t*>(message);
+	if (!bpage) {
 		return;
-	case FIL_TYPE_TABLESPACE:
-	case FIL_TYPE_TEMPORARY:
-	case FIL_TYPE_IMPORT:
-		srv_set_io_thread_op_info(segment, "complete io for buf page");
+	}
 
-		/* async single page writes from the dblwr buffer don't have
-		access to the page */
-		buf_page_t* bpage = static_cast<buf_page_t*>(message);
-		if (!bpage) {
-			return;
-		}
+	ulint offset = bpage->id.page_no();
+	if (dblwr && bpage->init_on_flush) {
+		bpage->init_on_flush = false;
+		dblwr = false;
+	}
+	err = buf_page_io_complete(bpage, dblwr);
+	if (err == DB_SUCCESS) {
+		return;
+	}
 
-		ulint offset = bpage->id.page_no();
-		if (dblwr && bpage->init_on_flush) {
-			bpage->init_on_flush = false;
-			dblwr = false;
-		}
-		dberr_t err = buf_page_io_complete(bpage, dblwr);
-		if (err == DB_SUCCESS) {
-			return;
-		}
+	ut_ad(type.is_read());
+	if (recv_recovery_is_on() && !srv_force_recovery) {
+		recv_sys.found_corrupt_fs = true;
+	}
 
-		ut_ad(type.is_read());
-		if (recv_recovery_is_on() && !srv_force_recovery) {
-			recv_sys.found_corrupt_fs = true;
+	if (fil_space_t* space = fil_space_acquire_for_io(space_id)) {
+		if (space == node->space) {
+			ib::error() << "Failed to read file '" << node->name
+				    << "' at offset " << offset << ": "
+				    << ut_strerr(err);
 		}
 
-		if (fil_space_t* space = fil_space_acquire_for_io(space_id)) {
-			if (space == node->space) {
-				ib::error() << "Failed to read file '"
-					    << node->name
-					    << "' at offset " << offset
-					    << ": " << ut_strerr(err);
-			}
-
-			space->release_for_io();
-		}
-		return;
+		space->release_for_io();
 	}
-
-	ut_ad(0);
 }
 
 /**********************************************************************//**
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 00e85fb0f3a..226c6231b05 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -635,7 +635,6 @@ static PSI_rwlock_info all_innodb_rwlocks[] = {
 #  endif /* UNIV_DEBUG */
 	PSI_RWLOCK_KEY(dict_operation_lock),
 	PSI_RWLOCK_KEY(fil_space_latch),
-	PSI_RWLOCK_KEY(checkpoint_lock),
 	PSI_RWLOCK_KEY(fts_cache_rw_lock),
 	PSI_RWLOCK_KEY(fts_cache_init_rw_lock),
 	PSI_RWLOCK_KEY(trx_i_s_cache_lock),
diff --git a/storage/innobase/include/log0log.h b/storage/innobase/include/log0log.h
index cc8625dfa32..fac4ed4ece7 100644
--- a/storage/innobase/include/log0log.h
+++ b/storage/innobase/include/log0log.h
@@ -35,7 +35,6 @@ Created 12/9/1995 Heikki Tuuri
 #define log0log_h
 
 #include "dyn0buf.h"
-#include "sync0rw.h"
 #include "log0types.h"
 #include "os0event.h"
 #include "os0file.h"
@@ -189,9 +188,8 @@ log_buffer_sync_in_background(
 blocks from the buffer pool: it only checks what is lsn of the oldest
 modification in the pool, and writes information about the lsn in
 log files. Use log_make_checkpoint() to flush also the pool.
-@param[in]	sync		whether to wait for the write to complete
 @return true if success, false if a checkpoint write was already running */
-bool log_checkpoint(bool sync);
+bool log_checkpoint();
 
 /** Make a checkpoint */
 void log_make_checkpoint();
@@ -208,10 +206,8 @@ logs_empty_and_mark_files_at_shutdown(void);
 @param[in]	header	0 or LOG_CHECKPOINT_1 or LOG_CHECKPOINT2 */
 void log_header_read(ulint header);
 /** Write checkpoint info to the log header and invoke log_mutex_exit().
-@param[in]	sync	whether to wait for the write to complete
 @param[in]	end_lsn	start LSN of the MLOG_CHECKPOINT mini-transaction */
-void
-log_write_checkpoint_info(bool sync, lsn_t end_lsn);
+void log_write_checkpoint_info(lsn_t end_lsn);
 
 /** Set extra data to be written to the redo log during checkpoint.
 @param[in]	buf	data to be appended on checkpoint, or NULL
@@ -655,10 +651,6 @@ struct log_t{
 	ulint		n_pending_checkpoint_writes;
 					/*!< number of currently pending
 					checkpoint writes */
-	rw_lock_t	checkpoint_lock;/*!< this latch is x-locked when a
-					checkpoint write is running; a thread
-					should wait for this without owning
-					the log mutex */
 
 	/** buffer for checkpoint header */
 	MY_ALIGNED(OS_FILE_LOG_BLOCK_SIZE)
@@ -681,9 +673,6 @@ public:
 
   bool is_initialised() const { return m_initialised; }
 
-  /** Complete an asynchronous checkpoint write. */
-  void complete_checkpoint();
-
   /** @return the log block header + trailer size */
   unsigned framing_size() const
   {
diff --git a/storage/innobase/include/sync0sync.h b/storage/innobase/include/sync0sync.h
index 5562b54c296..a3feefef9f5 100644
--- a/storage/innobase/include/sync0sync.h
+++ b/storage/innobase/include/sync0sync.h
@@ -119,7 +119,6 @@ extern	mysql_pfs_key_t	buf_block_lock_key;
 extern	mysql_pfs_key_t	buf_block_debug_latch_key;
 # endif /* UNIV_DEBUG */
 extern	mysql_pfs_key_t	dict_operation_lock_key;
-extern	mysql_pfs_key_t	checkpoint_lock_key;
 extern	mysql_pfs_key_t	fil_space_latch_key;
 extern	mysql_pfs_key_t	fts_cache_rw_lock_key;
 extern	mysql_pfs_key_t	fts_cache_init_rw_lock_key;
diff --git a/storage/innobase/log/log0log.cc b/storage/innobase/log/log0log.cc
index 200e12c4b53..827ec8eb0cb 100644
--- a/storage/innobase/log/log0log.cc
+++ b/storage/innobase/log/log0log.cc
@@ -275,7 +275,7 @@ log_margin_checkpoint_age(
 		if (!flushed_enough) {
 			os_thread_sleep(100000);
 		}
-		log_checkpoint(true);
+		log_checkpoint();
 
 		log_mutex_enter();
 	}
@@ -569,7 +569,6 @@ void log_t::create()
   n_pending_checkpoint_writes= 0;
 
   last_checkpoint_lsn= lsn;
-  rw_lock_create(checkpoint_lock_key, &checkpoint_lock, SYNC_NO_ORDER_CHECK);
 
   log_block_init(buf, lsn);
   log_block_set_first_rec_group(buf, LOG_BLOCK_HDR_SIZE);
@@ -1161,57 +1160,29 @@ static bool log_preflush_pool_modified_pages(lsn_t new_oldest)
 	return(success);
 }
 
-/******************************************************//**
-Completes a checkpoint. */
-static
-void
-log_complete_checkpoint(void)
-/*=========================*/
+/** Read a log group header page to log_sys.checkpoint_buf.
+@param[in]	header	0 or LOG_CHECKPOINT_1 or LOG_CHECKPOINT2 */
+void log_header_read(ulint header)
 {
 	ut_ad(log_mutex_own());
-	ut_ad(log_sys.n_pending_checkpoint_writes == 0);
-
-	log_sys.next_checkpoint_no++;
-
-	log_sys.last_checkpoint_lsn = log_sys.next_checkpoint_lsn;
-	MONITOR_SET(MONITOR_LSN_CHECKPOINT_AGE,
-		    log_sys.lsn - log_sys.last_checkpoint_lsn);
-
-	DBUG_PRINT("ib_log", ("checkpoint ended at " LSN_PF
-			      ", flushed to " LSN_PF,
-			      log_sys.last_checkpoint_lsn,
-			      log_sys.flushed_to_disk_lsn));
-
-	rw_lock_x_unlock_gen(&(log_sys.checkpoint_lock), LOG_CHECKPOINT);
-}
-
-/** Complete an asynchronous checkpoint write. */
-void log_t::complete_checkpoint()
-{
-	ut_ad(this == &log_sys);
-	MONITOR_DEC(MONITOR_PENDING_CHECKPOINT_WRITE);
-
-	log_mutex_enter();
 
-	ut_ad(n_pending_checkpoint_writes > 0);
+	log_sys.n_log_ios++;
 
-	if (!--n_pending_checkpoint_writes) {
-		log_complete_checkpoint();
-	}
+	MONITOR_INC(MONITOR_LOG_IO);
 
-	log_mutex_exit();
+	fil_io(IORequestLogRead, true,
+	       page_id_t(SRV_LOG_SPACE_FIRST_ID,
+			 header >> srv_page_size_shift),
+	       0, header & (srv_page_size - 1),
+	       OS_FILE_LOG_BLOCK_SIZE, log_sys.checkpoint_buf, NULL);
 }
 
-/** Write checkpoint info to the log header.
+/** Write checkpoint info to the log header and invoke log_mutex_exit().
 @param[in]	end_lsn	start LSN of the MLOG_CHECKPOINT mini-transaction */
-static
-void
-log_group_checkpoint(lsn_t end_lsn)
+void log_write_checkpoint_info(lsn_t end_lsn)
 {
-	lsn_t		lsn_offset;
-
-	ut_ad(!srv_read_only_mode);
 	ut_ad(log_mutex_own());
+	ut_ad(!srv_read_only_mode);
 	ut_ad(end_lsn == 0 || end_lsn >= log_sys.next_checkpoint_lsn);
 	ut_ad(end_lsn <= log_sys.lsn);
 	ut_ad(end_lsn + SIZE_OF_MLOG_CHECKPOINT <= log_sys.lsn
@@ -1232,7 +1203,8 @@ log_group_checkpoint(lsn_t end_lsn)
 		log_crypt_write_checkpoint_buf(buf);
 	}
 
-	lsn_offset = log_sys.log.calc_lsn_offset(log_sys.next_checkpoint_lsn);
+	lsn_t lsn_offset
+		= log_sys.log.calc_lsn_offset(log_sys.next_checkpoint_lsn);
 	mach_write_to_8(buf + LOG_CHECKPOINT_OFFSET, lsn_offset);
 	mach_write_to_8(buf + LOG_CHECKPOINT_LOG_BUF_SIZE,
 			srv_log_buffer_size);
@@ -1249,64 +1221,50 @@ log_group_checkpoint(lsn_t end_lsn)
 	ut_ad(LOG_CHECKPOINT_1 < srv_page_size);
 	ut_ad(LOG_CHECKPOINT_2 < srv_page_size);
 
-	if (log_sys.n_pending_checkpoint_writes++ == 0) {
-		rw_lock_x_lock_gen(&log_sys.checkpoint_lock,
-				   LOG_CHECKPOINT);
-	}
+	++log_sys.n_pending_checkpoint_writes;
+
+	log_mutex_exit();
 
 	/* Note: We alternate the physical place of the checkpoint info.
 	See the (next_checkpoint_no & 1) below. */
 
-	fil_io(IORequestLogWrite, false,
+	fil_io(IORequestLogWrite, true,
 	       page_id_t(SRV_LOG_SPACE_FIRST_ID, 0),
 	       0,
 	       (log_sys.next_checkpoint_no & 1)
 	       ? LOG_CHECKPOINT_2 : LOG_CHECKPOINT_1,
 	       OS_FILE_LOG_BLOCK_SIZE,
-	       buf, reinterpret_cast<void*>(1) /* checkpoint write */);
-}
-
-/** Read a log group header page to log_sys.checkpoint_buf.
-@param[in]	header	0 or LOG_CHECKPOINT_1 or LOG_CHECKPOINT2 */
-void log_header_read(ulint header)
-{
-	ut_ad(log_mutex_own());
+	       buf, nullptr);
 
-	log_sys.n_log_ios++;
+	switch (srv_flush_t(srv_file_flush_method)) {
+	case SRV_O_DSYNC:
+	case SRV_NOSYNC:
+		break;
+	default:
+		fil_flush(SRV_LOG_SPACE_FIRST_ID);
+	}
 
-	MONITOR_INC(MONITOR_LOG_IO);
+	log_mutex_enter();
 
-	fil_io(IORequestLogRead, true,
-	       page_id_t(SRV_LOG_SPACE_FIRST_ID,
-			 header >> srv_page_size_shift),
-	       0, header & (srv_page_size - 1),
-	       OS_FILE_LOG_BLOCK_SIZE, log_sys.checkpoint_buf, NULL);
-}
+	--log_sys.n_pending_checkpoint_writes;
+	ut_ad(log_sys.n_pending_checkpoint_writes == 0);
 
-/** Write checkpoint info to the log header and invoke log_mutex_exit().
-@param[in]	sync	whether to wait for the write to complete
-@param[in]	end_lsn	start LSN of the MLOG_CHECKPOINT mini-transaction */
-void
-log_write_checkpoint_info(bool sync, lsn_t end_lsn)
-{
-	ut_ad(log_mutex_own());
-	ut_ad(!srv_read_only_mode);
+	log_sys.next_checkpoint_no++;
 
-	log_group_checkpoint(end_lsn);
+	log_sys.last_checkpoint_lsn = log_sys.next_checkpoint_lsn;
+	MONITOR_SET(MONITOR_LSN_CHECKPOINT_AGE,
+		    log_sys.lsn - log_sys.last_checkpoint_lsn);
 
-	log_mutex_exit();
+	DBUG_PRINT("ib_log", ("checkpoint ended at " LSN_PF
+			      ", flushed to " LSN_PF,
+			      log_sys.last_checkpoint_lsn,
+			      log_sys.flushed_to_disk_lsn));
 
 	MONITOR_INC(MONITOR_NUM_CHECKPOINT);
 
-	if (sync) {
-		/* Wait for the checkpoint write to complete */
-		rw_lock_s_lock(&log_sys.checkpoint_lock);
-		rw_lock_s_unlock(&log_sys.checkpoint_lock);
+	DBUG_EXECUTE_IF("crash_after_checkpoint", DBUG_SUICIDE(););
 
-		DBUG_EXECUTE_IF(
-			"crash_after_checkpoint",
-			DBUG_SUICIDE(););
-	}
+	log_mutex_exit();
 }
 
 /** Set extra data to be written to the redo log during checkpoint.
@@ -1327,9 +1285,8 @@ log_append_on_checkpoint(
 blocks from the buffer pool: it only checks what is lsn of the oldest
 modification in the pool, and writes information about the lsn in
 log files. Use log_make_checkpoint() to flush also the pool.
-@param[in]	sync		whether to wait for the write to complete
 @return true if success, false if a checkpoint write was already running */
-bool log_checkpoint(bool sync)
+bool log_checkpoint()
 {
 	lsn_t	oldest_lsn;
 
@@ -1426,17 +1383,11 @@ bool log_checkpoint(bool sync)
 		/* A checkpoint write is running */
 		log_mutex_exit();
 
-		if (sync) {
-			/* Wait for the checkpoint write to complete */
-			rw_lock_s_lock(&log_sys.checkpoint_lock);
-			rw_lock_s_unlock(&log_sys.checkpoint_lock);
-		}
-
 		return(false);
 	}
 
 	log_sys.next_checkpoint_lsn = oldest_lsn;
-	log_write_checkpoint_info(sync, end_lsn);
+	log_write_checkpoint_info(end_lsn);
 	ut_ad(!log_mutex_own());
 
 	return(true);
@@ -1451,7 +1402,7 @@ void log_make_checkpoint()
 		/* Flush as much as we can */
 	}
 
-	while (!log_checkpoint(true)) {
+	while (!log_checkpoint()) {
 		/* Force a checkpoint */
 	}
 }
@@ -1494,21 +1445,11 @@ loop:
 
 	checkpoint_age = log_sys.lsn - log_sys.last_checkpoint_lsn;
 
-	bool	checkpoint_sync;
-	bool	do_checkpoint;
-
-	if (checkpoint_age > log_sys.max_checkpoint_age) {
-		/* A checkpoint is urgent: we do it synchronously */
-		checkpoint_sync = true;
-		do_checkpoint = true;
-	} else if (checkpoint_age > log_sys.max_checkpoint_age_async) {
-		/* A checkpoint is not urgent: do it asynchronously */
-		do_checkpoint = true;
-		checkpoint_sync = false;
-		log_sys.check_flush_or_checkpoint = false;
-	} else {
-		do_checkpoint = false;
-		checkpoint_sync = false;
+	ut_ad(log_sys.max_checkpoint_age >= log_sys.max_checkpoint_age_async);
+	const bool do_checkpoint
+		= checkpoint_age > log_sys.max_checkpoint_age_async;
+
+	if (checkpoint_age <= log_sys.max_checkpoint_age) {
 		log_sys.check_flush_or_checkpoint = false;
 	}
 
@@ -1531,12 +1472,7 @@ loop:
 	}
 
 	if (do_checkpoint) {
-		log_checkpoint(checkpoint_sync);
-
-		if (checkpoint_sync) {
-
-			goto loop;
-		}
+		log_checkpoint();
 	}
 }
 
@@ -1930,7 +1866,6 @@ void log_t::close()
   buf = NULL;
 
   os_event_destroy(flush_event);
-  rw_lock_free(&checkpoint_lock);
   mutex_free(&mutex);
   mutex_free(&write_mutex);
   mutex_free(&log_flush_order_mutex);
diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc
index acb72b7cbc9..c40267844a1 100644
--- a/storage/innobase/log/log0recv.cc
+++ b/storage/innobase/log/log0recv.cc
@@ -984,7 +984,7 @@ recv_synchronize_groups()
 	checkpoint info on disk certain */
 
 	if (!srv_read_only_mode) {
-		log_write_checkpoint_info(true, 0);
+		log_write_checkpoint_info(0);
 		log_mutex_enter();
 	}
 }
diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc
index 5da83f51b29..52b4f6ef921 100644
--- a/storage/innobase/srv/srv0srv.cc
+++ b/storage/innobase/srv/srv0srv.cc
@@ -2218,7 +2218,7 @@ srv_master_do_active_tasks(void)
 	/* Make a new checkpoint */
 	if (cur_time % SRV_MASTER_CHECKPOINT_INTERVAL == 0) {
 		srv_main_thread_op_info = "making checkpoint";
-		log_checkpoint(true);
+		log_checkpoint();
 		MONITOR_INC_TIME_IN_MICRO_SECS(
 			MONITOR_SRV_CHECKPOINT_MICROSECOND, counter_time);
 	}
@@ -2297,7 +2297,7 @@ srv_master_do_idle_tasks(void)
 
 	/* Make a new checkpoint */
 	srv_main_thread_op_info = "making checkpoint";
-	log_checkpoint(true);
+	log_checkpoint();
 	MONITOR_INC_TIME_IN_MICRO_SECS(MONITOR_SRV_CHECKPOINT_MICROSECOND,
 				       counter_time);
 }
diff --git a/storage/innobase/sync/sync0debug.cc b/storage/innobase/sync/sync0debug.cc
index 15c0a952cb0..dc33985d416 100644
--- a/storage/innobase/sync/sync0debug.cc
+++ b/storage/innobase/sync/sync0debug.cc
@@ -1459,8 +1459,6 @@ sync_latch_meta_init()
 	LATCH_ADD_RWLOCK(DICT_OPERATION, SYNC_DICT_OPERATION,
 			 dict_operation_lock_key);
 
-	LATCH_ADD_RWLOCK(CHECKPOINT, SYNC_NO_ORDER_CHECK, checkpoint_lock_key);
-
 	LATCH_ADD_RWLOCK(FIL_SPACE, SYNC_FSP, fil_space_latch_key);
 
 	LATCH_ADD_RWLOCK(FTS_CACHE, SYNC_FTS_CACHE, fts_cache_rw_lock_key);
diff --git a/storage/innobase/sync/sync0sync.cc b/storage/innobase/sync/sync0sync.cc
index 6b293fb55f1..3d4aba114cb 100644
--- a/storage/innobase/sync/sync0sync.cc
+++ b/storage/innobase/sync/sync0sync.cc
@@ -101,7 +101,6 @@ mysql_pfs_key_t	buf_block_lock_key;
 # ifdef UNIV_DEBUG
 mysql_pfs_key_t	buf_block_debug_latch_key;
 # endif /* UNIV_DEBUG */
-mysql_pfs_key_t	checkpoint_lock_key;
 mysql_pfs_key_t	dict_operation_lock_key;
 mysql_pfs_key_t	dict_table_stats_key;
 mysql_pfs_key_t	hash_table_locks_key;
author	Eugene Kosov <claprix@yandex.ru>	2019-10-22 18:52:56 +0200
committer	Eugene Kosov <claprix@yandex.ru>	2019-10-29 22:03:02 +0300
commit	7440e61a64489a7eba9d0384366ab7aad99efe30 (patch)
tree	3dcbe2024ef165f21f719cbda6fa0b88ad6758df /storage
parent	2b710090aa51914e47d5066b7ce49e6cff3ea32e (diff)
download	mariadb-git-7440e61a64489a7eba9d0384366ab7aad99efe30.tar.gz