Skip to content

Collection of fixes for macOS - #5654

Open
DaveGosselin-MariaDB wants to merge 12 commits into
11.4from
11.4-macos-testing-fixes
Open

DaveGosselin-MariaDB wants to merge 12 commits into
11.4from
11.4-macos-testing-fixes

Conversation

@DaveGosselin-MariaDB

Copy link
Copy Markdown
Member

Each commit in this collection fixes a test failure specific to macOS.

Three commits, "MDEV-33616: Charge and credit the same size for the recovery buffer", "MDEV-33616: Only one of two routines named in a statement is found", and "MDEV-33616: Routines of a mixed case database are not listed" change the server code.

Each commit has its own explanation in its commit message.

@gkodinov gkodinov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks mostly good. Some small improvements suggested below.

Please get the innodb team to review as well.

Comment thread mysql-test/include/have_sparse_files.inc Outdated
Comment thread mysql-test/suite/atomic/drop_db_long_names.test Outdated
Comment thread mysql-test/suite/innodb/t/innodb_buffer_pool_resize_temporary.test
Comment thread storage/innobase/include/log0recv.h Outdated
Comment on lines +269 to +271
/** the number of bytes allocated for tmp_buf, which is tmp_buf_size
rounded up to a multiple of the large page size */
size_t tmp_buf_alloc_size;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This would increase sizeof recv_sys. Is this really needed on all platforms (certainly not on Windows), or at all? What damage is done if the core dump exclusion is only requested on the requested size, and anything after it is allowed to be included in core dumps?

Does macOS even support any madvise(2) option that would allow the memory to be omitted from a core dump?

What if we just allocate recv_sys.tmp_buf as regular memory on macOS? It will be freed after crash recovery is completed or aborted.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change is not really needed on Windows, but the imbalance is not specific to macOS. Likely any platform where the rounding happens is exposed to this problem.

on macOS ut_dontdump and ut_dodump are empty functions because their bodies compile only under DBUG_OFF && HAVE_MADVISE && MADV_DODUMP; while macOS has madvise(2), it has no "advice" that omits memory from a core dump. MADV_DONTDUMP and MADV_DODUMP don't exist on mac.

Allocating tmp_buf as regular memory could work. The tradeoff is scope. It could also remove the only macOS dependency on large pages. That means that the fix likely becomes an #ifdef __APPLE__ in log0recv.cc. Doing it on every platform is smaller.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think that changing this to a regular memory allocation is the lesser evil. Please do that.

@vaintroub vaintroub Sep 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I fixed main.large_pages last week in 6ab0036. I think all claims "not really needed on Windows" are wrong, the "memory leak" was reported on Windows, by me. what is needed is large pages tests in CI.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks Wlad; correct me if I'm wrong but I don't see that I need to make a change in this PR for Windows specifically, especially as we're moving this to a "regular" memory allocation.

Comment thread mysql-test/suite/innodb/t/log_upgrade.test Outdated
@DaveGosselin-MariaDB
DaveGosselin-MariaDB force-pushed the 11.4-macos-testing-fixes branch 2 times, most recently from 12dedca to e74018c Compare September 17, 2026 17:28

@dr-m dr-m left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very good. The latest run on macOS got a timeout in perfschema.statement_program_non_nested (which I guess is a spurious failure not specific to macOS) and a failure of plugins.multiauth that looks deterministic.

OK to push after addressing my comments.

Comment thread mysql-test/suite/plugins/t/multiauth.test Outdated
Comment thread storage/innobase/include/ut0new.h Outdated

@dr-m dr-m left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

OK to push after changing the allocation of recv_sys.tmp_buf to use malloc() and free(). No need to ask for another review.

Comment thread mysql-test/suite/plugins/t/multiauth.test
Comment thread storage/innobase/log/log0recv.cc Outdated
Comment thread storage/innobase/log/log0recv.cc
Comment thread mysql-test/suite/innodb/t/innodb_buffer_pool_resize_temporary.test Outdated
DaveGosselin-MariaDB and others added 12 commits September 21, 2026 14:13
At lower_case_table_names=2 this returns nothing.

  CREATE DATABASE Db1;
  CREATE FUNCTION Db1.f1(a INT) RETURNS INT RETURN a;
  SELECT ROUTINE_NAME FROM information_schema.ROUTINES
   WHERE ROUTINE_SCHEMA='Db1';

