MDEV-31956 SSD based InnoDB buffer pool extension

Code cleanup

Author: vlad-lesin

mysql-test/suite/innodb/t/ext_buf_pool.test

@@ -25,12 +25,6 @@ START TRANSACTION WITH CONSISTENT SNAPSHOT;
 
 --let $DATADIR = `select @@datadir`
 
-#--let $restart_parameters=--innodb-extended-buffer-pool-size=10M
-#--source include/restart_mysqld.inc
-
-# Set ext buffer pool file size and check it was created, check it's size
-#--list_files $DATADIR ext_buffer_pool
-
 --disable_query_log
 --error 0,ER_UNKNOWN_SYSTEM_VARIABLE
 SET @old_innodb_limit_optimistic_insert_debug = @@innodb_limit_optimistic_insert_debug;
@@ -169,13 +163,3 @@ SET GLOBAL innodb_limit_optimistic_insert_debug = @old_innodb_limit_optimistic_i
 
 --disconnect prevent_purge
 --source include/wait_until_count_sessions.inc
-
-
-# Restart server with different buffer pool size, check the size is changed
-
-# Restart server with different ext buffer pool location, check it was created
-
-# Restart server with some impossible ext buffer pool location, check on error
-
-# Check extended buffer pool file is not copied during backup and created
-# during backup restoring

storage/innobase/buf/buf0buf.cc

@@ -1651,7 +1651,6 @@ void buf_pool_t::close() noexcept
   page_hash.free();
 
   io_buf.close();
-
   aligned_free(const_cast<byte*>(field_ref_zero));
   field_ref_zero= nullptr;
 }
@@ -3744,11 +3743,8 @@ dberr_t buf_page_t::read_complete(const fil_node_t &node,
     ut_ad(f > READ_FIX);
     ut_ad(f < WRITE_FIX);
   }
-  else
-  {
-    if (!recv_recover_page(node.space, this))
-      return DB_PAGE_CORRUPTED;
-  }
+  else if (!recv_recover_page(node.space, this))
+    return DB_PAGE_CORRUPTED;
 
   if (UNIV_UNLIKELY(MONITOR_IS_ON(MONITOR_MODULE_BUF_PAGE)))
     buf_page_monitor(*this, true);

storage/innobase/buf/buf0flu.cc

