MDEV-24313 (2 of 2): Silently ignored innodb_use_native_aio=1bb-10.5-MDEV-24313

In commit 5e62b6a5e06eb02cbde1e34e95e26f42d87fce02 (MDEV-16264)
the logic of os_aio_init() was changed so that it will never fail,
but instead automatically disable innodb_use_native_aio (which is
enabled by default) if the io_setup() system call would fail due
to resource limits being exceeded. This is questionable, especially
because falling back to simulated AIO may lead to significantly
reduced performance.

srv_n_file_io_threads, srv_n_read_io_threads, srv_n_write_io_threads:
Change the data type from ulong to uint.

os_aio_init(): Remove the parameters, and actually return an error code.

thread_pool::configure_aio(): Do not silently fall back to simulated AIO.

Reviewed by: Vladislav Vaintroub

10 files changed, 58 insertions, 83 deletions

diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc
index 94b10019e1d..3fef49e787a 100644
--- a/extra/mariabackup/xtrabackup.cc
+++ b/extra/mariabackup/xtrabackup.cc
@@ -2128,9 +2128,9 @@ static bool innodb_init_param()
 	srv_buf_pool_size = (ulint) xtrabackup_use_memory;
 	srv_buf_pool_chunk_unit = (ulong)srv_buf_pool_size;
 
-	srv_n_file_io_threads = (ulint) innobase_file_io_threads;
-	srv_n_read_io_threads = innobase_read_io_threads;
-	srv_n_write_io_threads = innobase_write_io_threads;
+	srv_n_file_io_threads = (uint) innobase_file_io_threads;
+	srv_n_read_io_threads = (uint) innobase_read_io_threads;
+	srv_n_write_io_threads = (uint) innobase_write_io_threads;
 
 	srv_max_n_open_files = ULINT_UNDEFINED - 5;
 
@@ -4273,8 +4273,10 @@ fail:
 	xb_fil_io_init();
 	srv_n_file_io_threads = srv_n_read_io_threads;
 
-	os_aio_init(srv_n_read_io_threads, srv_n_write_io_threads,
-		    SRV_MAX_N_PENDING_SYNC_IOS);
+	if (os_aio_init()) {
+		msg("Error: cannot initialize AIO subsystem");
+		goto fail;
+	}
 
 	log_sys.create();
 	log_sys.log.create();
diff --git a/mysql-test/suite/sys_vars/r/sysvars_innodb,32bit.rdiff b/mysql-test/suite/sys_vars/r/sysvars_innodb,32bit.rdiff
index 78d3689e69d..ee9a1eee717 100644
--- a/mysql-test/suite/sys_vars/r/sysvars_innodb,32bit.rdiff
+++ b/mysql-test/suite/sys_vars/r/sysvars_innodb,32bit.rdiff
@@ -371,15 +371,6 @@
  VARIABLE_COMMENT	Number of pages that must be accessed sequentially for InnoDB to trigger a readahead.
  NUMERIC_MIN_VALUE	0
  NUMERIC_MAX_VALUE	64
-@@ -1609,7 +1609,7 @@
- SESSION_VALUE	NULL
- DEFAULT_VALUE	4
- VARIABLE_SCOPE	GLOBAL
--VARIABLE_TYPE	BIGINT UNSIGNED
-+VARIABLE_TYPE	INT UNSIGNED
- VARIABLE_COMMENT	Number of background read I/O threads in InnoDB.
- NUMERIC_MIN_VALUE	1
- NUMERIC_MAX_VALUE	64
 @@ -1705,7 +1705,7 @@
  SESSION_VALUE	NULL
  DEFAULT_VALUE	1048576
@@ -429,12 +420,3 @@
  VARIABLE_COMMENT	Number of undo tablespaces to use.
  NUMERIC_MIN_VALUE	0
  NUMERIC_MAX_VALUE	127
