Bug #103620 No need to acquire shard mutex if setting to O_DIRECT_NO_FSYNC
Submitted: 7 May 2021 8:31 Modified: 8 May 2021 3:48
Reporter: zhai weixiang (OCA) Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:8.0.24 OS:Any
Assigned to: CPU Architecture:Any

[7 May 2021 8:31] zhai weixiang
Description:
In function Fil_shard::flush_file_space, it always acquire shard mutex and iterate the unflushed list, to collect space ids. But if setting flush method to O_DIRECT_NO_FSYNC. I think it's not necessary:
- with O_DIRECT_NO_FSYNC, the space is not added to flush list.
- One exception is that needs to flush is extending tablespace. But the space is already flushed by invoking space_flush() in  Fil_shard::space_extend

So I think we can avoid acquiring mutex if setting to O_DIRECT_NO_FSYNC

How to repeat:
read the code

Suggested fix:
slightly modify Fil_shard::flush_file_spaces:

diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
index 73372c7..b28e0e7 100644
--- a/storage/innobase/fil/fil0fil.cc
+++ b/storage/innobase/fil/fil0fil.cc
@@ -8538,6 +8538,22 @@ void Fil_shard::flush_file_spaces(uint8_t purpose) {
   Space_ids space_ids;
 
   ut_ad((purpose & FIL_TYPE_TABLESPACE) || (purpose & FIL_TYPE_LOG));
+  if (purpose == FIL_TYPE_TABLESPACE &&
+      srv_unix_file_flush_method == SRV_UNIX_O_DIRECT_NO_FSYNC) {
+#ifdef UNIV_DEBUG
+    mutex_acquire();
+    for (auto space = UT_LIST_GET_FIRST(m_unflushed_spaces); space != nullptr;
+        space = UT_LIST_GET_NEXT(unflushed_spaces, space)) {
+      if ((to_int(space->purpose) & purpose) && !space->stop_new_ops) {
+        space_ids.push_back(space->id);
+      }
+    }
+    mutex_release();
+    ut_ad(space_ids.empty());
+    space_ids.clear();
+#endif
+    return;
+  }
 
   mutex_acquire();
[7 May 2021 9:38] MySQL Verification Team
Hello Zhai,

Thank you for the report and feedback.

regards,
Umesh
[7 May 2021 16:21] Sunny Bains
The flush method is a dynamic variable therefore we cannot assume that this will always hold.
[8 May 2021 3:48] zhai weixiang
Hi, Sunny, I checked the code, it's readonly !

static MYSQL_SYSVAR_ENUM(flush_method, innodb_flush_method,
                         PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY,
                         "With which method to flush data", nullptr, nullptr, 0,
                         &innodb_flush_method_typelib);