mysql.proc records the function's database as db1, in lower case.
Creating a routine lower-cases its database name whenever
lower_case_table_names is anything but 0, at sql/sp_head.h:121.  The
datadir, SCHEMATA and DATABASE() all keep Db1.

CALL Db1.f1() still works, because calling a routine lower-cases the
database name too and then searches mysql.proc for db1.  The query
above never lower-cases it.  It searches for Db1, and mysql.proc.db
collates utf8mb3_bin, so the comparison runs byte for byte and no row
matches.

At setting 1 the server lower-cases the filter value as well, at
sql/sql_show.cc:4394, and lower-cases every name it stores, so the
query and the table always agree.  Setting 2 lower-cases the routine's
copy and nothing else.

The fix lower-cases the filter value before the search.

Sorting the same query brings the row back.

  SELECT ROUTINE_NAME FROM information_schema.ROUTINES
   WHERE ROUTINE_SCHEMA='Db1' ORDER BY ROUTINE_NAME;

The sort keeps the filter from reaching that search.  The server reads
all of mysql.proc instead, then applies the WHERE to ROUTINE_SCHEMA,
which compares case insensitively.  That shape answered correctly all
along.

The same search fills PARAMETERS and backs SHOW FUNCTION STATUS, SHOW
PROCEDURE STATUS, SHOW PACKAGE STATUS and SHOW PACKAGE BODY STATUS.
Every one returned nothing for Db1.  mariadb-dump lists routines with
SHOW FUNCTION STATUS WHERE Db=..., at client/mysqldump.cc:2859, which
is the main.mysqldump failure.

Setting 0 keeps Db1 and db1 as two databases holding two routines.  A
case sensitive volume confirms both stay distinct before and after this
change.  beb9a54 (MDEV-20609) added the search in 10.11.1.
main.lowercase_routines runs both query shapes.
With lower_case_table_names 0 the server can have databases Db1 and db1,
each with a function f1.  A single statement naming both databases, like
SELECT Db1.f1(), db1.f1(), reported that db1.f1 does not exist.

The set of routines a statement uses compared its entries without regard
to case.  Only one routine was loaded but the reference to the other
found nothing.  The set now compares its entries exactly, as the routine
cache and the lock manager already do.
macOS declares select() in sys/select.h, which the HAVE_SELECT probe did
not include.  clang rejects a call to an undeclared function, so the
probe failed and HAVE_SELECT was left undefined.

my_sleep() then took its last fallback, a busy loop on time() that
rounds the requested interval up to a whole second.  Every sub-second
sleep in the server became a one second spin on a CPU, which is what
made rpl.rpl_perfschema_applier_status_by_worker,
rpl.rpl_shutdown_sighup and rpl.rpl_semi_sync_shutdown_await_ack fail.
macOS puts the data directory on a case insensitive file system, so
lower_case_table_names is 2 and both tests recorded an answer that only
holds for 0.

period.i_s_notembedded looked up I_S.PERIODS and I_S.KEY_PERIOD_USAGE by
the schema name TEST.  That comparison follows the table name
comparison, so it finds the table under 1 and 2 and finds nothing under
0.  Those four queries move to the new test period.i_s_case_sensitive,
which requires lower_case_table_names=0.  The win rdiff of
period.i_s_notembedded covered the same difference and is no longer
needed.

atomic.drop_db_long_names generated table and view names in upper case
and compared the DROP statements that DDL recovery writes to the binary
log.  Under 2 the names come back from the directory in lower case.
Generating them in lower case to begin with gives the same names on
every setting.  Lower case also changes where the view name sorts
against its table name for the letters after v, which moves one view
between two of the recorded DROP VIEW statements.
Introduces a new MTR include, not_mac.inc, which when included at the
top of a test, prevents that test from running on macOS.