-@@ -2053,7 +2053,7 @@
- SESSION_VALUE	NULL
- DEFAULT_VALUE	4
- VARIABLE_SCOPE	GLOBAL
--VARIABLE_TYPE	BIGINT UNSIGNED
-+VARIABLE_TYPE	INT UNSIGNED
- VARIABLE_COMMENT	Number of background write I/O threads in InnoDB.
- NUMERIC_MIN_VALUE	2
- NUMERIC_MAX_VALUE	64
diff --git a/mysql-test/suite/sys_vars/r/sysvars_innodb.result b/mysql-test/suite/sys_vars/r/sysvars_innodb.result
index ef4c337d471..591471cbbff 100644
--- a/mysql-test/suite/sys_vars/r/sysvars_innodb.result
+++ b/mysql-test/suite/sys_vars/r/sysvars_innodb.result
@@ -1621,7 +1621,7 @@ VARIABLE_NAME	INNODB_READ_IO_THREADS
 SESSION_VALUE	NULL
 DEFAULT_VALUE	4
 VARIABLE_SCOPE	GLOBAL
-VARIABLE_TYPE	BIGINT UNSIGNED
+VARIABLE_TYPE	INT UNSIGNED
 VARIABLE_COMMENT	Number of background read I/O threads in InnoDB.
 NUMERIC_MIN_VALUE	1
 NUMERIC_MAX_VALUE	64
@@ -2065,7 +2065,7 @@ VARIABLE_NAME	INNODB_WRITE_IO_THREADS
 SESSION_VALUE	NULL
 DEFAULT_VALUE	4
 VARIABLE_SCOPE	GLOBAL
-VARIABLE_TYPE	BIGINT UNSIGNED
+VARIABLE_TYPE	INT UNSIGNED
 VARIABLE_COMMENT	Number of background write I/O threads in InnoDB.
 NUMERIC_MIN_VALUE	2
 NUMERIC_MAX_VALUE	64
diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index a93e4284452..3f3f164e505 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -3756,9 +3756,6 @@ static int innodb_init_params()
 	}
 
 #ifdef LINUX_NATIVE_AIO
-	if (srv_use_native_aio) {
-		ib::info() << "Using Linux native AIO";
-	}
 #elif !defined _WIN32
 	/* Currently native AIO is supported only on windows and linux
 	and that also when the support is compiled in. In all other
@@ -19431,12 +19428,12 @@ static MYSQL_SYSVAR_BOOL(optimize_fulltext_only, innodb_optimize_fulltext_only,
   "Only optimize the Fulltext index of the table",
   NULL, NULL, FALSE);
 
-static MYSQL_SYSVAR_ULONG(read_io_threads, srv_n_read_io_threads,
+static MYSQL_SYSVAR_UINT(read_io_threads, srv_n_read_io_threads,
   PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
   "Number of background read I/O threads in InnoDB.",
   NULL, NULL, 4, 1, 64, 0);
 
-static MYSQL_SYSVAR_ULONG(write_io_threads, srv_n_write_io_threads,
+static MYSQL_SYSVAR_UINT(write_io_threads, srv_n_write_io_threads,
   PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
   "Number of background write I/O threads in InnoDB.",
   NULL, NULL, 4, 2, 64, 0);
diff --git a/storage/innobase/include/os0file.h b/storage/innobase/include/os0file.h
index 149da54b63c..ab1d5bb1dea 100644
--- a/storage/innobase/include/os0file.h
+++ b/storage/innobase/include/os0file.h
@@ -1135,21 +1135,9 @@ void
 unit_test_os_file_get_parent_dir();
 #endif /* UNIV_ENABLE_UNIT_TEST_GET_PARENT_DIR */
 
