- developers - lists.mariadb.org

On the question on producing bintars going forward
by Kristian Nielsen 22 Jan '25

22 Jan '25

Hi Vicentiu, I read through your proposal for how to handle MariaDB bintars. I know a lot of the old history on this (like from working in the MySQL build team back in 2005), I also know first-hand of the tough spot between what people think is "easily possible" and the practicalities of packaging for real linux systems. So I hope my input can be useful in some way: 1. To me, providing bintars is not important. Nor is providing any binaries for that matter. MariaDB is open source, the most important thing is it is easy to compile. Others can provide binaries as needed, and in fact distros are already doing so very well. So I personally am fine with not releasing bintars going forward. I know binaries are considered important by others. 2. I agree a clear definition is lacking of exactly what binaries should be/provide. My opinion for bintars is that they are a way to use MariaDB easily similar to compiling the source, but without the need for having installed a compiler toolchain. So bintars should be the result of a mostly default compile of the source, on a stable/oldstable linux distro like Debian or some redhat with minimal optional dependencies installed. Most work should go into making default compile do the right thing, as opposed to endless tweaking in build scripts or Buildbot for how bintars are compiled. 3. In your proposal, I think there is too much focus (out of good intentions, I'm sure) about solving all possible problems. No install method is perfect for all use cases. Bintars provide very simple install or even easy-of-use without installing, but they have other limitations. Let us not try to make bintars "perfect". Rather make them simple to produce, and simple to use in the limited scenarious where they can work well. 4. Let us firmly give up on building static binaries. Static binaries do not work well, and have not done so in many years. Here is a reference for a problem with linking libc statically: https://sourceware.org/glibc/wiki/FAQ#Even_statically_linked_programs_need_… And mixing static and dynamic linking creates some very serious problems, see MDEV-32251 and MDEV-25633. We should just build a default `cmake` binary with dynamic linking against the required library dependencies, do the build on a minimal linux where most optional dependencies are not installed. This will then work well on user's systems that have compatible libraries. On systems that have not, there are other options for running MariaDB. 5. I think we should _definitely_ not maintain an infrastructure for building our own static versions of various system libraries / 3rd-party libraries, see also (4). 6. I agree with concerns about too much effort needed to maintain bintars for many years. Let us be pragmatic, make things simple, and only do what can be done with small amount of effort. Build the bintars on stable linux systems for compatibility, and use default installs as much as possible for simplicity. When those systems become too old to easily continue building bintars, just stop building them. Existing users can easily transition to compiling themselves if they really need, thanks to (2); or use another of the different options available for running MariaDB. And finally Vicentiu, let me commend you for taking responsibility of the task of producing bintars and working so constructively on getting to a working result. It is a task the difficulty of which is easily underappreciated. So these are my opinions and suggestions, hope this helps. - Kristian.

2 1

Re: d47b1ebc06d: MDEV-35326: Memory Leak in init_io_cache_ext upon SHUTDOWN
by Sergei Golubchik 10 Jan '25

10 Jan '25

Hi, Oleksandr, On Jan 09, Oleksandr Byelkin wrote: > revision-id: d47b1ebc06d (mariadb-10.5.27-143-gd47b1ebc06d) > parent(s): 3bbbeae7924 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2025-01-08 14:14:10 +0100 > message: > > MDEV-35326: Memory Leak in init_io_cache_ext upon SHUTDOWN > > The problems were that: > 1) resources was freed "asimetric" normal execution in send_eof, > in case of error in destructor. > 2) destructor was not called in case of SP for result objects. > (so if the last SP execution ended with error resorces was not > freeded on reinit before execution (cleanup() called before next > execution) and destructor also was not called due to lack of > delete call for the object) > > All result method revised and freeing resources made "symetric". > > Destructor of result object called for SP. > > Added skipped invalidation in case of error in insert. > > Removed misleading naming of reset(thd) (could be mixed with > with reset()) > > diff --git a/sql/sp_head.cc b/sql/sp_head.cc > --- a/sql/sp_head.cc > +++ b/sql/sp_head.cc > @@ -3665,6 +3665,18 @@ int sp_lex_keeper::cursor_reset_lex_and_exec_core(THD *thd, uint *nextp, > return res; > } > > +sp_lex_keeper::~sp_lex_keeper() > +{ > + if (m_lex_resp) > + { > + /* Prevent endless recursion. */ > + m_lex->sphead= NULL; > + if (m_lex->result) you don't need this if(), the standard explicitly says that delete of a NULL pointer is valid. > + delete m_lex->result; > + lex_end(m_lex); > + delete m_lex; > + } > +} > > /* > sp_instr class functions > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -3192,10 +3193,22 @@ bool select_send::send_eof() > Handling writing to file > ************************************************************************/ > > +bool select_to_file::free_recources() > +{ > + if (file >= 0) > + { > + (void) end_io_cache(&cache); > + bool error= mysql_file_close(file, MYF(MY_WME)); > + file= -1; > + return error; > + } > + return FALSE; > +} > + > bool select_to_file::send_eof() > { > int error= MY_TEST(end_io_cache(&cache)); > - if (unlikely(mysql_file_close(file, MYF(MY_WME))) || > + if (unlikely(free_recources()) || free_recources() incluses end_io_cache(), you don't need to call it separately above. > unlikely(thd->is_error())) > error= true; > > @@ -5934,7 +5935,15 @@ class select_to_file :public select_result_interceptor { > { path[0]=0; } > ~select_to_file(); > bool send_eof() override; > + void abort_result_set() override; > void cleanup() override; > + /* > + It is separated from cleanup() becaouse: 1. typo 2. if you rename cleanup() to reset_for_next_ps_execution() then you won't need that comment, because, obviously, "freeing resources" is distinct from "resetting for next ps execution" but it's not obvious that "freeing resources" is distinct from "cleanup" > + 1. cleanup can not be called (conventional execution) > + 2. cleanup not only free recources but reset counters (some sends > + counter updates in destructor) > + */ > + bool free_recources(); > }; > > > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc > --- a/sql/sql_delete.cc > +++ b/sql/sql_delete.cc > @@ -1422,6 +1422,13 @@ void multi_delete::abort_result_set() > { > DBUG_ENTER("multi_delete::abort_result_set"); > > + /**************************************************************************** > + > + NOTE: if you change here be aware that almost the same code is in > + multi_delete::send_eof(). > + > + ***************************************************************************/ Let's talk about it separately. > + > /* the error was handled or nothing deleted and no side effects return */ > if (error_handled || > (!thd->transaction->stmt.modified_non_trans_table && !deleted)) > diff --git a/sql/sql_update.cc b/sql/sql_update.cc > --- a/sql/sql_update.cc > +++ b/sql/sql_update.cc > @@ -2743,6 +2743,13 @@ void multi_update::abort_result_set() > (!thd->transaction->stmt.modified_non_trans_table && !updated))) > return; > > + /**************************************************************************** > + > + NOTE: if you change here be aware that almost the same code is in > + multi_delete::send_eof(). multi_update::send_eof() > + > + ***************************************************************************/ > + > /* Something already updated so we have to invalidate cache */ > if (updated) > query_cache_invalidate3(thd, update_tables, 1); Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Re: 331430a6a73: MDEV-33658 2/2 Cannot add a foreign key on a table with a matching long UNIQUE
by Sergei Golubchik 09 Jan '25

09 Jan '25

Hi, Nikita, On Jan 01, Nikita Malyavin wrote: > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index de688b4cb31..2ffceb3932b 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -217,6 +218,16 @@ Foreign_key::Foreign_key(const Foreign_key &rhs, MEM_ROOT *mem_root) > > bool is_foreign_key_prefix(Key *a, Key *b) > { > + ha_key_alg a_alg= a->key_create_info.algorithm; > + ha_key_alg b_alg= b->key_create_info.algorithm; > + > + // The real algorithm will be BTREE is none was given by user. Technically, this is not correct, the algorithm will be whatever the engine wants. For MEMORY engine it'll be HASH. But only InnoDB can do foreign keys now, and for InnoDB th3 default key algorithm is BTREE. Let's reflect it in the comment, like, for example (also fixed a typo "is" -> "if") // The real algorithm in InnoDB will be BTREE if none was given by user > + a_alg= a_alg == HA_KEY_ALG_UNDEF ? HA_KEY_ALG_BTREE : a_alg; > + b_alg= b_alg == HA_KEY_ALG_UNDEF ? HA_KEY_ALG_BTREE : b_alg; > + > + if (a_alg != b_alg) > + return false; > + > /* Ensure that 'a' is the generated key */ > if (a->generated) > { > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 3f035defbd3..d7aeb5d7cbf 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -3556,6 +3556,303 @@ key_add_part_check_null(const handler *file, KEY *key_info, > } > > > +static > +my_bool key_check_without_overlaps(THD *thd, HA_CREATE_INFO *create_info, > + Alter_info *alter_info, > + Key &key) > +{ > + DBUG_ENTER("key_check_without_overlaps"); > + if (!key.without_overlaps) > + DBUG_RETURN(FALSE); > + // append_system_key_parts is already called, so we should check all the > + // columns except the last two. > + const auto &period_start= create_info->period_info.period.start; > + const auto &period_end= create_info->period_info.period.end; > + List_iterator<Key_part_spec> part_it_forwarded(key.columns); > + List_iterator<Key_part_spec> part_it(key.columns); > + part_it_forwarded++; > + part_it_forwarded++; > + while (part_it_forwarded++) > + { > + Key_part_spec *key_part= part_it++; > + > + if (period_start.streq(key_part->field_name) > + || period_end.streq(key_part->field_name)) > + { > + my_error(ER_KEY_CONTAINS_PERIOD_FIELDS, MYF(0), key.name.str, > + key_part->field_name.str); > + DBUG_RETURN(TRUE); > + } > + } It was simpler to have this check inside append_system_key_parts(), why did you move it? > + > + if (key.key_create_info.algorithm == HA_KEY_ALG_HASH || > + key.key_create_info.algorithm == HA_KEY_ALG_LONG_HASH) > + > + { > + my_error(ER_KEY_CANT_HAVE_WITHOUT_OVERLAPS, MYF(0), key.name.str); > + DBUG_RETURN(TRUE); > + } <...> > + KEY_CREATE_INFO *key_cinfo= &key.key_create_info; > + > + if (is_hash_field_needed) > + { > + if (key_cinfo->algorithm == HA_KEY_ALG_UNDEF) > + key_cinfo->algorithm= HA_KEY_ALG_LONG_HASH; Perhaps you shouldn't set key algorithm in add_hash_field() then? or, rather replace the assignment with an assert? > + > + if (key_cinfo->algorithm != HA_KEY_ALG_HASH && > + key_cinfo->algorithm != HA_KEY_ALG_LONG_HASH) > + { > + my_error(ER_TOO_LONG_KEY, MYF(0), max_key_length); > + DBUG_RETURN(TRUE); > + } > + } > + } > + } > + DBUG_RETURN(FALSE); > +} > + > + > /* > Preparation for table creation Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 2

MDEV-29744 fix required when merging to 11.4
by Kristian Nielsen 09 Jan '25

09 Jan '25

Hi Sanja, others who might need to merge 10.6->10.11->11.4: The patch for MDEV-29744 in 10.6 needs a fix when merged to 11.4, otherwise test binlog.max_binlog_total_size (at least) will fail. I have attached the required fix. I could not push this fix to 11.4 myself, as branch protection is broken and rejected my push on a false positive. - Kristian.

1 0

Re: 87188cd3309: MDEV-29744: Fix incorrect locking order of LOCK_log/LOCK_commit_ordered and LOCK_global_system_variables
by Sergei Golubchik 08 Jan '25

08 Jan '25

Hi, Kristian, Mostly ok, but I'd prefer not to access max_binlog_size without any protection, so suggest to do something like ulong saved= max_binlog_size; mysql_mutex_unlock(&LOCK_global_system_variables); mysql_bin_log.set_max_size(saved); mysql_mutex_lock(&LOCK_global_system_variables); On Jan 08, Kristian Nielsen wrote: > revision-id: 87188cd3309 (mariadb-10.6.20-47-g87188cd3309) > parent(s): 7372ecc3965 > author: Kristian Nielsen > committer: Kristian Nielsen > timestamp: 2024-12-05 09:51:17 +0100 > message: > > MDEV-29744: Fix incorrect locking order of LOCK_log/LOCK_commit_ordered and LOCK_global_system_variables > > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -1660,7 +1660,9 @@ Sys_max_binlog_stmt_cache_size( > > static bool fix_max_binlog_size(sys_var *self, THD *thd, enum_var_type type) > { > + mysql_mutex_unlock(&LOCK_global_system_variables); > mysql_bin_log.set_max_size(max_binlog_size); > + mysql_mutex_lock(&LOCK_global_system_variables); > return false; > } > static Sys_var_on_access_global<Sys_var_ulong, Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Why is innodb-log-file-mmap is available only on amd64/arm64/ppc64el but not other Linux architectures?
by Otto Kekäläinen 08 Jan '25

08 Jan '25

Hi, The documentation at https://mariadb.com/kb/en/innodb-system-variables/#innodb_log_file_mmap says this is supposed to be enabled by default on Linux, regardless of the architecture. However, in my testing it is not available on all architectures. Can somebody help me read the code path of innodb-log-file-mmap to figure out why the visibility/availability of this option it is currently architecture-dependent? ## Context In Debian I maintain an extra test called "configuration-trace" which basically dumps the output of these four commands into a file: /usr/bin/mariadb --print-defaults /usr/bin/mariadb --verbose --help /usr/sbin/mariadbd --print-defaults /usr/sbin/mariadbd --verbose --help Then, using the static file containing the text output of these files, it compares to the output of these commands on a running system to detect if the MariaDB defaults or features randomly changed (for example due to changes in dependencies or platform). This "configuration-trace" is also a handy way to detect what changed upstream in minor MariaDB releases, which aren't supposed to have much changes. The update of the static text output files for MariaDB 11.4.4 were done in https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/e2851fcfca9e8… and they show among others a new variable innodb-log-file-mmap ## Problem The issues I am running into is that this new innodb-log-file-mmap only seems to exist on amd64 and arm64? I discovered this in the Debian CI system (autpkgtests) at https://ci.debian.net/packages/m/mariadb/ which is passing for amd64/arm64/ppc64el, but failing on other platforms: https://ci.debian.net/packages/m/mariadb/unstable/armel/55869348/ https://ci.debian.net/packages/m/mariadb/unstable/armhf/55869349/ https://ci.debian.net/packages/m/mariadb/unstable/i386/55869350/ https://ci.debian.net/packages/m/mariadb/unstable/loong64/55869351/ https://ci.debian.net/packages/m/mariadb/unstable/s390x/55869354/ (The output also shows that mariadbd --verbose --help prints out wsrep-provider twice, once wsrep-provider=ON and once wsrep-provider=none, but that is a different issue.) On systems where it isn't available, shouldn't the option still show up (but as permanently disabled)? It seems odd to have system variables appear or disappear depending on the Linux architecture.

2 8

Re: b16319bdd0f: MDEV-35469 Heap tables are calling mallocs to often
by Sergei Golubchik 30 Dec '24

30 Dec '24

Hi, Monty, > commit b16319bdd0f > Author: Michael Widenius <monty(a)mariadb.org> > AuthorDate: Thu Nov 21 12:28:57 2024 +0200 > Commit: Michael Widenius <monty(a)mariadb.org> > CommitDate: Fri Dec 27 13:53:37 2024 +0200 > > diff --git a/storage/heap/hp_block.c b/storage/heap/hp_block.c > index 324efc8b4af..44bca363746 100644 > --- a/storage/heap/hp_block.c > +++ b/storage/heap/hp_block.c > @@ -75,9 +75,11 @@ int hp_get_new_block(HP_SHARE *info, HP_BLOCK *block, size_t *alloc_length) > Next time we allocate data for X rows. > When level 1 is full, we allocate data for HP_PTRS_IN_NOD at level 2 and 1 > + X rows at level 0. > */ > *alloc_length= (sizeof(HP_PTRS) * ((i == block->levels) ? i : i - 1) + > (ulonglong)block->records_in_block * block->recbuffer); > + /* Alloc in blocks of powers of 2 */ > + *alloc_length= MY_MAX(*alloc_length, block->alloc_size); What will happen to block->alloc_size - *alloc_length bytes? they'll be unused? > if (!(root=(HP_PTRS*) my_malloc(hp_key_memory_HP_PTRS, *alloc_length, > MYF(MY_WME | > (info->internal ? > diff --git a/storage/heap/hp_create.c b/storage/heap/hp_create.c > index c07a1e968c4..29812f08107 100644 > --- a/storage/heap/hp_create.c > +++ b/storage/heap/hp_create.c > @@ -15,11 +15,23 @@ > Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1335 USA */ > > #include "heapdef.h" > +#include <my_bit.h> > > static int keys_compare(void *heap_rb, const void *key1, const void *key2); > static void init_block(HP_BLOCK *block,uint reclength,ulong min_records, > ulong max_records); > > + > +/* > + In how many parts are we going to do allocations of memory and indexes > + If we assigne 1M to the heap table memory, we will allocate roughly > + (1M/16) bytes per allocaiton > +*/ > +int heap_allocation_parts= 16; static. technically, it could be `static const`, But I presume the compiler can figure out the `const` part if the variable is static. And e.g. use >> 4. Otherwise it has to assume it can be modified in some other file. > + > +/* min block allocation */ > +ulong heap_min_allocation_block= 16384; same. static. > + > /* Create a heap table */ > > int heap_create(const char *name, HP_CREATE_INFO *create_info, > @@ -170,7 +182,8 @@ int heap_create(const char *name, HP_CREATE_INFO *create_info, > share->keydef= (HP_KEYDEF*) (share + 1); > share->key_stat_version= 1; > keyseg= (HA_KEYSEG*) (share->keydef + keys); > - init_block(&share->block, visible_offset + 1, min_records, max_records); > + init_block(&share->block, hp_memory_needed_per_row(visible_offset + 1), visible_offset is already MY_MAX(reclength, sizeof (char*)). perhaps you don't need visible_offset and should use hp_memory_needed_per_row(reclength) here? > + min_records, max_records); > /* Fix keys */ > memcpy(share->keydef, keydef, (size_t) (sizeof(keydef[0]) * keys)); > for (i= 0, keyinfo= share->keydef; i < keys; i++, keyinfo++) > @@ -266,44 +279,90 @@ static int keys_compare(void *heap_rb_, const void *key1_, > heap_rb->search_flag, not_used); > } > > + > +/* > + Calculate length needed for storing one row > +*/ > + > +size_t hp_memory_needed_per_row(size_t reclength) > +{ > + /* Data needed for storing record + pointer to records */ > + reclength= MY_MAX(reclength, sizeof(char*) + 1); > + reclength= MY_ALIGN(reclength, sizeof(char*)); > + return reclength; > +} > + > +/* > + Calculate the number of rows that fits into a given memory size > +*/ > + > +ha_rows hp_rows_in_memory(size_t reclength, size_t index_size, > + size_t memory_limit) > +{ > + /* We need one extra byte for the delete flag */ > + reclength= hp_memory_needed_per_row(reclength+1); you have +1 for the delete flag in hp_memory_needed_per_row() > + if ((memory_limit - sizeof(HP_PTRS)) < (index_size + reclength)) > + return 0; /* Wrong arguments */ Why do you need this if()? If its condition is true the expression below will return 0 naturally, you don't need a separate check for that. > + return (ha_rows) ((memory_limit - sizeof(HP_PTRS)) / > + (index_size + reclength)); > +} > + > + > static void init_block(HP_BLOCK *block, uint reclength, ulong min_records, > ulong max_records) > { > diff --git a/storage/heap/hp_write.c b/storage/heap/hp_write.c > index 5469784c8c1..cb079eac757 100644 > --- a/storage/heap/hp_write.c > +++ b/storage/heap/hp_write.c > @@ -145,6 +145,8 @@ static uchar *next_free_record_pos(HP_SHARE *info) > DBUG_PRINT("exit",("Used old position: %p", pos)); > DBUG_RETURN(pos); > } > + if (!(block_pos=(info->records % info->block.records_in_block))) > + { why? > if ((info->records > info->max_records && info->max_records) || > (info->data_length + info->index_length >= info->max_table_size)) > { Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 2

Re: 448e82f9f0d: MDEV-35720 Add query_time to statistics
by Sergei Golubchik 30 Dec '24

30 Dec '24

Hi, Monty, Only a couple of comments about the test case: > commit 448e82f9f0d > Author: Michael Widenius <monty(a)mariadb.org> > Date: Fri Dec 27 19:26:55 2024 +0200 > > diff --git a/mysql-test/main/log_state.test b/mysql-test/main/log_state.test > --- a/mysql-test/main/log_state.test > +++ b/mysql-test/main/log_state.test > @@ -321,6 +321,48 @@ SET GLOBAL general_log_file = @old_general_log_file; > > --enable_ps_protocol > > +--echo # > +--echo # MDEV-35720 Add query_time to statistics > +--echo # > + > +connect (con2,localhost,root,,); > +let $s1=`show status like "query_time"`; > +select sleep(3) into @a; > +let $s2=`show status like "query_time"`; > +let $s3=`show global status like "query_time"`; > +--disable_query_log > +--eval SET @s1= SUBSTRING_INDEX('$s1', '\t', -1); > +--eval SET @s2= SUBSTRING_INDEX('$s2', '\t', -1); > +--eval SET @s3= SUBSTRING_INDEX('$s3', '\t', -1); Eh, that's not how one is supposed to do it. I mean, it works, but the easier approach is let $s1=query_get_value(show status like "query_time", Value); or purely in SQL set @s1=(select variable_value from information_schema.global_status where variable_name='query_time'); > +select @s1 >= 0.00 and @s1 <= 2.00 as "should be true"; > +--eval select @s2 >= 2.00 and @s2 < 10.00 as "should be true"; > +--eval select @s3 >= 3.00 as "should be true"; 1. you don't need eval here 2. I expect this test, like almost all other timing tests will be horribly unstable in buildbot and will be eventually disabled. No, I don't have a recipe for a stable timing test. > +--enable_query_log > + > +# select @s1,@s2,@s3; > +disconnect con2; > +connection default; > + > +delimiter |; > +create procedure p1() > +begin > +select sleep(1) into @a; > +select sleep(2) into @a; > +end| > +delimiter ;| > + > +connect (con2,localhost,root,,); > +call p1(); > +let $s1=`show status like "query_time"`; > +--disable_query_log > +--eval SET @s1= SUBSTRING_INDEX('$s1', '\t', -1); > +--eval select @s1 > 2.00 and @s1 < 10.00 as "should be true"; > +--enable_query_log > + > +# select @s1; > +disconnect con2; > +connection default; > + > # > # Cleanup > # Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Re: dfc1632da73: Change default MEM_ROOT allocations to decrease calls to malloc
by Sergei Golubchik 27 Dec '24

27 Dec '24

Hi, Monty, Please, cherry-pick it into 10.11 and push as bb-10.11-merge. There will be *a lot* of conflicts in memroot code because of your "Added support to MEM_ROOT for write protected memory" change, it'd be best if you resolve your conflicts yourself. On Dec 22, Michael Widenius wrote: > revision-id: dfc1632da73 (mariadb-10.6.20-60-gdfc1632da73) > parent(s): 3297680579f > author: Michael Widenius > committer: Michael Widenius > timestamp: 2024-12-17 10:45:25 +0200 > message: > > Change default MEM_ROOT allocations to decrease calls to malloc ... > --- a/mysys/my_alloc.c > +++ b/mysys/my_alloc.c > @@ -173,7 +180,7 @@ void *alloc_root(MEM_ROOT *mem_root, size_t length) > DBUG_ENTER("alloc_root"); > - DBUG_PRINT("enter",("root: %p", mem_root)); > + DBUG_PRINT("enter",("root: %p length: %ld", mem_root, (long) length)); FYI, my_vsnprintf() supports standard "%zu" to print size_t values. Not important here, of course. > DBUG_ASSERT(alloc_root_inited(mem_root)); Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Re: 2ba3bbb2463: Updated misc_session_status.test to not fail if select does not fail
by Sergei Golubchik 27 Dec '24

27 Dec '24

Hi, Monty, On Dec 22, Michael Widenius wrote: > revision-id: 2ba3bbb2463 (mariadb-10.6.20-61-g2ba3bbb2463) > parent(s): dfc1632da73 > author: Michael Widenius > committer: Michael Widenius > timestamp: 2024-12-17 10:45:25 +0200 > message: > > Updated misc_session_status.test to not fail if select does not fail > > The test with memory restrictions randomly works or fails depending on > server configurations. > As the test was there to just check if the server crashes when run with > small memory configurations, I disabled testing if the query would fail > or not. The test still has its original purpose. This is very questionable. The test would still serve its original purpose indeed if it'll hit OOM at least somewhere. But with your change it's not clear that it'll ever do. The query might as well succeed everywhere and we'll never know. Or it might start succeeding in the future. That is, technically you are right, the test still does its job (presuming you've verified that it hits OOM at least on some builders). But your change basically guarantees that it'll become meaningless in the future. Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1