@@ -762,7 +762,7 @@ ATTRIBUTE_COLD void buf_pool_t::release_freed_page(buf_page_t *bpage) noexcept
 
 /** Write a flushable page to a file or free a freeable block.
 @param space       tablespace
-@param to_ext_buf  wherher to write the page to external buffer pull file
+@param to_ext_buf  whether to write the page to external buffer pull file
 @return whether a page write was initiated and buf_pool.mutex released */
 bool buf_page_t::flush(fil_space_t *space, bool to_ext_buf) noexcept
 {
@@ -2987,6 +2987,7 @@ void buf_flush_sync() noexcept
   thd_wait_begin(nullptr, THD_WAIT_DISKIO);
   tpool::tpool_wait_begin();
   log_sys.latch.wr_lock(SRW_LOCK_CALL);
+
   for (lsn_t lsn= log_sys.get_lsn();;)
   {
     log_sys.latch.wr_unlock();
@@ -2997,7 +2998,6 @@ void buf_flush_sync() noexcept
     while (buf_flush_sync_lsn)
       my_cond_wait(&buf_pool.done_flush_list,
                    &buf_pool.flush_list_mutex.m_mutex);
-
     --buf_pool.done_flush_list_waiters_count;
     mysql_mutex_unlock(&buf_pool.flush_list_mutex);
     log_sys.latch.wr_lock(SRW_LOCK_CALL);

storage/innobase/buf/buf0rea.cc

@@ -66,7 +66,6 @@ struct page_init_result {
                                read from from exretnal buffer pool file */
 };
 
-
 /** Initialize a page for read to the buffer buf_pool. If the page is
 (1) already in buf_pool, or
 (2) if the tablespace has been or is being deleted,

storage/innobase/fil/fil0fil.cc

@@ -1754,14 +1754,6 @@ fil_space_t *fil_space_t::drop(uint32_t id, pfs_os_file_t *detached_handle)
   return space;
 }
 
-void fil_space_t::remove_file_low()
-{
-  fil_node_t *node= chain.start;
-  ut_ad(node);
-  ut_ad(!node->is_open());
-  os_file_delete(innodb_data_file_key, node->name);
-}
-
 /** Close a single-table tablespace on failed IMPORT TABLESPACE.
 The tablespace must be cached in the memory cache.
 Free all pages used by the tablespace. */
@@ -2916,8 +2908,8 @@ fil_io_t fil_space_t::io(const IORequest &type, os_offset_t offset, size_t len,
 		goto release_sync_write;
 	} else {
 		/* Queue the aio request */
-		err = os_aio(IORequest{bpage, type.slot, node, type.type}, buf,
-		    offset, len);
+		err = os_aio(IORequest{bpage, type.slot, node, type.type},
+			     buf, offset, len);
 	}
 
 	if (!type.is_async()) {
@@ -2937,11 +2929,11 @@ fil_io_t fil_space_t::io(const IORequest &type, os_offset_t offset, size_t len,
 	return {err, node};
 }
 
-bool fil_system_t::create_ext_file() {
+bool fil_system_t::create_ext_file() noexcept {
   bool ret;
   ext_bp_file= pfs_create_temp_file(
       ext_bp_path ? ext_bp_path : fil_path_to_mysql_datadir,
-      "/Extended buffer pool file", "ext_buf_", 0);
+      "/Extended buffer pool file", "ext_buf_");
   if (ext_bp_file == OS_FILE_CLOSED)
   {
     sql_print_error("Cannot open/create extended buffer pool file");
@@ -3067,8 +3059,12 @@ void IORequest::read_complete(int io_error) const noexcept
   {
     sql_print_error("InnoDB: Read error %d of page " UINT32PF " in file %s",
                     io_error, id.page_no(),
-                    ext_buf() ? "of external buffer pool" : node_ptr->name);
-    if (!ext_buf())
+                    ext_buf() ? "of external buffer pool, external buffer "
+                                "pool is disabled"
+                              : node_ptr->name);
+    if (ext_buf())
+      fil_system.ext_buf_pool_disable();
+    else
       recv_sys.free_corrupted_page(id, *node_ptr);
     buf_pool.corrupted_evict(buf_page, buf_page_t::READ_FIX + 1);
   corrupted:
@@ -3455,13 +3451,8 @@ fil_space_t *fil_space_t::prev_in_unflushed_spaces() noexcept
 
 #endif
 
-/** Create temporary merge files in the given paramater path, and if
-UNIV_PFS_IO defined, register the file descriptor with Performance Schema.
-@param[in]	path	location for creating temporary merge files, or NULL
-@return File descriptor */
-pfs_os_file_t pfs_create_temp_file(const char *path,
-                                   const char *label, const char *prefix,
-                                   int mode)
+pfs_os_file_t pfs_create_temp_file(const char *path, const char *label,
+                                   const char *prefix)
 {
   if (!path)
   {

storage/innobase/handler/ha_innodb.cc

@@ -3671,6 +3671,9 @@ static void innodb_force_LRU_eviction_set(THD *, st_mysql_sys_var *, void *,
   buf_pool.force_LRU_eviction_to_ebp= *static_cast<const my_bool *>(save);
   if (buf_pool.force_LRU_eviction_to_ebp)
   {
+    /* Wake up page cleaner twice, the first one is to flush dirty pages to
+    data files, the second one is to flush clean pages to external buffer pool.
+    */
     mysql_mutex_lock(&buf_pool.flush_list_mutex);
     buf_pool.page_cleaner_wakeup(true);
     my_cond_wait(&buf_pool.done_flush_list,

storage/innobase/include/buf0buf.h

@@ -866,24 +866,6 @@ class buf_page_t : public buf_page_base_t
   unsigned is_accessed() const noexcept
   { ut_ad(in_file()); return access_time; }
 };
-/* TODO: offsetof() doesn't work on non-standard classes
-static_assert(offsetof(ext_buf_page_t, id) == offsetof(buf_page_t, id_), "");
-static_assert(offsetof(ext_buf_page_t, hash) == offsetof(buf_page_t, hash),
-              "");
-static_assert(offsetof(ext_buf_page_t, frame) == offsetof(buf_page_t, frame),
-              "");
-#ifdef UNIV_DEBUG
-static_assert(offsetof(ext_buf_page_t, in_LRU_list) ==
-                  offsetof(buf_page_t, in_LRU_list),
-              "");
-static_assert(offsetof(ext_buf_page_t, in_page_hash) ==
-                  offsetof(buf_page_t, in_page_hash),
-              "");
-static_assert(offsetof(ext_buf_page_t, in_free_list) ==
-                  offsetof(buf_page_t, in_free_list),
-              "");
-#endif
-*/
 
 /** The buffer control block structure */
 
@@ -1191,6 +1173,8 @@ class buf_pool_t
   ext_buf_pages_array */
   size_t extended_pages;
 #ifdef UNIV_DEBUG
+  /** Shows if force LRU eviction to external buffer poll is currently on.
+  Debug only. */
   my_bool force_LRU_eviction_to_ebp;
 #endif
   /** Hash cell chain in page_hash_table */

storage/innobase/include/fil0fil.h

@@ -524,8 +524,6 @@ struct fil_space_t final
   another concurrent thread */
   static fil_space_t *drop(uint32_t id, pfs_os_file_t *detached_handle);
 
-  void remove_file_low();
-
 private:
   MY_ATTRIBUTE((warn_unused_result))
   /** Try to acquire a tablespace reference (increment referenced()).
@@ -1420,6 +1418,9 @@ struct fil_system_t
 #ifdef __linux__
   /** available block devices that reside on non-rotational storage */
   std::vector<dev_t> ssd;
+
+  /** External buffer pool file handler */
+  pfs_os_file_t ext_bp_file;
 public:
   /** @return whether a file system device is on non-rotational storage */
   bool is_ssd(dev_t dev) const noexcept
@@ -1445,9 +1446,6 @@ struct fil_system_t
   mysql_mutex_t mutex;
 	fil_space_t*	sys_space;	/*!< The innodb_system tablespace */
 	fil_space_t*	temp_space;	/*!< The innodb_temporary tablespace */
-private:
-  pfs_os_file_t ext_bp_file;
-
 public:
 
   /** Extended buffer pool file path */
@@ -1457,14 +1455,6 @@ struct fil_system_t
   not used. */
   size_t ext_bp_size;
 
-  /** Create external buffer pool file.
-  @return whether the creation failed */
-  bool create_ext_file();
-  dberr_t ext_bp_io(buf_page_t &bpage, ext_buf_page_t &ext_buf_page,
-                    IORequest::Type io_request_type, buf_tmp_buffer_t *slot,
-                    size_t len, void *buf) noexcept;
-  bool ext_buf_pool_enabled() const { return ext_bp_size; }
-  void ext_buf_pool_disable() { ext_bp_size= 0; }
   /** Map of fil_space_t::id to fil_space_t* */
   hash_table_t spaces;
 
@@ -1522,6 +1512,33 @@ struct fil_system_t
   potential space_id reuse */
   bool space_id_reuse_warned;
 
+  /** Create external buffer pool file.
+  @return whether the creation failed */
+  bool create_ext_file() noexcept;
+
+  /** External bufer pool os_aio() wrapper.
+  @param bpage           buffer pool page for read/write
+  @param ext_buf_page    external buffer pool page which will be freed on read
+                         completion and replace bpage in buffer pool on write
+                         completion
+  @param io_request_type IORequest::WRITE_ASYNC, IORequest::READ_SYNC or
+                         IORequest::READ_ASYNC
+  @param slot            memory to be used for encrypted or page_compressed
+                         pages
+  @param len             length to read/write
+  @param buf             buffer
+  @retval DB_SUCCESS if request was queued successfully
+  @retval DB_IO_ERROR on I/O error */
+  dberr_t ext_bp_io(buf_page_t &bpage, ext_buf_page_t &ext_buf_page,
+                    IORequest::Type io_request_type, buf_tmp_buffer_t *slot,
+                    size_t len, void *buf) noexcept;
+
+  /** Returns if external buffer pool is enabled. */
+  bool ext_buf_pool_enabled() const { return ext_bp_size; }
+
+  /** Disable external boffer pool */
+  void ext_buf_pool_disable() { ext_bp_size= 0; }
+
   /** Add the file to the end of opened spaces list in
   fil_system.space_list, so that fil_space_t::try_to_close() should close
   it as a last resort.
@@ -1858,7 +1875,12 @@ ulint fil_space_get_block_size(const fil_space_t* space, unsigned offset)
 bool fil_crypt_check(fil_space_crypt_t *crypt_data, const char *f_name)
   noexcept;
 
+/** Create temporary files in the given paramater path, and if
+UNIV_PFS_IO defined, register the file descriptor with Performance Schema.
+@param path   location for creating temporary merge files, or NULL
+@param label  label for registration in Performance Schema if path == nullptr
+@param prefix temporary file name prefix
+@return File descriptor */
 pfs_os_file_t pfs_create_temp_file(const char *path, const char *label,
-                                   const char *prefix, int mode);
-
+                                   const char *prefix);
 #endif /* UNIV_INNOCHECKSUM */

storage/innobase/os/os0file.cc

@@ -3347,21 +3347,22 @@ dberr_t os_aio(const IORequest &type, void *buf, os_offset_t offset,
 }
 
 /** Request a read or write.
-@param type		I/O request
-@param buf		buffer
-@param offset		file offset
-@param n		number of bytes
+@param type        I/O request
+@param buf         buffer
+@param offset      file offset
+@param n           number of bytes
 @retval DB_SUCCESS if request was queued successfully
 @retval DB_IO_ERROR on I/O error */
-dberr_t os_aio(const IORequest &type, void *buf, os_offset_t offset, size_t n)
-  noexcept {
-	ut_ad(type.node());
-	ut_ad(type.node()->is_open());
-	dberr_t err = os_aio(type, buf, offset, n, type.node()->handle,
-	    type.node()->name);
-	if (err == DB_IO_ERROR)
-	  type.node()->space->release();
-	return err;
+dberr_t os_aio(const IORequest &type, void *buf, os_offset_t offset,
+               size_t n) noexcept
+{
+  ut_ad(type.node());
+  ut_ad(type.node()->is_open());
+  dberr_t err=
+      os_aio(type, buf, offset, n, type.node()->handle, type.node()->name);
+  if (err == DB_IO_ERROR)
+    type.node()->space->release();
+  return err;
 }
 
 void os_aio_print(FILE *file) noexcept

storage/innobase/row/row0merge.cc

@@ -4347,9 +4347,8 @@ void row_merge_drop_temp_indexes()
 @return File descriptor */
 pfs_os_file_t row_merge_file_create_low(const char *path)
 {
-  auto fd=
-      pfs_create_temp_file(path, merge_temp_file_label, merge_temp_file_prefix,
-                           O_BINARY | O_SEQUENTIAL);
+  auto fd= pfs_create_temp_file(path, merge_temp_file_label,
+                                merge_temp_file_prefix);
   if (fd == OS_FILE_CLOSED)
   {
     ib::error() << "Cannot create temporary merge file";
@@ -4371,13 +4370,7 @@ row_merge_file_create(
 	merge_file->fd =
 		pfs_create_temp_file(path,
 				     merge_temp_file_label,
-				     merge_temp_file_prefix,
-#if !defined _WIN32 && defined O_DIRECT
-				     srv_disable_sort_file_cache
-				     ? O_DIRECT | O_BINARY | O_SEQUENTIAL
-				     :
-#endif
-				     O_BINARY | O_SEQUENTIAL);
+				     merge_temp_file_prefix);
 	if (merge_file->fd == OS_FILE_CLOSED)
 	{
 	  ib::error() << "Cannot create temporary merge file";