sys_vars.sysvars_readonly_debug is the first user.  It expects the
server to fault when a read only sysvar is written behind the sysvar
interface.  That protection needs the ro_after_init section, which a
linker script places and ld64 has no option to take, so
HAVE_RO_AFTER_INIT stays undefined on macOS.  Without it no variable is
moved into the read only root either, so neither of the two assignments
is refused.
Its default value depends on the operating system, ON where the log can
be memory mapped and OFF elsewhere, so the recorded row only holds on
some platforms.  The other variables whose default depends on the
operating system are already excluded the same way.
The test replaces the number of buffer pool blocks with a fixed value so
that the message is stable.  The pattern only accepted 5.., and macOS
builds without a futex use SUX_LOCK_GENERIC, which enlarges buf_block_t
enough to bring the count down into 4...
The injected deadlock reaches the client as ER_GET_ERRNO carrying errno
11, and the text comes from my_strerror().  11 is EAGAIN on Linux and
EDEADLK on macOS, so the message reads "Resource temporarily
unavailable" on one and "Resource deadlock avoided" on the other.
Replace the quoted text so the test does not depend on it.
The client reports why it could not load client_ed25519, and macOS names
every path that dlopen() tried.  Two expressions are added, one for the
chunk that holds the start of that message and one for the chunk that
holds the rest of it.

The line runs to 563 bytes, 52 of prefix and the 511 that the client
error buffer holds, while do_exec() reads the output with fgets() into a
512 byte buffer and runs the replacements on each chunk on its own.  A
long enough vardir therefore splits the line, because the path appears
four times in the dlopen text.  The second chunk is the tail of a path
and carries no colon, where the first chunk keeps the colons of the
mysqltest prefix.  That chunk also holds the only line terminator the
error line gets, so the expression captures the newline and the
replacement puts it back.  A replacement is inserted as written, so a \n
spelled there would reach the output as a backslash and an n.

Both expressions stop at a newline.  reg_replace compiles with
REG_DOTALL, so an unrestricted .* runs past the line terminator whenever
the whole message reaches the replacement in one chunk, and the error
line then joins the line after it.
recv_sys.tmp_buf comes from malloc() rather than from the large page
allocator.

main.large_pages fails on macOS with "Warning: Memory not freed: 16375"
at shutdown.  recv_sys_t::find_checkpoint() asks for 1048585 bytes,
my_large_malloc() rounds that up to 1064960 and charges the rounded
figure to the server memory accounting, and recv_sys_t::tmp_free()
credits back the 1048585 that was requested.  ut_malloc_dontdump() takes
the size by value, so it has nowhere to report what my_large_malloc()
wrote back.

The rounding happens whenever my_next_large_page_size() finds a reported
large page size at or below the request.  macOS has no huge page
interface for my_get_large_page_sizes() to consult, so its fallback
branch reports the ordinary page size, 16384 on Apple silicon, and the
request is always rounded.  Linux reads the sizes from
/sys/kernel/mm/hugepages, where the smallest entry is usually 2 MiB, and
a 1 MiB request then gets no large page and no rounding.

The buffer has no alignment requirement.  recv_sys_t::parse() copies a
mini-transaction into it when the record is encrypted in the
FORMAT_ENC_11 log, where it is then decrypted in place, or when the
record wraps around the end of the log file, and reads it back as a byte
sequence.  tmp_free() calls std::free() because the member function
recv_sys_t::free() hides the one from <cstdlib>.

log_sys.buf and log_sys.flush_buf keep the large page allocator.  They
round the same way, so a server started with --large-pages
--innodb-log-buffer-size=2101248 still reports 24576 on macOS.  The core
dump exclusion that recv_sys.tmp_buf gives up applies only where
MADV_DONTDUMP exists, so nothing changes on macOS, while a release build
on Linux would now include the buffer in a dump.  tmp_free() overwrites
the redo log records that innodb_encrypt_log decrypted before releasing
the memory, through a volatile function pointer because GCC removes a
plain memset() that is followed by free().

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
innodb.log_upgrade and innodb.log_upgrade_101_flags build 8GB redo log
files by seeking past the end of an empty file and writing a single
byte.  That needs a filesystem which leaves the skipped range
unallocated.  HFS on macOS allocates every block of it instead, so the
write fails with ENOSPC and the test reports a perl failure.

include/have_sparse_files.inc probes a directory the caller names,
writing one byte 64MB into an empty file there and comparing the
allocated block count against that offset.
The wait timer can have a granularity coarser than the time an
uncontended read lock is held, so the recorded duration of one lock can
be zero, which reads back as NULL.  This can cause the test to fail with
a false negative.

Take the lock twenty more times at each measurement point, with the
extra statements silent so the recorded result does not change.  The
mutex part of the test already works this way, since one SELECT
produces ten THR_LOCK::mutex events.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants