cleanup redo log

class log_file_t: more or less sane RAII wrapper around redo log file
descriptor and its path.

This change is motivated by the need of using that log_file_t somewhere else.

3 files changed, 113 insertions, 51 deletions

diff --git a/storage/innobase/include/log0log.h b/storage/innobase/include/log0log.h
index 4e619a203d9..399ab6d0601 100644
--- a/storage/innobase/include/log0log.h
+++ b/storage/innobase/include/log0log.h
@@ -448,7 +448,36 @@ or the MySQL version that created the redo log file. */
 typedef ib_mutex_t	LogSysMutex;
 typedef ib_mutex_t	FlushOrderMutex;
 
-extern my_bool	srv_read_only_mode;
+/** RAII wrapper over path and file descriptor. Supposed to be used for log
+files only */
+class log_file_t
+{
+public:
+  log_file_t()= default;
+  log_file_t(std::string path) : m_path{std::move(path)} {}
+
+  log_file_t(const log_file_t &)= delete;
+  log_file_t &operator=(const log_file_t &)= delete;
+
+  log_file_t(log_file_t &&rhs);
+  log_file_t &operator=(log_file_t &&rhs);
+
+  ~log_file_t();
+
+  bool open();
+
+  bool is_opened() const { return m_fd != OS_FILE_CLOSED; }
+  const std::string get_path() const { return m_path; }
+
+  bool close();
+  dberr_t read(size_t offset, span<byte> buf);
+  dberr_t write(size_t offset, span<const byte> buf);
+  bool flush_data_only();
+
+private:
+  pfs_os_file_t m_fd;
+  std::string m_path;
+};
 
 /** Redo log buffer */
 struct log_t{
@@ -537,9 +566,7 @@ struct log_t{
     lsn_t				scanned_lsn;
 
     /** file descriptors for all log files */
-    std::vector<pfs_os_file_t> files;
-    /** file names for all log files */
-    std::vector<std::string> file_names;
+    std::vector<log_file_t> files;
 
     /** simple setter, does not close or open log files */
     void set_file_names(std::vector<std::string> names);
diff --git a/storage/innobase/log/log0log.cc b/storage/innobase/log/log0log.cc
index d0b325c05aa..8694e2f19bf 100644
--- a/storage/innobase/log/log0log.cc
+++ b/storage/innobase/log/log0log.cc
@@ -587,72 +587,111 @@ void log_t::create()
   }
 }
 
