Skip to content

Commit

Permalink
Fix bug 1669414 (Failed to set O_DIRECT on xb_doublewrite when runnin…
Browse files Browse the repository at this point in the history
…g MTR test cases)

Fix handling of failure to set O_DIRECT on the parallel doublewrite
buffer by the following:
- downgrade diagnostics severity from warning to note by introducing
  an extra flag to os_file_setnocache and creating new logger helper
  class warn_or_info;
- introduce new flag parallel_dblwr_t::needs_flush. Initialize it in
  buf_parallel_dblwr_file_create according to innodb_flush_method
  value and whether setting O_DIRECT succeeded. Use it instead of
  innodb_flush_method value in buf_dblwr_flush_buffered_writes.
- fix an incorrect comment at os_file_create_func.
  • Loading branch information
laurynas-biveinis committed Jul 18, 2017
1 parent 2a1a373 commit 7f41824
Show file tree
Hide file tree
Showing 7 changed files with 67 additions and 28 deletions.
36 changes: 19 additions & 17 deletions storage/innobase/buf/buf0dblwr.cc
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,7 @@ buf_dblwr_init_or_load_pages(

os_file_set_nocache(parallel_dblwr_buf.file,
parallel_dblwr_buf.path,
"open");
"open", false);

os_offset_t size = os_file_get_size(parallel_dblwr_buf.file);

Expand Down Expand Up @@ -1262,21 +1262,8 @@ buf_dblwr_flush_buffered_writes(
srv_stats.dblwr_pages_written.add(dblwr_shard->first_free);
srv_stats.dblwr_writes.inc();

/* Now flush the doublewrite buffer data to disk, unless
innodb_flush_method is one of O_SYNC, O_DIRECT_NO_FSYNC, or
ALL_O_DIRECT. */
switch (srv_unix_file_flush_method) {
case SRV_UNIX_NOSYNC:
case SRV_UNIX_O_DSYNC:
case SRV_UNIX_O_DIRECT_NO_FSYNC:
case SRV_UNIX_ALL_O_DIRECT:
break;
case SRV_UNIX_FSYNC:
case SRV_UNIX_LITTLESYNC:
case SRV_UNIX_O_DIRECT:
if (parallel_dblwr_buf.needs_flush)
os_file_flush(parallel_dblwr_buf.file);
break;
}

/* We know that the writes have been flushed to disk now
and in recovery we will find them in the doublewrite buffer
Expand Down Expand Up @@ -1530,8 +1517,23 @@ buf_parallel_dblwr_file_create(void)
return(DB_ERROR);
}

os_file_set_nocache(parallel_dblwr_buf.file, parallel_dblwr_buf.path,
"create");
const bool o_direct_set
= os_file_set_nocache(parallel_dblwr_buf.file,
parallel_dblwr_buf.path,
"create", false);
switch (srv_unix_file_flush_method) {
case SRV_UNIX_NOSYNC:
case SRV_UNIX_O_DSYNC:
case SRV_UNIX_O_DIRECT_NO_FSYNC:
case SRV_UNIX_ALL_O_DIRECT:
parallel_dblwr_buf.needs_flush = !o_direct_set;
break;
case SRV_UNIX_FSYNC:
case SRV_UNIX_LITTLESYNC:
case SRV_UNIX_O_DIRECT:
parallel_dblwr_buf.needs_flush = true;
break;
}

success = os_file_set_size(parallel_dblwr_buf.path,
parallel_dblwr_buf.file, size, false);
Expand Down
3 changes: 3 additions & 0 deletions storage/innobase/include/buf0dblwr.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ class parallel_dblwr_t {
public:
/** Parallel doublewrite buffer file handle */
pfs_os_file_t file;
/** Whether the doublewrite buffer file needs flushing after each
write */
bool needs_flush;
/** Path to the parallel doublewrite buffer */
char* path;
/** Individual parallel doublewrite partitions */
Expand Down
10 changes: 8 additions & 2 deletions storage/innobase/include/os0file.h
Original file line number Diff line number Diff line change
Expand Up @@ -1059,25 +1059,31 @@ os_file_create_simple_no_error_handling_func(
@param[in] file_name file name, used in the diagnostic message
@param[in] name "open" or "create"; used in the diagnostic
message
@param[in] failure_warning if true (the default), the failure to disable
caching is diagnosed at warning severity, and at note severity otherwise
@return true if operation is success and false */
bool
os_file_set_nocache(
int fd,
const char* file_name,
const char* operation_name);
const char* operation_name,
bool failure_warning = true);

/** Tries to disable OS caching on an opened file file.
@param[in] file file to alter
@param[in] file_name file name, used in the diagnostic message
@param[in] name "open" or "create"; used in the diagnostic
message
@param[in] failure_warning if true (the default), the failure to disable
caching is diagnosed at warning severity, and at note severity otherwise
@return true if operation is success and false */
UNIV_INLINE
bool
os_file_set_nocache(
pfs_os_file_t file,
const char* file_name,
const char* operation_name);
const char* operation_name,
bool failure_warning = true);

