| 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: | |
| Category: | MySQL Server: InnoDB storage engine | Severity: | S3 (Non-critical) |
| Version: | 8.0.24 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[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);
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();