Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MDEV-35785 innodb_log_file_mmap is not defined on 32-bit systems #3732

Draft
wants to merge 3 commits into
base: 10.11
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions extra/mariabackup/xtrabackup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1331,9 +1331,7 @@ enum options_xtrabackup
OPT_INNODB_BUFFER_POOL_FILENAME,
OPT_INNODB_LOCK_WAIT_TIMEOUT,
OPT_INNODB_LOG_BUFFER_SIZE,
#ifdef HAVE_INNODB_MMAP
OPT_INNODB_LOG_FILE_MMAP,
#endif
#if defined __linux__ || defined _WIN32
OPT_INNODB_LOG_FILE_BUFFERING,
#endif
Expand Down Expand Up @@ -1895,13 +1893,11 @@ struct my_option xb_server_options[] =
(G_PTR*) &log_sys.buf_size, (G_PTR*) &log_sys.buf_size, 0,
GET_UINT, REQUIRED_ARG, 2U << 20,
2U << 20, log_sys.buf_size_max, 0, 4096, 0},
#ifdef HAVE_INNODB_MMAP
{"innodb_log_file_mmap", OPT_INNODB_LOG_FILE_SIZE,
"Whether ib_logfile0 should be memory-mapped",
(G_PTR*) &log_sys.log_mmap,
(G_PTR*) &log_sys.log_mmap, 0, GET_BOOL, NO_ARG,
log_sys.log_mmap_default, 0, 0, 0, 0, 0},
#endif
#if defined __linux__ || defined _WIN32
{"innodb_log_file_buffering", OPT_INNODB_LOG_FILE_BUFFERING,
"Whether the file system cache for ib_logfile0 is enabled during --backup",
Expand Down Expand Up @@ -3370,7 +3366,6 @@ static my_bool xtrabackup_copy_datafile(ds_ctxt *ds_data,
return(FALSE);
}

#ifdef HAVE_INNODB_MMAP
static int
xtrabackup_copy_mmap_snippet(ds_file_t *ds, const byte *start, const byte *end)
{
Expand Down Expand Up @@ -3470,7 +3465,6 @@ static bool xtrabackup_copy_mmap_logfile()
msg(">> log scanned up to (" LSN_PF ")", recv_sys.lsn);
return false;
}
#endif

/** Copy redo log until the current end of the log is reached
@return whether the operation failed */
Expand All @@ -3482,10 +3476,9 @@ static bool xtrabackup_copy_logfile()
ut_a(dst_log_file);
ut_ad(recv_sys.is_initialised());

#ifdef HAVE_INNODB_MMAP
if (log_sys.is_mmap())
return xtrabackup_copy_mmap_logfile();
#endif

const size_t sequence_offset{log_sys.is_encrypted() ? 8U + 5U : 5U};
const size_t block_size_1{log_sys.write_size - 1};

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
[pread]
--innodb-log-file-mmap=OFF
[mmap]
--innodb-log-file-mmap=ON
13 changes: 12 additions & 1 deletion mysql-test/suite/sys_vars/r/sysvars_innodb.result
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ variable_name not in (
'innodb_numa_interleave', # only available WITH_NUMA
'innodb_evict_tables_on_commit_debug', # one may want to override this
'innodb_use_native_aio', # default value depends on OS
'innodb_log_file_mmap', # only available on 64-bit
'innodb_log_file_buffering', # only available on Linux and Windows
'innodb_buffer_pool_load_pages_abort') # debug build only, and is only for testing
order by variable_name;
Expand Down Expand Up @@ -1028,6 +1027,18 @@ NUMERIC_BLOCK_SIZE NULL
ENUM_VALUE_LIST OFF,ON
READ_ONLY NO
COMMAND_LINE_ARGUMENT OPTIONAL
VARIABLE_NAME INNODB_LOG_FILE_MMAP
SESSION_VALUE NULL
DEFAULT_VALUE ON
VARIABLE_SCOPE GLOBAL
VARIABLE_TYPE BOOLEAN
VARIABLE_COMMENT Whether ib_logfile0 should initially be memory-mapped
NUMERIC_MIN_VALUE NULL
NUMERIC_MAX_VALUE NULL
NUMERIC_BLOCK_SIZE NULL
ENUM_VALUE_LIST OFF,ON
READ_ONLY YES
COMMAND_LINE_ARGUMENT OPTIONAL
VARIABLE_NAME INNODB_LOG_FILE_SIZE
SESSION_VALUE NULL
DEFAULT_VALUE 100663296
Expand Down
2 changes: 1 addition & 1 deletion mysql-test/suite/sys_vars/t/sysvars_innodb.test
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

--vertical_results
--replace_regex /^\/\S+/PATH/ /\.\//PATH/
--replace_result 'resides in persistent memory or ' ''
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I see this unifies the variation in the help text:

   --innodb-log-file-mmap 
-                      Whether ib_logfile0 resides in persistent memory or
-                      should initially be memory-mapped
+                      Whether ib_logfile0 should initially be memory-mapped
                       (Defaults to on; use --skip-innodb-log-file-mmap to disable.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s right.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically places that have documentation to users show one version per software version. For example https://manpages.debian.org/unstable/mariadb-server-core/mariadbd.8.en.html has 3 versions for the 3 versions of MariaDB in Debian currently. Now it seems that the text will be different depending on architecture, so documentation should maybe have its own version per architecture.. obviously that would not make sense in any online collection of documentation, but for binaries outputting different help texts on different platforms it is less clear that may be the case

select VARIABLE_NAME, SESSION_VALUE, DEFAULT_VALUE, VARIABLE_SCOPE, VARIABLE_TYPE, VARIABLE_COMMENT, NUMERIC_MIN_VALUE, NUMERIC_MAX_VALUE, NUMERIC_BLOCK_SIZE, ENUM_VALUE_LIST, READ_ONLY, COMMAND_LINE_ARGUMENT from information_schema.system_variables
where variable_name like 'innodb%' and
variable_name not in (
'innodb_numa_interleave', # only available WITH_NUMA
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..and this seems to be a collection of all variables that appear/disappear based on platform capabilities, even though the source code and dependency versions would be the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically places that have documentation to users show one version per software version. For example https://manpages.debian.org/unstable/mariadb-server-core/mariadbd.8.en.html has 3 versions for the 3 versions of MariaDB in Debian currently. Now it seems that the text will be different depending on architecture, so documentation should maybe have its own version per architecture.. obviously that would not make sense in any online collection of documentation, but for binaries outputting different help texts on different platforms it is less clear that may be the case

'innodb_evict_tables_on_commit_debug', # one may want to override this
'innodb_use_native_aio', # default value depends on OS
'innodb_log_file_mmap', # only available on 64-bit
'innodb_log_file_buffering', # only available on Linux and Windows
'innodb_buffer_pool_load_pages_abort') # debug build only, and is only for testing
order by variable_name;
2 changes: 1 addition & 1 deletion storage/innobase/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ IF(UNIX)
LINK_LIBRARIES(numa)
ENDIF()
IF(CMAKE_SIZEOF_VOID_P EQUAL 8)
IF(CMAKE_SYSTEM_PROCESSOR MATCHES "(aarch|AARCH|p(ower)?pc|x86_|amd)64")
IF(CMAKE_SYSTEM_PROCESSOR MATCHES "(aarch|AARCH|p(ower)?pc|x86_|amd|loongarch|riscv)64")
OPTION(WITH_INNODB_PMEM "Support memory-mapped InnoDB redo log" ON)
ELSE() # Disable by default on ISA that are not covered by our CI
OPTION(WITH_INNODB_PMEM "Support memory-mapped InnoDB redo log" OFF)
Expand Down
4 changes: 0 additions & 4 deletions storage/innobase/handler/ha_innodb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19478,7 +19478,6 @@ static MYSQL_SYSVAR_UINT(log_buffer_size, log_sys.buf_size,
"Redo log buffer size in bytes.",
NULL, NULL, 16U << 20, 2U << 20, log_sys.buf_size_max, 4096);

#ifdef HAVE_INNODB_MMAP
static constexpr const char *innodb_log_file_mmap_description=
"Whether ib_logfile0"
# ifdef HAVE_PMEM
Expand All @@ -19489,7 +19488,6 @@ static MYSQL_SYSVAR_BOOL(log_file_mmap, log_sys.log_mmap,
PLUGIN_VAR_OPCMDARG | PLUGIN_VAR_READONLY,
innodb_log_file_mmap_description,
nullptr, nullptr, log_sys.log_mmap_default);
#endif

#if defined __linux__ || defined _WIN32
static MYSQL_SYSVAR_BOOL(log_file_buffering, log_sys.log_buffered,
Expand Down Expand Up @@ -19985,9 +19983,7 @@ static struct st_mysql_sys_var* innobase_system_variables[]= {
MYSQL_SYSVAR(deadlock_report),
MYSQL_SYSVAR(page_size),
MYSQL_SYSVAR(log_buffer_size),
#ifdef HAVE_INNODB_MMAP
MYSQL_SYSVAR(log_file_mmap),
#endif
#if defined __linux__ || defined _WIN32
MYSQL_SYSVAR(log_file_buffering),
#endif
Expand Down
11 changes: 0 additions & 11 deletions storage/innobase/include/log0log.h
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,6 @@ struct log_t
uint write_size;
/** format of the redo log: e.g., FORMAT_10_8 */
uint32_t format;
#ifdef HAVE_INNODB_MMAP
/** whether the memory-mapped interface is enabled for the log */
my_bool log_mmap;
/** the default value of log_mmap */
Expand All @@ -294,7 +293,6 @@ struct log_t
# else /* an unnecessary read-ahead of a large ib_logfile0 is a risk */
# endif
false;
#endif
#if defined __linux__ || defined _WIN32
/** whether file system caching is enabled for the log */
my_bool log_buffered;
Expand Down Expand Up @@ -347,11 +345,7 @@ struct log_t
void set_buf_free(size_t f) noexcept
{ ut_ad(f < buf_free_LOCK); buf_free.store(f, std::memory_order_relaxed); }

#ifdef HAVE_INNODB_MMAP
bool is_mmap() const noexcept { return !flush_buf; }
#else
static constexpr bool is_mmap() { return false; }
#endif

/** @return whether a handle to the log is open;
is_mmap() && !is_opened() holds for PMEM */
Expand Down Expand Up @@ -412,14 +406,9 @@ struct log_t
@return whether the memory allocation succeeded */
bool attach(log_file_t file, os_offset_t size);

#ifdef HAVE_INNODB_MMAP
/** Disable memory-mapped access (update log_mmap) */
void clear_mmap();
void close_file(bool really_close= true);
#else
static void clear_mmap() {}
void close_file();
#endif
#if defined __linux__ || defined _WIN32
/** Try to enable or disable file system caching (update log_buffered) */
void set_buffered(bool buffered);
Expand Down
7 changes: 1 addition & 6 deletions storage/innobase/include/log0recv.h
Original file line number Diff line number Diff line change
Expand Up @@ -430,12 +430,7 @@ struct recv_sys_t
@tparam storing whether to store the records
@param if_exists storing=YES: whether to check if the tablespace exists */
template<store storing>
static parse_mtr_result parse_mmap(bool if_exists) noexcept
#ifdef HAVE_INNODB_MMAP
;
#else
{ return parse_mtr<storing>(if_exists); }
#endif
static parse_mtr_result parse_mmap(bool if_exists) noexcept;

/** Erase log records for a page. */
void erase(map::iterator p);
Expand Down
3 changes: 0 additions & 3 deletions storage/innobase/include/univ.i
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,6 @@ using the call command. */
#define UNIV_INLINE static inline

#define UNIV_WORD_SIZE SIZEOF_SIZE_T
#if SIZEOF_SIZE_T == 8
# define HAVE_INNODB_MMAP
#endif
dr-m marked this conversation as resolved.
Show resolved Hide resolved

/** The following alignment is used in memory allocations in memory heap
management to ensure correct alignment for doubles etc. */
Expand Down
44 changes: 16 additions & 28 deletions storage/innobase/log/log0log.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ void log_file_t::write(os_offset_t offset, span<const byte> buf) noexcept
abort();
}

#ifdef HAVE_INNODB_MMAP
# ifdef HAVE_PMEM
# include "cache.h"
# endif
Expand All @@ -203,6 +202,10 @@ static void *log_mmap(os_file_t file,
# endif
os_offset_t size)
{
#if SIZEOF_SIZE_T < 8
if (size != os_offset_t(size_t(size)))
return MAP_FAILED;
#endif
if (my_system_page_size > 4096)
return MAP_FAILED;
# ifndef HAVE_PMEM
Expand Down Expand Up @@ -300,20 +303,17 @@ static void *log_mmap(os_file_t file,
# endif
return ptr;
}
#endif

#if defined __linux__ || defined _WIN32
/** Display a message about opening the log */
ATTRIBUTE_COLD static void log_file_message()
{
sql_print_information("InnoDB: %s (block size=%u bytes)",
# ifdef HAVE_INNODB_MMAP
log_sys.log_mmap
? (log_sys.log_buffered
? "Memory-mapped log"
: "Memory-mapped unbuffered log")
:
# endif
log_sys.log_buffered
? "Buffered log writes"
: "File system buffers for log disabled",
Expand All @@ -332,7 +332,6 @@ bool log_t::attach(log_file_t file, os_offset_t size)
ut_ad(!buf);
ut_ad(!flush_buf);
ut_ad(!writer);
#ifdef HAVE_INNODB_MMAP
if (size)
{
# ifdef HAVE_PMEM
Expand Down Expand Up @@ -363,7 +362,6 @@ bool log_t::attach(log_file_t file, os_offset_t size)
}
}
log_mmap= false;
#endif
buf= static_cast<byte*>(ut_malloc_dontdump(buf_size, PSI_INSTRUMENT_ME));
if (!buf)
{
Expand Down Expand Up @@ -400,9 +398,7 @@ bool log_t::attach(log_file_t file, os_offset_t size)
writer_update();
memset_aligned<512>(checkpoint_buf, 0, write_size);

#ifdef HAVE_INNODB_MMAP
func_exit:
#endif
log_file_message();
return true;
}
Expand Down Expand Up @@ -477,25 +473,19 @@ ATTRIBUTE_COLD static void log_close_failed(dberr_t err)
ib::fatal() << "closing ib_logfile0 failed: " << err;
}

#ifdef HAVE_INNODB_MMAP
void log_t::close_file(bool really_close)
#else
void log_t::close_file()
#endif
{
#ifdef HAVE_INNODB_MMAP
if (is_mmap())
{
ut_ad(!checkpoint_buf);
ut_ad(!flush_buf);
if (buf)
{
my_munmap(buf, file_size);
my_munmap(buf, size_t(file_size));
buf= nullptr;
}
}
else
#endif
{
ut_ad(!buf == !flush_buf);
ut_ad(!buf == !checkpoint_buf);
Expand All @@ -512,9 +502,7 @@ void log_t::close_file()

writer= nullptr;

#ifdef HAVE_INNODB_MMAP
if (really_close)
#endif
if (is_opened())
if (const dberr_t err= log.close())
log_close_failed(err);
Expand Down Expand Up @@ -621,21 +609,18 @@ log_t::resize_start_status log_t::resize_start(os_offset_t size) noexcept
void *ptr= nullptr, *ptr2= nullptr;
success= os_file_set_size(path.c_str(), resize_log.m_file, size);
if (!success);
#ifdef HAVE_INNODB_MMAP
#ifdef HAVE_PMEM
else if (is_mmap())
{
ptr= ::log_mmap(resize_log.m_file,
#ifdef HAVE_PMEM
is_pmem,
#endif
size);
ptr= ::log_mmap(resize_log.m_file, is_pmem, size);

if (ptr == MAP_FAILED)
goto alloc_fail;
}
#endif
else
{
ut_ad(!is_mmap());
ptr= ut_malloc_dontdump(buf_size, PSI_INSTRUMENT_ME);
if (ptr)
{
Expand Down Expand Up @@ -705,21 +690,26 @@ void log_t::resize_abort() noexcept

if (resize_in_progress() > 1)
{
if (!is_mmap())
#ifdef HAVE_PMEM
const bool is_mmap{this->is_mmap()};
#else
constexpr bool is_mmap{false};
#endif
if (!is_mmap)
{
ut_free_dodump(resize_buf, buf_size);
ut_free_dodump(resize_flush_buf, buf_size);
resize_flush_buf= nullptr;
}
#ifdef HAVE_INNODB_MMAP
else
{
ut_ad(!resize_log.is_opened());
ut_ad(!resize_flush_buf);
#ifdef HAVE_PMEM
if (resize_buf)
my_munmap(resize_buf, resize_target);
#endif /* HAVE_PMEM */
}
#endif
if (resize_log.is_opened())
resize_log.close();
resize_buf= nullptr;
Expand Down Expand Up @@ -1224,7 +1214,6 @@ ATTRIBUTE_COLD void log_write_and_flush_prepare()
group_commit_lock::ACQUIRED);
}

#ifdef HAVE_INNODB_MMAP
void log_t::clear_mmap()
{
if (!is_mmap() ||
Expand Down Expand Up @@ -1258,7 +1247,6 @@ void log_t::clear_mmap()
}
log_resize_release();
}
#endif

/** Durably write the log up to log_sys.get_lsn(). */
ATTRIBUTE_COLD void log_write_and_flush()
Expand Down
Loading