/** NOTE! Use the corresponding macro os_file_create(), not directly
this function!
Expand Down
8 changes: 6 additions & 2 deletions storage/innobase/include/os0file.ic
Original file line number Diff line number Diff line change
Expand Up @@ -673,13 +673,17 @@ pfs_os_file_set_eof_at_func(
@param[in] file_name file name, used in the diagnostic message
@param[in] name "open" or "create"; used in the diagnostic
message
@param[in] failure_warning if true (the default), the failure to disable
caching is diagnosed at warning severity, and at note severity otherwise
@return true if operation is success and false */
UNIV_INLINE
bool
os_file_set_nocache(
pfs_os_file_t file,
const char* file_name,
const char* operation_name)
const char* operation_name,
bool failure_warning)
{
return os_file_set_nocache(file.m_file, file_name, operation_name);
return os_file_set_nocache(file.m_file, file_name, operation_name,
failure_warning);
}
13 changes: 13 additions & 0 deletions storage/innobase/include/ut0ut.h
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,19 @@ class fatal_or_error : public logger {
const bool m_fatal;
};

/** Emit a warning message if the given predicate is true, otherwise emit an
informational message. */
class warn_or_info : public logger {
public:
warn_or_info(bool pred)
: m_warn(pred)
{}

~warn_or_info();
private:
const bool m_warn;
};

} // namespace ib

#ifndef UNIV_NONINL
Expand Down
16 changes: 9 additions & 7 deletions storage/innobase/os/os0file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3706,9 +3706,8 @@ os_file_create_func(
} else if (!srv_read_only_mode
&& *success
&& srv_unix_file_flush_method == SRV_UNIX_ALL_O_DIRECT) {
/* Do fsync() on log and parallel doublewrite files
when setting O_DIRECT fails.
See log_io_complete() and buf_dblwr_flush_buffered_writes() */
/* Do fsync() on log and files when setting O_DIRECT fails.
See log_io_complete() */
if (!os_file_set_nocache(file.m_file, name, mode_str)) {
srv_unix_file_flush_method = SRV_UNIX_O_DIRECT;
}
Expand Down Expand Up @@ -6010,12 +6009,15 @@ os_file_handle_error_no_exit(
@param[in] file_name file name, used in the diagnostic message
@param[in] name "open" or "create"; used in the diagnostic
message
@param[in] failure_warning if true (the default), the failure to disable
caching is diagnosed at warning severity, and at note severity otherwise
@return true if operation is success and false */
bool
os_file_set_nocache(
int fd MY_ATTRIBUTE((unused)),
const char* file_name MY_ATTRIBUTE((unused)),
const char* operation_name MY_ATTRIBUTE((unused)))
const char* operation_name MY_ATTRIBUTE((unused)),
bool failure_warning MY_ATTRIBUTE((unused)))
{
/* some versions of Solaris may not have DIRECTIO_ON */
#if defined(UNIV_SOLARIS) && defined(DIRECTIO_ON)
Expand All @@ -6037,8 +6039,8 @@ os_file_set_nocache(
if (!warning_message_printed) {
warning_message_printed = true;
# ifdef UNIV_LINUX
ib::warn()
<< "Failed to set O_DIRECT on file"
ib::warn_or_info(failure_warning)
<< "Failed to set O_DIRECT on file "
<< file_name << ";" << operation_name
<< ": " << strerror(errno_save) << ", "
<< "continuing anyway. O_DIRECT is "
Expand All @@ -6053,7 +6055,7 @@ os_file_set_nocache(
# ifndef UNIV_LINUX
short_warning:
# endif
ib::warn()
ib::warn_or_info(failure_warning)
<< "Failed to set O_DIRECT on file "
<< file_name << "; " << operation_name
<< " : " << strerror(errno_save)
Expand Down
9 changes: 9 additions & 0 deletions storage/innobase/ut/ut0ut.cc
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,15 @@ fatal_or_error::~fatal_or_error()
ut_a(!m_fatal);
}

warn_or_info::~warn_or_info()
{
if (m_warn) {
sql_print_warning("InnoDB: %s", m_oss.str().c_str());
} else {
sql_print_information("InnoDB: %s", m_oss.str().c_str());
}
}

} // namespace ib

#endif /* !UNIV_INNOCHECKSUM */

0 comments on commit 7f41824

Please sign in to comment.