-/** Initializes the asynchronous io system. Creates one array each for ibuf
-and log i/o. Also creates one array each for read and write where each
-array is divided logically into n_read_segs and n_write_segs
-respectively. The caller must create an i/o handler thread for each
-segment in these arrays. This function also creates the sync array.
-No i/o handler thread needs to be created for that
-@param[in]	n_read_segs	number of reader threads
-@param[in]	n_write_segs	number of writer threads
-@param[in]	n_slots_sync	number of slots in the sync aio array */
-
-bool
-os_aio_init(
-	ulint		n_read_segs,
-	ulint		n_write_segs,
-	ulint		n_slots_sync);
+/**
+Initializes the asynchronous io system. */
+int os_aio_init();
 
 /**
 Frees the asynchronous io system. */
diff --git a/storage/innobase/include/srv0srv.h b/storage/innobase/include/srv0srv.h
index 7b65000c115..7c90c7f8864 100644
--- a/storage/innobase/include/srv0srv.h
+++ b/storage/innobase/include/srv0srv.h
@@ -337,11 +337,11 @@ extern ulong srv_buf_pool_load_pages_abort;
 /** Lock table size in bytes */
 extern ulint	srv_lock_table_size;
 
-extern ulint	srv_n_file_io_threads;
+extern uint	srv_n_file_io_threads;
 extern my_bool	srv_random_read_ahead;
 extern ulong	srv_read_ahead_threshold;
-extern ulong	srv_n_read_io_threads;
-extern ulong	srv_n_write_io_threads;
+extern uint	srv_n_read_io_threads;
+extern uint	srv_n_write_io_threads;
 
 /* Defragmentation, Origianlly facebook default value is 100, but it's too high */
 #define SRV_DEFRAGMENT_FREQUENCY_DEFAULT 40
diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc
index 7286b6b62f5..dcadeea3525 100644
--- a/storage/innobase/os/os0file.cc
+++ b/storage/innobase/os/os0file.cc
@@ -3887,7 +3887,6 @@ versions where native aio is supported it won't work on tmpfs. In such
 cases we can't use native aio.
 
 @return: true if supported, false otherwise. */
-#include <libaio.h>
 static bool is_linux_native_aio_supported()
 {
 	File		fd;
@@ -4009,34 +4008,40 @@ static bool is_linux_native_aio_supported()
 }
 #endif
 
-
-
-bool os_aio_init(ulint n_reader_threads, ulint n_writer_threads, ulint)
+int os_aio_init()
 {
-  int max_write_events= int(n_writer_threads * OS_AIO_N_PENDING_IOS_PER_THREAD);
-  int max_read_events= int(n_reader_threads * OS_AIO_N_PENDING_IOS_PER_THREAD);
-  int max_events = max_read_events + max_write_events;
-	int ret;
-
+  int max_write_events= int(srv_n_write_io_threads) *
+    OS_AIO_N_PENDING_IOS_PER_THREAD;
+  int max_read_events= int(srv_n_read_io_threads) *
+    OS_AIO_N_PENDING_IOS_PER_THREAD;
+  int max_events= max_read_events + max_write_events;
+  int ret;
 #if LINUX_NATIVE_AIO
-	if (srv_use_native_aio && !is_linux_native_aio_supported())
-		srv_use_native_aio = false;
+  if (srv_use_native_aio && !is_linux_native_aio_supported())
+    goto disable;
 #endif
-	ret = srv_thread_pool->configure_aio(srv_use_native_aio, max_events);
-	if(ret) {
-		ut_a(srv_use_native_aio);
-		srv_use_native_aio = false;
+
+  ret= srv_thread_pool->configure_aio(srv_use_native_aio, max_events);
+
 #ifdef LINUX_NATIVE_AIO
-		ib::info() << "Linux native AIO disabled";
+  if (ret)
+  {
+    ut_ad(srv_use_native_aio);
+disable:
+    ib::warn() << "Linux Native AIO disabled.";
+    ret= srv_thread_pool->configure_aio(srv_use_native_aio= false, max_events);
+  }
 #endif
-		ret = srv_thread_pool->configure_aio(srv_use_native_aio, max_events);
-		DBUG_ASSERT(!ret);
-	}
-	read_slots = new io_slots(max_read_events, (uint)n_reader_threads);
-	write_slots = new io_slots(max_write_events, (uint)n_writer_threads);
-	return true;
+
+  if (!ret)
+  {
+    read_slots= new io_slots(max_read_events, srv_n_read_io_threads);
+    write_slots= new io_slots(max_write_events, srv_n_write_io_threads);
+  }
+  return ret;
 }
 