+log_file_t::log_file_t(log_file_t &&rhs)
+{
+  m_fd= std::move(rhs.m_fd);
+  rhs.m_fd= OS_FILE_CLOSED;
+  m_path= std::move(rhs.m_path);
+}
+log_file_t &log_file_t::operator=(log_file_t &&rhs)
+{
+  std::swap(m_fd, rhs.m_fd);
+  std::swap(m_path, rhs.m_path);
+  return *this;
+}
+
+log_file_t::~log_file_t()
+{
+  if (is_opened())
+    os_file_close(m_fd);
+}
+
+bool log_file_t::open()
+{
+  ut_a(!is_opened());
+
+  bool success;
+  m_fd= os_file_create(innodb_log_file_key, m_path.c_str(),
+                       OS_FILE_OPEN | OS_FILE_ON_ERROR_NO_EXIT, OS_FILE_NORMAL,
+                       OS_LOG_FILE, srv_read_only_mode, &success);
+  if (!success)
+    m_fd= OS_FILE_CLOSED;
+
+  return success;
+}
+
+bool log_file_t::close()
+{
+  ut_a(is_opened());
+  bool result= os_file_close(m_fd);
+  m_fd= OS_FILE_CLOSED;
+  return result;
+}
+
+dberr_t log_file_t::read(size_t offset, span<byte> buf)
+{
+  ut_ad(is_opened());
+  return os_file_read(IORequestRead, m_fd, buf.data(), offset, buf.size());
+}
+
+dberr_t log_file_t::write(size_t offset, span<const byte> buf)
+{
+  ut_ad(is_opened());
+  return os_file_write(IORequestWrite, m_path.c_str(), m_fd, buf.data(),
+                       offset, buf.size());
+}
+
+bool log_file_t::flush_data_only()
+{
+  ut_ad(is_opened());
+  return os_file_flush_data(m_fd);
+}
+
 void log_t::files::set_file_names(std::vector<std::string> names)
 {
-  file_names= std::move(names);
+  files.clear();
+
+  for (auto &&name : names)
+    files.emplace_back(std::move(name));
 }
 
 void log_t::files::open_files()
 {
-  ut_ad(files.empty());
-  files.reserve(file_names.size());
-  for (const auto &name : file_names)
+  for (auto &file : files)
   {
-    bool success;
-    files.push_back(os_file_create(innodb_log_file_key, name.c_str(),
-                                   OS_FILE_OPEN | OS_FILE_ON_ERROR_NO_EXIT,
-                                   OS_FILE_NORMAL, OS_LOG_FILE,
-                                   srv_read_only_mode, &success));
-    if (!success)
-    {
-      ib::fatal() << "os_file_create(" << name << ") failed";
-    }
+    if (!file.open())
+      ib::fatal() << "os_file_create(" << file.get_path() << ") failed";
   }
 }
 
 void log_t::files::read(size_t total_offset, span<byte> buf)
 {
-  ut_ad(files.size() == file_names.size());
-
-  const size_t file_idx= total_offset / static_cast<size_t>(file_size);
+  auto &file= files[total_offset / static_cast<size_t>(file_size)];
   const size_t offset= total_offset % static_cast<size_t>(file_size);
 
-  if (const dberr_t err= os_file_read(IORequestRead, files[file_idx],
-                                      buf.data(), offset, buf.size()))
-  {
-    ib::fatal() << "os_file_read(" << file_names[file_idx] << ") returned "
+  if (const dberr_t err= file.read(offset, buf))
+    ib::fatal() << "log_file_t::read(" << file.get_path() << ") returned "
                 << err;
-  }
 }
 
 void log_t::files::write(size_t total_offset, span<byte> buf)
 {
-  ut_ad(files.size() == file_names.size());
-
-  const size_t file_idx= total_offset / static_cast<size_t>(file_size);
+  auto &file= files[total_offset / static_cast<size_t>(file_size)];
   const size_t offset= total_offset % static_cast<size_t>(file_size);
 
-  if (const dberr_t err=
-          os_file_write(IORequestWrite, file_names[file_idx].c_str(),
-                        files[file_idx], buf.data(), offset, buf.size()))
-  {
-    ib::fatal() << "os_file_write(" << file_names[file_idx] << ") returned "
+  if (const dberr_t err= file.write(offset, buf))
+    ib::fatal() << "log_file_t::d_write(" << file.get_path() << ") returned "
                 << err;
-  }
 }
 
 void log_t::files::flush_data_only()
 {
-  ut_ad(files.size() == file_names.size());
-
   log_sys.pending_flushes.fetch_add(1, std::memory_order_acquire);
-  for (auto it= files.begin(), end= files.end(); it != end; ++it)
+  for (auto &file : files)
   {
-    if (!os_file_flush_data(*it))
-    {
-      const auto idx= std::distance(files.begin(), it);
-      ib::fatal() << "os_file_flush_data(" << file_names[idx] << ") failed";
-    }
+    if (!file.flush_data_only())
+      ib::fatal() << "log_file_t::flush_data_only(" << file.get_path()
+                  << ") failed";
   }
   log_sys.pending_flushes.fetch_sub(1, std::memory_order_release);
   log_sys.flushes.fetch_add(1, std::memory_order_release);
@@ -660,15 +699,11 @@ void log_t::files::flush_data_only()
 
 void log_t::files::close_files()
 {
-  for (auto it= files.begin(), end= files.end(); it != end; ++it)
+  for (auto &file : files)
   {
-    if (!os_file_close(*it))
-    {
-      const auto idx= std::distance(files.begin(), it);
-      ib::fatal() << "os_file_close(" << file_names[idx] << ") failed";
-    }
+    if (file.is_opened() && !file.close())
+      ib::fatal() << "log_file_t::close(" << file.get_path() << ") failed";
   }
-  files.clear();
 }
 
 /** Initialize the redo log.
diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
index d60e3d088ad..6ad528a6827 100644
--- a/storage/innobase/srv/srv0start.cc
+++ b/storage/innobase/srv/srv0start.cc
@@ -360,7 +360,6 @@ create_log_files(
 	}
 
 	logfile0 = file_names[0];
-	log_sys.log.set_file_names(std::move(file_names));
 
 	DBUG_EXECUTE_IF("innodb_log_abort_8", return(DB_ERROR););
 	DBUG_PRINT("ib_log", ("After innodb_log_abort_8"));
@@ -374,6 +373,7 @@ create_log_files(
 		return(DB_ERROR);
 	}
 
+	log_sys.log.set_file_names(std::move(file_names));
 	log_sys.log.open_files();
 	fil_open_system_tablespace_files();
 
@@ -447,7 +447,7 @@ static dberr_t create_log_files_rename(char *logfilename, size_t dirnamelen,
 
   /* Replace the first file with ib_logfile0. */
   logfile0= logfilename;
-  log_sys.log.file_names[0]= logfilename;
+  log_sys.log.files[0]= log_file_t(logfilename);
   log_mutex_exit();
 
   DBUG_EXECUTE_IF("innodb_log_abort_10", err= DB_ERROR;);