MDEV-35785 innodb_log_file_mmap is not defined on 32-bit systems #3732
Conversation
|
|
|
4268d64 needs to be tested on the Debian CI by @ottok, because we have neither RISC-V nor LoongArch on https://buildbot.mariadb.org. |
HAVE_INNODB_MMAP: Remove, and unconditionally enable this code. log_mmap(): On 32-bit systems, ensure that the size fits in 32 bits. log_t::resize_start(), log_t::resize_abort(): Only handle memory-mapping if HAVE_PMEM is defined. The generic memory-mapped interface is only for reading the log in recovery. Writable memory mappings are only for persistent memory, that is, Linux file systems with mount -o dax. Reviewed by: Debarun Banerjee
Let us enable pmem_persist() on RISC-V and LoongArch, because those are available in the Debian CI. In commit 3f9f5ca these were initially disabled by default.
I am in the process of testing with riscv64, but builds are very slow.. |
|
I converted this to a draft so it does not get accidentally merged before it has been finalized and rebased so the fixup commit is squashed into the first one. |
|
To summarize original situation in MariaDB 11.4.4:
|
I pointed this out a week ago, including some reasoning why we can’t support persistent memory on IBM System Z (s390x), or on any 32-bit architecture. |
I intend this to be merged as 2 commits, addressing separate issues in the same area. |
| @@ -5,13 +5,13 @@ | |||
|
|
|||
| --vertical_results | |||
| --replace_regex /^\/\S+/PATH/ /\.\//PATH/ | |||
| --replace_result 'resides in persistent memory or ' '' | |||
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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
| @@ -5,13 +5,13 @@ | |||
|
|
|||
| --vertical_results | |||
| --replace_regex /^\/\S+/PATH/ /\.\//PATH/ | |||
| --replace_result 'resides in persistent memory or ' '' | |||
| 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 | |||
There was a problem hiding this comment.
..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.
There was a problem hiding this comment.
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
|
My riscv64 test build is still running.. so slow, and also host/queue seems to be busy |
Thanks for clarification. I misunderstood that "fixup" keyword as it (or "!fixup") is typically used as command words for The third commit is a merge commit, surely you don't want that? Maybe it came because Daniel updated with "Merge" instead of "Rebase" in the UI? (Wondering why GitHub even offers merge updates at all, or as primary option)
|
|
I prefer to push this manually (first rebasing and force-pushing the source branch, then pushing it to the target branch as soon as the tests on the mandatory builders have completed), because it is not clear to me how and when the GitHub auto-merge is being triggered.
Would that be acceptable? |
|
Unfortunately the riscv64 build is now failing on unrelated error: Trying to debug it.. |
Description
HAVE_INNODB_MMAP: Remove, and unconditionally enable this code.log_mmap(): On 32-bit systems, ensure that the size fits in 32 bits.log_t::resize_start(),log_t::resize_abort(): Only handle memory-mapping ifHAVE_PMEMis defined. The generic memory-mapped interface is only for reading the log in recovery. Writable memory mappings are only for persistent memory, that is, Linux file systems withmount -o dax.Release Notes
The configuration parameter
innodb_log_file_mmapwill be available in all target platforms.How can this PR be tested?
This is covered by the regression test suite. The test
sys_vars.sysvars_innodbwill ensure that the parameter is present.I tested this by running the regression test suite on IA-32 and AMD64 in my local development environment.
Basing the PR against the correct MariaDB version
mainbranch.PR quality check