+
 void os_aio_free()
 {
   srv_thread_pool->disable_aio();
@@ -4154,8 +4159,8 @@ os_aio_print(FILE*	file)
 	time_t		current_time;
 	double		time_elapsed;
 
-	for (ulint i = 0; i < srv_n_file_io_threads; ++i) {
-		fprintf(file, "I/O thread " ULINTPF " state: %s (%s)",
+	for (uint i = 0; i < srv_n_file_io_threads; ++i) {
+		fprintf(file, "I/O thread %u state: %s (%s)",
 			i,
 			srv_io_thread_op_info[i],
 			srv_io_thread_function[i]);
diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc
index 71ae6e04881..42cb76dcf76 100644
--- a/storage/innobase/srv/srv0srv.cc
+++ b/storage/innobase/srv/srv0srv.cc
@@ -228,9 +228,9 @@ ulint	srv_lock_table_size	= ULINT_MAX;
 ulong	srv_idle_flush_pct;
 
 /** innodb_read_io_threads */
-ulong	srv_n_read_io_threads;
+uint	srv_n_read_io_threads;
 /** innodb_write_io_threads */
-ulong	srv_n_write_io_threads;
+uint	srv_n_write_io_threads;
 
 /** innodb_random_read_ahead */
 my_bool	srv_random_read_ahead;
diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
index a25335eba84..b7c4baa0ef7 100644
--- a/storage/innobase/srv/srv0start.cc
+++ b/storage/innobase/srv/srv0start.cc
@@ -111,7 +111,7 @@ lsn_t	srv_shutdown_lsn;
 ibool	srv_start_raw_disk_in_use;
 
 /** Number of IO threads to use */
-ulint	srv_n_file_io_threads;
+uint	srv_n_file_io_threads;
 
 /** UNDO tablespaces starts with space id. */
 ulint	srv_undo_space_id_start;
@@ -1192,9 +1192,7 @@ dberr_t srv_start(bool create_new_db)
 		return(srv_init_abort(err));
 	}
 
-	srv_n_file_io_threads = srv_n_read_io_threads;
-
-	srv_n_file_io_threads += srv_n_write_io_threads;
+	srv_n_file_io_threads = srv_n_read_io_threads + srv_n_write_io_threads;
 
 	if (!srv_read_only_mode) {
 		/* Add the log and ibuf IO threads. */
@@ -1206,15 +1204,18 @@ dberr_t srv_start(bool create_new_db)
 
 	ut_a(srv_n_file_io_threads <= SRV_MAX_N_IO_THREADS);
 
-	if (!os_aio_init(srv_n_read_io_threads,
-			 srv_n_write_io_threads,
-			 SRV_MAX_N_PENDING_SYNC_IOS)) {
-
+	if (os_aio_init()) {
 		ib::error() << "Cannot initialize AIO sub-system";
 
 		return(srv_init_abort(DB_ERROR));
 	}
 
+#ifdef LINUX_NATIVE_AIO
+	if (srv_use_native_aio) {
+		ib::info() << "Using Linux native AIO";
+	}
+#endif
+
 	fil_system.create(srv_file_per_table ? 50000 : 5000);
 
 	ib::info() << "Initializing buffer pool, total size = "
diff --git a/tpool/tpool.h b/tpool/tpool.h
index 0d83af5bd74..3a5658c0f36 100644
--- a/tpool/tpool.h
+++ b/tpool/tpool.h
@@ -220,7 +220,7 @@ public:
   {
     if (use_native_aio)
       m_aio.reset(create_native_aio(max_io));
-    if (!m_aio)
+    else
       m_aio.reset(create_simulated_aio(this));
     return !m_aio ? -1 : 0;
   }