- developers - lists.mariadb.org

Re: 0a0c5f349c6: MDEV-36425 Extend read_only to also block share locks and super user
by Sergei Golubchik 01 Apr '25

01 Apr '25

Hi, Monty, On Apr 01, Michael Widenius wrote: > revision-id: 0a0c5f349c6 (mariadb-11.8.1-42-g0a0c5f349c6) > parent(s): 21321025e5e > author: Michael Widenius > committer: Michael Widenius > timestamp: 2025-03-31 20:09:03 +0300 > message: > > MDEV-36425 Extend read_only to also block share locks and super user > > The main purpose of this allow one to use the --read-only > option to ensure that no one can issue a query that can > block replication. > > The --read-only option can now take 4 different values: > 0 No read only (as before). > 1 Blocks changes for users without the 'READ ONLY ADMIN' > privilege (as before). > 2 Blocks in addition LOCK TABLES and SELECT IN SHARE MODE > for not 'READ ONLY ADMIN' users. > 3 Blocks in addition 'READ_ONLY_ADMIN' users for all the > previous statements. > diff --git a/mysql-test/main/read_only.result b/mysql-test/main/read_only.result > index 65cc12ffce9..89390ad65d5 100644 > --- a/mysql-test/main/read_only.result > +++ b/mysql-test/main/read_only.result > @@ -69,11 +69,14 @@ set global read_only=1; > connection con1; > select @@global.read_only; > @@global.read_only > -0 > +OFF > unlock tables ; > +Timeout in wait_condition.inc for SELECT @@global.read_only= 1 > +Id User Host db Command Time State Info Progress > +5 test localhost test Query 0 starting show full processlist 0.000 Forgot to fix? It should wait for @@global.read_only='READ_ONLY' now > select @@global.read_only; > @@global.read_only > -1 > +READ_ONLY > connection default; > reap; > connection default; > diff --git a/sql/lock.cc b/sql/lock.cc > index 1296e5455d5..8f9f97f88b7 100644 > --- a/sql/lock.cc > +++ b/sql/lock.cc > @@ -179,19 +198,48 @@ lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags) > > /* > Prevent modifications to base tables if READ_ONLY is activated. > - In any case, read only does not apply to temporary tables. > + In any case, read only does not apply to temporary tables or slave > + threads. > */ > - if (!(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) && !t->s->tmp_table) > + if (unlikely(opt_readonly) && > + !(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) && !t->s->tmp_table && > + !thd->slave_thread) > { > - if (t->reginfo.lock_type >= TL_FIRST_WRITE && > - !ignore_read_only && opt_readonly && !thd->slave_thread) > + switch (opt_readonly) // Impossible you have "Impossible" comment on a wrong line > { > - my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); > - DBUG_RETURN(1); > + case READONLY_OFF: > + DBUG_ASSERT(0); > + break; > + case READONLY_ON: // Compatibility > + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > + t->reginfo.lock_type >= TL_FIRST_WRITE) > + { > + mariadb_error_read_only(); > + DBUG_RETURN(1); I'd make mariadb_error_read_only() to return 1. Then you can write DBUG_RETURN(mariadb_error_read_only()); here and also in all other places, check it out, it's always followed by return 1. > + } > + break; > + case READONLY_NO_LOCK: > + if (!(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > + (thd->lex->sql_command == SQLCOM_LOCK_TABLES || > + t->reginfo.lock_type >= TL_FIRST_WRITE || > + t->reginfo.lock_type == TL_READ_WITH_SHARED_LOCKS)) may be t->reginfo.lock_type >= TL_READ_WITH_SHARED_LOCKS ? > + { > + mariadb_error_read_only(); > + DBUG_RETURN(1); > + } > + break; > + case READONLY_NO_LOCK_NO_ADMIN: > + if (thd->lex->sql_command == SQLCOM_LOCK_TABLES || > + t->reginfo.lock_type >= TL_FIRST_WRITE || > + t->reginfo.lock_type == TL_READ_WITH_SHARED_LOCKS) and here > + { > + mariadb_error_read_only(); > + DBUG_RETURN(1); > + } > + break; > } > } > } > - > /* > Locking of system tables is restricted: > locking a mix of system and non-system tables in the same lock > diff --git a/sql/sql_class.h b/sql/sql_class.h > index d65cec37154..34ee8391009 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -3448,9 +3452,12 @@ class THD: public THD_count, /* this must be first */ > */ > inline bool is_read_only_ctx() rename to, for example, check_read_only_with_errror() > { > - return opt_readonly && > - !(security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > - !slave_thread; > + if (likely(!opt_readonly) || slave_thread || > + ((security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && > + opt_readonly != READONLY_NO_LOCK_NO_ADMIN)) > + return false; > + mariadb_error_read_only(); > + return true; > } > > private: > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index 0247afd91a6..545b4674da1 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -3253,15 +3253,24 @@ static bool fix_read_only(sys_var *self, THD *thd, enum_var_type type) > transition (especially when transitioning from false to true) and > synchronizes both booleans in the end. > */ > -static Sys_var_on_access_global<Sys_var_mybool, > + > +const char *read_only_mode_names[]= > +{"OFF", "READ_ONLY", "READ_ONLY_NO_LOCK", "READ_ONLY_NO_LOCK_NO_ADMIN"}; > + > +static Sys_var_on_access_global<Sys_var_enum, > PRIV_SET_SYSTEM_GLOBAL_VAR_READ_ONLY> > Sys_readonly( > "read_only", > - "Make all non-temporary tables read-only, with the exception for " > - "replication (slave) threads and users with the 'READ ONLY ADMIN' " > - "privilege", > - GLOBAL_VAR(read_only), CMD_LINE(OPT_ARG), DEFAULT(FALSE), > - NO_MUTEX_GUARD, NOT_IN_BINLOG, > + "Do not allow changes to non-temporary tables. Options are:" > + "OFF changes allowed. " > + "READ_ONLY blocks changes for users without the 'READ ONLY ADMIN' " "Disallow changes for users without the READ ONLY ADMIN privilege" > + "privilege. " > + "READ_ONLY_NO_LOCK blocks in addition LOCK TABLES and " > + "SELECT IN SHARE MODE for not ADMIN users. " "Additionally disallows LOCK TABLES and SELECT IN SHARE MODE" > + "READ_ONLY_NO_LOCK_NO_ADMIN blocks in addition 'ADMIN' users. " "Disallows also for users with READ_ONLY ADMIN privilege" > + "Replication (slave) threads are not affected by this option", > + GLOBAL_VAR(read_only), CMD_LINE(OPT_ARG), > + read_only_mode_names, DEFAULT(0), NO_MUTEX_GUARD, NOT_IN_BINLOG, > ON_CHECK(check_read_only), ON_UPDATE(fix_read_only)); > > // Small lower limit to be able to test MRR Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Inquiry About GSoC 2025 and Proposal Submission
by Harsh Gupta 31 Mar '25

31 Mar '25

*Dear MariaDB Developers,* I hope you are doing well. My name is *Harsh Gupta*, and I am a *second-year B.Tech Computer Science and Design student at Madhav Institute of Technology and Science, Gwalior*. I am passionate about databases and open-source development, and I am eager to contribute to MariaDB through *Google Summer of Code 2025*. I have gone through the project ideas listed for GSoC and found the MDEV-28395 <https://jira.mariadb.org/browse/MDEV-28395> LOAD DATA plugins particularly interesting. I have experience with *C++ and database systems*, and I have been exploring MariaDB’s codebase to get familiar with its architecture. I have prepared a draft proposal outlining my approach to this project, including my implementation plan and timeline. I would appreciate your feedback to refine it further before submitting it officially. Could you please review my proposal and provide any suggestions for improvement? Also, I would love to connect with potential mentors to discuss the project's scope and feasibility. Looking forward to your guidance and feedback! Best regards, *Harsh Gupta* 📧 harshgupta9329(a)gmail.com

1 0

Re: b7dbe187b1a: MDEV-34680 Asynchronous and Buffered Logging for Audit Plugin.
by Sergei Golubchik 31 Mar '25

31 Mar '25

Hi, Alexey, See few comments below. But the main question, could you have used FILE or IO_CACHE instead of a custom implementation? On Mar 08, Alexey Botchkov wrote: > revision-id: b7dbe187b1a (mariadb-11.8.1-8-gb7dbe187b1a) > parent(s): 6f1161aa34c > author: Alexey Botchkov > committer: Alexey Botchkov > timestamp: 2025-03-02 16:27:23 +0400 > message: > > MDEV-34680 Asynchronous and Buffered Logging for Audit Plugin. > > Buffering added to the file logger. > Two buffers are created so writing to the file doesn't > block other threads from logging. > Operations isolated with two mutexes. > diff --git a/include/mysql/service_logger.h b/include/mysql/service_logger.h > index 35c2eb1e3a9..adce7bc9038 100644 > --- a/include/mysql/service_logger.h > +++ b/include/mysql/service_logger.h > @@ -64,41 +64,44 @@ extern struct logger_service_st { > void (*logger_init_mutexes)(); > LOGGER_HANDLE* (*open)(const char *path, > unsigned long long size_limit, > - unsigned int rotations); > + unsigned int rotations, size_t buffer_size); incompatible API change. MUST change the major service version. > int (*close)(LOGGER_HANDLE *log); > int (*vprintf)(LOGGER_HANDLE *log, const char *fmt, va_list argptr) > ATTRIBUTE_FORMAT_FPTR(printf, 2, 0); > int (*printf)(LOGGER_HANDLE *log, const char *fmt, ...) > ATTRIBUTE_FORMAT_FPTR(printf, 2, 3); > - int (*write)(LOGGER_HANDLE *log, const char *buffer, size_t size); > + int (*write)(LOGGER_HANDLE *log, const void *data, size_t size); > int (*rotate)(LOGGER_HANDLE *log); > + int (*sync)(LOGGER_HANDLE *log); > } *logger_service; > > #ifdef MYSQL_DYNAMIC_PLUGIN > > #define logger_init_mutexes logger_service->logger_init_mutexes > -#define logger_open(path, size_limit, rotations) \ > - (logger_service->open(path, size_limit, rotations)) > +#define logger_open(path, size_limit, rotations, buffer_size) \ > + (logger_service->open(path, size_limit, rotations, buffer_size)) > #define logger_close(log) (logger_service->close(log)) > #define logger_rotate(log) (logger_service->rotate(log)) > +#define logger_rsync(log) (logger_service->sync(log)) > #define logger_vprintf(log, fmt, argptr) (logger_service->\ > vprintf(log, fmt, argptr)) > #define logger_printf (*logger_service->printf) > -#define logger_write(log, buffer, size) \ > - (logger_service->write(log, buffer, size)) > +#define logger_write(log, data, size) \ > + (logger_service->write(log, data, size)) > #else > > void logger_init_mutexes(); > LOGGER_HANDLE *logger_open(const char *path, > unsigned long long size_limit, > - unsigned int rotations); > + unsigned int rotations, size_t buffer_size); > int logger_close(LOGGER_HANDLE *log); > int logger_vprintf(LOGGER_HANDLE *log, const char *fmt, va_list argptr) > ATTRIBUTE_FORMAT(printf, 2, 0); > int logger_printf(LOGGER_HANDLE *log, const char *fmt, ...) > ATTRIBUTE_FORMAT(printf, 2, 3); > - int logger_write(LOGGER_HANDLE *log, const char *buffer, size_t size); > + int logger_write(LOGGER_HANDLE *log, const void *data, size_t size); > int logger_rotate(LOGGER_HANDLE *log); > + int logger_sync(LOGGER_HANDLE *log); > #endif > > > diff --git a/mysys/file_logger.c b/mysys/file_logger.c > index a753c049f68..5464e43eddf 100644 > --- a/mysys/file_logger.c > +++ b/mysys/file_logger.c > @@ -145,85 +230,108 @@ static int do_rotate(LOGGER_HANDLE *log) > namebuf[log->path_len]= 0; > result= my_rename(namebuf, logname(log, log->path, 1), MYF(0)); > log->file= my_open(namebuf, LOG_FLAGS, MYF(0)); > + log->in_file= 0; > exit: > errno= my_errno; > return log->file < 0 || result; > } > > > -/* > - Return 1 if we should rotate the log > -*/ > - > -my_bool logger_time_to_rotate(LOGGER_HANDLE *log) > -{ > - my_off_t filesize; > - if (log->rotations > 0 && > - (filesize= my_tell(log->file, MYF(0))) != (my_off_t) -1 && > - ((ulonglong) filesize >= log->size_limit)) > - return 1; > - return 0; > -} > - > - > int logger_vprintf(LOGGER_HANDLE *log, const char* fmt, va_list ap) > { > - int result; > char cvtbuf[1024]; > size_t n_bytes; > > - flogger_mutex_lock(&log->lock); > - if (logger_time_to_rotate(log) && do_rotate(log)) > - { > - result= -1; > - errno= my_errno; > - goto exit; /* Log rotation needed but failed */ > - } > - > n_bytes= my_vsnprintf(cvtbuf, sizeof(cvtbuf), fmt, ap); > if (n_bytes >= sizeof(cvtbuf)) > n_bytes= sizeof(cvtbuf) - 1; > > - result= (int)my_write(log->file, (uchar *) cvtbuf, n_bytes, MYF(0)); > - > -exit: > - flogger_mutex_unlock(&log->lock); > - return result; > + return logger_write(log, cvtbuf, n_bytes); > } > > > -static int logger_write_r(LOGGER_HANDLE *log, my_bool allow_rotations, > - const char *buffer, size_t size) > +int logger_write(LOGGER_HANDLE *log, const void *data, size_t size) > { > int result; > + size_t in_buffer; > + > + mysql_mutex_lock(&log->lock); > > - flogger_mutex_lock(&log->lock); > - if (allow_rotations && logger_time_to_rotate(log) && do_rotate(log)) > + if (log->buffer_left >= size) > + { > + memcpy(log->buffer_pos, data, size); > + log->buffer_pos+= size; > + log->buffer_left-= size; > + mysql_mutex_unlock(&log->lock); > + return size; > + } > + > + mysql_mutex_lock(&log->lock_buffer); > + > + in_buffer= log->buffer_pos - log->buffer; > + > + { /* swap buffers. */ > + uchar *tmp_buf= log->buffer; > + log->buffer= log->idle_buffer; > + log->idle_buffer= tmp_buf; > + } > + > + set_buffer_pos_and_left(log); > + > + /* Now other threads can write to the new buffer. */ > + mysql_mutex_unlock(&log->lock); > + > + if ((in_buffer && > + my_write(log->file, log->idle_buffer, in_buffer, MYF(0)) != in_buffer)|| > + my_write(log->file, (uchar *) data, size, MYF(0)) != size) > { > result= -1; > errno= my_errno; > - goto exit; /* Log rotation needed but failed */ > + goto exit_buf; > } > + else > + result= 0; > > - result= (int)my_write(log->file, (uchar *) buffer, size, MYF(0)); > + log->in_file+= in_buffer + size; > + if (log->rotations > 0 && log->in_file >= log->size_limit) > + { > + if (do_rotate(log)) > + { > + result= -1; > + errno= my_errno; > + goto exit_buf; /* Log rotation needed but failed */ > + } > + } > > -exit: > - flogger_mutex_unlock(&log->lock); > +exit_buf: > + mysql_mutex_unlock(&log->lock_buffer); > return result; > } > > > -int logger_write(LOGGER_HANDLE *log, const char *buffer, size_t size) > -{ > - return logger_write_r(log, TRUE, buffer, size); > -} > - > int logger_rotate(LOGGER_HANDLE *log) > { > int result; > - flogger_mutex_lock(&log->lock); > - result= do_rotate(log); > - flogger_mutex_unlock(&log->lock); > + mysql_mutex_lock(&log->lock); > + mysql_mutex_lock(&log->lock_buffer); > + result= flush_buffer(log) || do_rotate(log); I'd though you can switch buffers here too and flush/rotate only under the lock_buffer mutex > + mysql_mutex_unlock(&log->lock_buffer); > + mysql_mutex_unlock(&log->lock); > + return result; > +} > + > + > +int logger_sync(LOGGER_HANDLE *log) Why did you introduce an explicit sync API? > +{ > + int result; > + mysql_mutex_lock(&log->lock); > + mysql_mutex_lock(&log->lock_buffer); > + if ((result= flush_buffer(log))) > + { > + result= fsync(log->file); > + } > + mysql_mutex_unlock(&log->lock_buffer); > + mysql_mutex_unlock(&log->lock); > return result; > } > > diff --git a/plugin/server_audit/server_audit.c b/plugin/server_audit/server_audit.c > index 5fc0e3373f3..e51df7ab6b5 100644 > --- a/plugin/server_audit/server_audit.c > +++ b/plugin/server_audit/server_audit.c > @@ -1393,26 +1401,15 @@ static void change_connection(struct connection_info *cn, > static int write_log(const char *message, size_t len, int take_lock) > { > int result= 0; > + > if (take_lock) > - { > - /* Start by taking a read lock */ > mysql_prlock_rdlock(&lock_operations); > - } > > if (output_type == OUTPUT_FILE) > { > if (logfile) > { > - my_bool allow_rotate= !take_lock; /* Allow rotate if caller write lock */ > - if (take_lock && logger_time_to_rotate(logfile)) > - { > - /* We have to rotate the log, change above read lock to write lock */ > - mysql_prlock_unlock(&lock_operations); > - mysql_prlock_wrlock(&lock_operations); > - allow_rotate= 1; > - } > - if (!(is_active= (logger_write_r(logfile, allow_rotate, message, len) == > - (int) len))) > + if (!(is_active= (logger_write(logfile, message, len) == (int) len))) nice, rotation is indeed the logger's headache, not the caller's > { > ++log_write_failures; > result= 1; > @@ -2899,6 +2906,36 @@ static void update_file_rotations(MYSQL_THD thd __attribute__((unused)), > } > > > +static void update_file_buffer_size(MYSQL_THD thd __attribute__((unused)), > + struct st_mysql_sys_var *var __attribute__((unused)), > + void *var_ptr __attribute__((unused)), const void *save) > +{ > + file_buffer_size= *(unsigned int *) save; > + > + error_header(); > + fprintf(stderr, "Log file buffer size was changed to '%d'.\n", file_buffer_size); > + > + if (!logging || output_type != OUTPUT_FILE) > + return; > + > + ADD_ATOMIC(internal_stop_logging, 1); > + if (!maria_55_started || !debug_server_started) > + mysql_prlock_wrlock(&lock_operations); > + > + stop_logging(); > + if (start_logging()) this could be an API call in the logger. Like "reinit(new params)" and it'll close, change params, open - all under a log mutex, which doesn't need to be destroyed and reinitialized. > + { > + error_header(); > + fprintf(stderr, "Logging was disabled..\n"); > + CLIENT_ERROR(1, "Logging was disabled.", MYF(ME_WARNING)); > + } > + > + if (!maria_55_started || !debug_server_started) > + mysql_prlock_unlock(&lock_operations); > + ADD_ATOMIC(internal_stop_logging, -1); > +} > + > + > static void update_file_rotate_size(MYSQL_THD thd __attribute__((unused)), > struct st_mysql_sys_var *var __attribute__((unused)), > void *var_ptr __attribute__((unused)), const void *save) Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Introduction & Interest in GSoC 2025 with MariaDB
by H.NAVEENKUMAR 30 Mar '25

30 Mar '25

*Hello MariaDB Developers,* I hope you're all doing well. My name is *Naveen Kumar*, and I am currently a student with a strong interest in open-source development, databases, and backend engineering. I have experience working with *Python, JavaScript, MySQL, MongoDB, and full-stack development*. I recently registered for *Google Summer of Code (GSoC) 2025* with MariaDB and am eager to contribute to the project. I am still exploring the available project ideas and would appreciate guidance on how to choose the best fit based on my skills. It would be great if mentors or experienced contributors could suggest beginner-friendly issues or key areas to focus on while getting familiar with the MariaDB codebase. Looking forward to learning, collaborating, and making meaningful contributions to the community! Best regards, *Naveen Kumar*

2 1

MariaDB 11.8.1 in Debian
by Otto Kekäläinen 23 Mar '25

23 Mar '25

Hi, I just wanted to let you know that MariaDB 11.8.1 just landed in Debian testing, which will be the upcoming Debian 13 "Trixie" release. I will continue with various quality assurance tasks so that eventually when 11.8.2 comes out it will be as polished as possible from Debian/Ubuntu perspective. Hopefully .2 does not have any big changes, but just bug fixes.

2 1

Prepared statements analytics problem
by Dmitrii Kochetov 19 Mar '25

19 Mar '25

Most database monitoring systems, such as Newrelic, Datadog, Dolphie, Ottertune, Releem, Solarwinds, use data from the performance_schema database from the events_statements_summary_by_digest and events_statements_* tables to get query analytics for databases. This information allows to calculate system performance metrics, in particular Latency. Also, the data from these tables allows you to get a complete picture of the database operation - what queries are executed, their execution statistics, to get query templates based on digest_text, and based on SQL_TEXT field from events_statements_* or QUERY_SAMPLE_TEXT from events_statements_summary_by_digest (for MySQL >= 8) to get a sample query for digest and optimize it. The prepared statements analytics are contained in a single table prepared_statements_instances, which is managed by Performance_schema as follows: - Statement preparation If the statement is successfully instrumented, a new row is added to the prepared_statements_instances table. If the statement cannot be instrumented, Performance_schema_prepared_statements_lost status variable is incremented. - Prepared statement execution Execution of a EXECUTE command for an instrumented prepared statement instance updates the corresponding prepared_statements_instances table row. - Prepared statement deallocation Execution of a CLOSE command for an instrumented prepared statement instance removes the corresponding prepared_statements_instances table row. To avoid resource leaks, removal occurs even if the prepared statement instruments described previously are disabled. In this regard, there are the following problems with analytics for prepared statements compared to regular queries: 1) Most applications work with prepared statements using the following method when processing a single query - creating a prepared statement, executing the prepared statement, and then dropping the prepared statement. This leads to the fact that query statistics exist for a very short period of time, during which it may not be possible to fetch them. 2) The second problem of prepared statement analytics in comparison with regular queries is the absence of the query hash (digest) and information about the schema_name in which the prepared statement is executed, which makes it difficult to identify unique queries, since a separate record in the Prepared_Statements_instances table is created for each creation of a prepared statement. 3) Execution of prepared statements is possible in two protocols: Binary and text. Execution of a prepared statement in the binary protocol does not create an entry in the events_statements_* tables, but execution in the text protocol creates an entry in the specified tables, which leads to different behaviors of the same prepared statement but in different protocols. 4) For prepared statements, there is no option in getting an example request without a ? placeholder. The SQL_TEXT field in Prepared_Statements_instances contains The prepared statement text, with ? placeholder markers, and in some cases (when executing in the text protocol) it is possible to find only the execute command of this statement, which further requires mapping ? placeholder markers with parameters from the execute request. But there are limitations that the binary protocol does not store EXECUTE requests for prepared statements, and most of the interactions with the database occur through it, through drivers for different programming languages, and another limitation may be the use of variables in the EXECUTE command, the value of which still needs to be obtained. Proposed solution: 1) Records should not be deleted from the Prepared_Statements_instances table after dropping prepared statements. If the table is full, the counter Performance_schema_prepared_statements_lost and If no row has the statement digest value for the statement that just completed, and the table is full, the statistics for the statement that just completed are added to a special “catch-all” row with DIGEST = NULL, which is created if necessary. 2) Each statement must be unique by the fields schema_name, in which the given statement is executed and its hash, digest, which will be the same for the same prepared statement. 3) In the tables events_statements_* must be recorded information about the fact of PREPARE, EXECUTE and DEALLOCATE execution for prepared statement, where in the DIGEST_TEXT field is stored the call of the execute command, and in SQL_TEXT - the normalized prepared query, in which ? placeholder markers are replaced by concrete values.

1 1

MariaDB 11.8(.1) to Debian
by Otto Kekäläinen 15 Mar '25

15 Mar '25

Hi, Just FYI, I am done with the February maintenance releases to Debian and Ubuntu, and this weekend started working on importing MairaDB 11.8(.1) to Debian. Draft is visible at https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/107

1 2

Using GitHub directly from inside Emacs
by Otto Kekäläinen 15 Mar '25

15 Mar '25

Hi! I don't use Emacs myself, but I learned last weekend at FOSDEM that some Emacs users love using the Emacs extension Magit [1] with the Forge [2] feature to list and review GitHub Pull Requests directly from Emacs without opening a browser. Just curious to know if any Emacs users here (Monty?) have tried Magit for interacting with https://github.com/MariaDB/server/pulls? There are currently 240 open PRs and 157 have zero reviews [3] from people with GitHub permissions to give reviews. To get more contributors, we need to have more people participating in reviewing/approving incoming contributions. Also some parts of the contributions process are not very optimal, and the process is likely to become smoother only if enough of current core developers at least occasionally "eat their own cooking" and experience (at least parts) of the contribution process. - Otto [1] https://magit.vc/ [2] https://magit.vc/manual/forge.html [3] https://github.com/MariaDB/server/pulls?q=is%3Apr+is%3Aopen+review%3Anone

2 3

Review of MDEV-35915 Implement Global temporary tables, part 2
by Michael Widenius 12 Mar '25

12 Mar '25

Review of Global temporary tables, part 2 commit a9478ca44b44118cf70de493569d64afbb5cd925 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Fri Nov 22 02:15:56 2024 +0100 Fix deallocation of Sql_cmd_dml::result Made as a part of MDEV-34309 Sql_cmd_dml::result is using mem_root to control memory allocation, but Sql_alloc allows to mark the memory as freed via destructor. Normal runs have DTRACE disabled, so accesses in MYSQL_DML_DONE are turned into no-op, and that is the bug is not shown. Enabling DTRACE brings a crash yet in a bootstrap. The fix is to carefully set Sql_cmd_dml::result to nullptr whenever it is freed, or is marked as freed. Please include the title of MDEV-354309 in the commit description @@ -34253,6 +34253,7 @@ bool Sql_cmd_dml::prepare(THD *thd) MYSQL_DML_START(thd); + result= nullptr; // It could've been set since the last run. lex->context_analysis_only|= CONTEXT_ANALYSIS_ONLY_DERIVED; if (open_tables_for_query(thd, lex->query_tables, &table_count, 0, How +++ b/sql/sql_delete.cc @@ -1882,6 +1882,7 @@ bool Sql_cmd_delete::execute_inner(THD *thd) { select_result *sel_result= NULL; delete result; + result= nullptr; /* This is DELETE ... RETURNING. It will return output to the client */ if (thd->lex->analyze_stmt) { @@ -1929,10 +1930,7 @@ bool Sql_cmd_delete::execute_inner(THD *thd) } if (result) - { res= false; - delete result; - } Why remove the delete. If the delete may free wrong things, then the newly added result is also likely to free wrong things. Anyway, the code is clearly wrong here. One cannot set res to 0 if res is 1. This means we are ignoring a possible error from the upper level (probably OOM) and we are probably going to assert later in the code that checks if we can send ok. Try to remove the seteting of res= false and run mtr. Same logics goes for all changes in Sql_cmd_update::execute_inner(THD *thd diff --git a/sql/sql_update.cc b/sql/sql_update.cc if (unlikely(res)) + if (unlikely(res) && result) result->abort_result_set(); else { The above is wrong. Should be: + if (unlikely(res)) { if (result) result->abort_result_set(); } else { As otherwise you may call send_explain() when you shouldn't ------------------ Suggestion: - Move delete of result to unprepare. This will ensure that result is always 0 on Sql_cmd_dml::prepare() and excute_innert and you can replace delete result with 'DBUG_ASSERT(result == 0)' in most of your changes. - Remove setting res= 0 if we have a result and check with mtr. Conclusion: - As this is not cricial and you are going on vacation, I will take over this issue --------- commit 6a000835f5f58aba221ab89c120adae4be16abf3 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Thu Feb 20 16:12:37 2025 +0100 fix show create diff --git a/sql/sql_show.cc b/sql/sql_show.cc index da0081fd1f8..201aa8e3110 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -1413,8 +1413,13 @@ mysqld_show_create(THD *thd, TABLE_LIST *table_list) if (table_list->table && table_list->table->s->tmp_table != NO_TMP_TABLE) { TMP_TABLE_SHARE *share= (TMP_TABLE_SHARE*)table_list->table->s; - if (share->from_share && table_list->table->file->records() == 0) - thd->drop_temporary_table(table_list->table, NULL, true); + if (share->from_share) + { + if (table_list->table->file->info(HA_STATUS_VARIABLE) + || (table_list->table->file->records() == 0 + && thd->drop_temporary_table(table_list->table, NULL, true))) + error= TRUE; + } Please move || and && to previous lines so we have the test lined up nicely in the editor. This is how most of the MariaDB code is written. commit 55ff0fb18bbe4539db27ea73cc2b795e53521772 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Thu Feb 20 16:12:49 2025 +0100 add rpl assertions if (thd->temporary_tables - && thd->temporary_tables->global_temporary_tables_count) // TODO this may be buggy on slave + && thd->temporary_tables->global_temporary_tables_count) { move && up to previous line. @@ -6051,6 +6053,7 @@ my_bool open_global_temporary_table(THD *thd, TABLE_SHARE *source, if (thd->mdl_context.clone_ticket(&share->mdl_request)) return TRUE; table->reginfo.lock_type= TL_IGNORE; + share->table_creation_was_logged= 1; } The above means that we assume that slave have the same global temporary table, which is probably not the case. Better to set share->table_creation_was_logged=0 for the local temporary tables we are creating which means that any update of the table through the GTT will use row based replication. Please also add a function comment before my_bool open_global_temporary_table(THD *thd, TABLE_SHARE *source, TABLE_LIST *out_table, MDL_ticket *mdl_ticket) commit a64bbaf483ad714493db401f68049ce85a886c96 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Thu Feb 27 01:07:45 2025 +0100 CREATE: use frm header to store GTT bits diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc index 45bca833d17..ea46c4035ba 100644 --- a/sql/temporary_tables.cc +++ b/sql/temporary_tables.cc @@ -1324,10 +1324,10 @@ int THD::commit_global_tmp_tables() All_share_tables_list::Iterator tables_it(share->all_tmp_tables); TABLE *table= tables_it++; DBUG_ASSERT(table); - if (!share->on_commit_delete) + if (!share->on_commit_delete()) table->file->info(HA_STATUS_VARIABLE); // update records() stat - if (share->on_commit_delete || table->file->records() == 0) + if (share->on_commit_delete() || table->file->records() == 0) error= drop_tmp_table_share(share, NULL, true); } I still think it would be better to not drop the temporary file based on file->records(). The reason is that some temporary table will be slow to drop and if there are users that is using these for many queries things will be slower than they need to be. - Not critical, can be discussed and changed later. commit 21f9ac1e56d6c81a431e5bdbc9b6b89c8b565ca2 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Thu Feb 27 01:12:34 2025 +0100 minor (typo, style) fixes -struct Closefrm_context +static my_bool open_gtt_on_error(TABLE *table) { - TABLE *table; - ~Closefrm_context() { closefrm(table); } -}; + closefrm(table); + return TRUE; +} Change return to void as this should never fail. Yes, it will require a few changes in callers, but it better to do return TRUE; in the caller than have the reviewer have to look at what another function is returning. +++ b/sql/temporary_tables.cc @@ -329,7 +329,9 @@ bool THD::internal_open_temporary_table(TABLE_LIST *tl, TABLE **table, { if (tmp_share->from_share && sql_command_flags() & (CF_ADMIN_COMMAND |CF_SCHEMA_CHANGE)) - DBUG_RETURN(false); + DBUG_RETURN(false); /* We want to use real global temporary table + when ALTER/DROP is executed. + */ Move the comment before the DBUG_RETURN() commit 9e1f5413f619bf4f88eef0f0a50fb7df4b8e7d44 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Sat Mar 1 01:27:04 2025 +0100 fix ALTER TABLE long wait @@ -11676,7 +11677,7 @@ do_continue:; */ if (alter_info->requested_lock != Alter_info::ALTER_TABLE_LOCK_EXCLUSIVE && thd->mdl_context.upgrade_shared_lock(mdl_ticket, MDL_SHARED_NO_WRITE, - lock_wait_timeout)) + thd->variables.lock_wait_timeout)) goto err_new_table_cleanup; Add a comment why we should not use lock_wait_timeout here. This is needed as we are using it a few lines above and it is important for coders to know why the value is not reused here. commit f7629d745fe729ea445688f5b710a97108da7c76 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Sat Mar 1 02:09:20 2025 +0100 Don't drop local data tables for ON COMMIT PRESERVE when records()==0 connect con1,localhost,root,,; +insert t values(1); +connection default; +alter table t add j int; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +set statement lock_wait_timeout=0 for drop table t; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction We have to add better error messages for known cases. "Cannot alter global temporary tables as we have open instances of it" "Cannot alter global temporary tables as there are other users for it" Can be done after vacation. diff --git a/mysql-test/main/global_temporary_table.test b/mysql-test/main/global_temporary_table.test index 0edc6eee493..17195eaf3ea 100644 --- a/mysql-test/main/global_temporary_table.test +++ b/mysql-test/main/global_temporary_table.test +--} insert into t1 values(1, 'one'); + select * from t1; No reason for having an extra empty line here. This commit is using: bool THD::use_real_global_temporary_share() const { return (sql_command_flags() & (CF_ALTER_TABLE | CF_SCHEMA_CHANGE | CF_STATUS_COMMAND) && lex->sql_command != SQLCOM_CREATE_TABLE) || lex->sql_command == SQLCOM_CREATE_VIEW; } Please add a comment and move |, && and || to the end of lines. Please also fix the indentation. You can also add a new flag CF_USE_LOCAL_GTT table and mark those commands for which using GTT will open a local GTT copy. In sql_show.cc:get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) there is this code: if (thd->use_real_global_temporary_share()) continue; This looks wrong. We are ignoring all temporary tables if the above case is true. To ignore GTT ables we have to verify that the found share is a GTT table. Shouldn't we also test for tmp_share->from_share ? Why are we testing for use_real_global_temporary_share() in this code? This is called only be DML. Shouldn't that be safe for GTT tables? For what commands are thd->use_real_global_temporary_share() not true here? commit 718afd78dae4a37ed009ab349dfe9bca68fbc3d7 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Tue Mar 4 18:30:07 2025 +0100 Extract THD::has_open_global_temporary_tables() No comments commit cc35b9cb54e74a2f0fbbf4a2400599c7fddb750d Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Tue Mar 4 21:01:07 2025 +0100 Add CF_ALTER_TABLE No comments commit 381243f4e75f61a31305c656f9b6be20cec042d2 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Wed Mar 5 00:18:42 2025 +0100 add global_temporary_tables plugin, move relevant code to temporary_tables.cc + MariaDB_PLUGIN_MATURITY_STABLE Should probably not be stable ;) Please change to BETA. We just have to ensure that this does not cause problems in Jenkin. If it does, you can change it to GAMMA or STABLE (depending on what Jenkins supports) commit a7fa6f835998bc33f1528420df5fe3f1b8a5dbfd Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Wed Mar 5 00:50:50 2025 +0100 refactor drop_tmp_table_share + while ((tab= it++)) { + if (tab != table && tab->query_id != 0) + { + /* Found a table instance in use. This table cannot be dropped. */ + my_error(ER_CANT_REOPEN_TABLE, MYF(0), table->alias.c_ptr()); + result= true; + goto end; + } Suggested comment change: /* Found a table instance in use. This table cannot be dropped. */ -> /* Found a table instance in use (query_id != 0). This table cannot be dropped. */ ------- commit bb9ff1b2ec45b437bd39b6dacef8885fadeb818b Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Wed Mar 5 02:51:23 2025 +0100 refactor THD::internal_open_temporary_table No comments commit 72914bfd92ae7390f8e7924944c92a1dc9591e1a Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Wed Mar 5 22:06:02 2025 +0100 enforce limitations + if (create_info->table_options & HA_OPTION_GLOBAL_TEMPORARY_TABLE) + { + const char *error= NULL; + for (const Key &fk: create_info->alter_info->key_list) + if (fk.type == Key::FOREIGN_KEY) + { + error= "FOREIGN KEY"; + break; + } You can replace the above loop with: if (thd->lex->alter_info_flags & ALTER_ADD_FOREIGN_KEY) Note also that in your old code, later errors could overwrite earlier ones. Changing this to a normal if, like the others, will solve that. commit a2831f8e8efcfecd548b8ced6e19c3fc4d6966f9 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Thu Mar 6 20:55:00 2025 +0100 information_schema support mysql-test/main/global_temporary_table.result +GLOBAL TEMPORARY +show table status where temporary='Y'; +Name Engine Version Row_format Rows Avg_row_length Data_length Max_data_length Index_length Data_free Auto_increment Create_time Update_time Check_time Collation Checksum Create_options Comment Max_index_length Temporary Please add a test that show the output from information_schema.tables for all fields that are stable for mtr. By the way, why does the above show table status not show any data while +select TABLE_TYPE from information_schema.tables where table_name = 't1'; +TABLE_TYPE +TEMPORARY +GLOBAL TEMPORARY Show two rows just above show table status; At least 'TEMPORARY' should have 'Temporary' set to 'Y'' commit a580c6bc85791829c850d7eedc03d62a5ee99ffd Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Thu Mar 6 23:43:38 2025 +0100 VIEW support Views already open the correct tables automatically, so it just works. Almost ;) + || lex->sql_command == SQLCOM_CREATE_VIEW ; .... commit d4c00419ebb39a07f87eabf0eee3660a768cbbed Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Fri Mar 7 02:16:23 2025 +0100 AS SELECT support + if (table->s->tmp_table && + table->s->db_create_options & + (HA_OPTION_GLOBAL_TEMPORARY_TABLE|HA_OPTION_ON_COMMIT_DELETE_ROWS)) + table= NULL; // It will be deleted on commit + This will not work because of the following code: if (WSREP(thd) && table->file->ht->db_type == DB_TYPE_INNODB) You either have to add a goto or fix all references to 'table'. Note this code later in the same function. if (m_plock) { MYSQL_LOCK *lock= *m_plock; *m_plock= NULL; m_plock= NULL; if (create_info->pos_in_locked_tables) { /* If we are under lock tables, we have created a table that was originally locked. We should add back the lock to ensure that all tables in the thd->open_list are locked! */ table->mdl_ticket= create_info->mdl_ticket; This should be safe as long as on cannot use LOCK TABLES on gtt tables. Please add a check for that! Another thing to check is with GALERA that local gtt tables are not replicated. commit 45e2f14bcbdd61c1ea5712f5b79b07a096aa46d2 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Mon Mar 10 20:16:16 2025 +0100 CREATE TABLE ... LIKE support sql_table.cc: local_create_info.table_options&= ~HA_OPTIONS_TO_RESET_CREATE_LIKE; Looking at the code, I belive the above is always 0 here. Please replace with DBUG_ASSERT(local_create_info.table_options == 0); commit 1c44e4dabe22555a3b517db7f67c1e4d8b03fc70 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Mon Mar 10 21:49:11 2025 +0100 RENAME support +++ b/sql/sql_parse.cc @@ -6351,6 +6351,13 @@ check_rename_table(THD *thd, TABLE_LIST *first_table, check_grant(thd, INSERT_ACL | CREATE_ACL, &new_list, FALSE, 1, FALSE))) return 1; + + if (table->table && + table->table->s->db_create_options & HA_OPTION_GLOBAL_TEMPORARY_TABLE) + { + my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0)); + return 1; + } We need a better error here. Something like: "Cannot rename a global temporary tables as this connection has open instances of it" commit b0f02644c22522c325bfbddd847030d61237b4e8 Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Tue Mar 11 00:48:11 2025 +0100 FLUSH support, forbid LOCK FLUSH TABLE on Global temporary table is basically no-op, since it never has open instances. - It is ok to ignore flush table on GTT tables. There is no point in doing that. Same goes for temporary tables. +/** + Check if any global temporary tables are still opened in this session. +*/ +static int flush_check_for_global_temporary_tables(THD *thd, TABLE_LIST *tl) +{ + if (thd->rgi_slave || !thd->has_open_global_temporary_tables()) + return 0; + + for (;tl; tl= tl->next_global) + { + TABLE_SHARE *share= thd->find_tmp_table_share(tl); + if (share && share->db_create_options & HA_OPTION_GLOBAL_TEMPORARY_TABLE) + { + my_error(ER_LOCK_WAIT_TIMEOUT, MYF(0)); + return 1; + } + } + return 0; +} I think you can remove the above. +flush table t; +lock table t write; +ERROR 42S02: Unknown table 't' I think it is wrong that we get an err for the lock. We should fix it so that we open the table local GTT table and do not take a lock. (In other words, it should work as normal lock tables for tmp tables). @@ -5970,6 +5970,12 @@ my_bool open_global_temporary_table(THD *thd, TABLE_SHARE *source, { DBUG_ASSERT(!thd->rgi_slave); // slave won't use global temporary tables + if (thd->lex->sql_command == SQLCOM_LOCK_TABLES) + { + my_error(ER_BAD_TABLE_ERROR, MYF(0), source->table_name.str); + return TRUE; + } + See above. We should remove the above and instead open the table normally. Author: Nikita Malyavin <nikitamalyavin(a)gmail.com> Date: Tue Mar 11 02:08:10 2025 +0100 Replication support Please do not use 'show binlog events' as this will cause result conflicts with any changes in the binlog format and server changes. You have now things like Server ver: 11.8.0-MariaDB-debug-log, Binlog ver: 4 in the test output use instead source include/show_binlog_events.inc; One can also does not need to use ../.. to access main tests. Here is a diff for the test: +++ b/mysql-test/suite/binlog/t/global_temporary_table.test @@ -4,7 +4,7 @@ set @old_binlog_annotate_row_events= @@global.binlog_annotate_row_events; set global binlog_annotate_row_events= OFF; set session binlog_annotate_row_events= OFF; -source ../../main/global_temporary_table.test; +source main/global_temporary_table.test; +source include/show_binlog_events.inc; -show binlog events; set global binlog_annotate_row_events= @old_binlog_annotate_row_events; ----- +bool is_user_tmp_table(TMP_TABLE_SHARE *share) +{ + if (share->db_create_options & HA_OPTION_GLOBAL_TEMPORARY_TABLE) + return false; + return share->tmp_table == TRANSACTIONAL_TMP_TABLE || + share->tmp_table == NON_TRANSACTIONAL_TMP_TABLE; +} The normal way to test for temporary table is: share->tmp_table != NO_TMP_TABLE; see is_temporary_table() This should be safe also for the above function. Add () around the return functions, to get editors to indent it correctly by default. I think the above function needs some comments. Please explain how things are: if (share->db_create_options & HA_OPTION_GLOBAL_TEMPORARY_TABLE) is true, this is a GTT definition table and tmp_table would be NO_TMP_TABLE. If this is a local GTT tables, then (share->db_create_options & HA_OPTION_GLOBAL_TEMPORARY_TABLE) is false and share->from_share is true. It would be good if the above function would have good enough comments to explain all the above. We need to describe exactly how things are in some please in the source. Could you add the description how to detect GTT tables and local GTT tables and how they differ from normal temporary tables at the start of temporary_tables.cc Especially how share->db_create_options and share->tmp_table are set. If you add this function first in tempoary_table.cc, it may explain how things are: bool is_user_tmp_table(TMP_TABLE_SHARE *share) { if (share->db_create_options & HA_OPTION_GLOBAL_TEMPORARY_TABLE) { /* This is either a global temporary table (GTT) definition table or local GTT table created from the GTT definition. If (share->tmp_table != NO_TMP_TABLE) then this is local GTT table. In this case share->from_share is also set and points to the global GTT definition. */ return false; } /* Return true of the table is non GTT temporary table */ return (share->tmp_table != NO_TMP_TABLE) }

1 0

Re: [PATCH] Fix redundant ER_PRIOR_COMMIT_FAILED in parallel replication
by Kristian Nielsen 11 Mar '25

11 Mar '25

Hi Monty, thanks for review! Michael Widenius <michael.widenius(a)gmail.com> writes: > Review of patch: > > wait_for_prior_commit() can be called multiple times per event group, > only do my_error() the first time the call fails. > > > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -8365,6 +8365,18 @@ wait_for_commit::wait_for_prior_commit2(THD > *thd, bool allow_kill) > } > > > +void > +wait_for_commit::prior_commit_error(THD *thd) > +{ > + /* > + Only raise a "prior commit failed" error if we didn't already raise > + an error. > + */ > + if (!thd->get_stmt_da()->is_set()) > + my_error(ER_PRIOR_COMMIT_FAILED, MYF(0)); > +} > + > + > /* > Wakeup anyone waiting for us to have committed. > > diff --git a/sql/sql_class.h b/sql/sql_class.h > index 4cee31b296e..69b021cd41d 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -2398,8 +2398,8 @@ struct wait_for_commit > return wait_for_prior_commit2(thd, allow_kill); > else > { > - if (wakeup_error) > - my_error(ER_PRIOR_COMMIT_FAILED, MYF(0)); > + if (unlikely(wakeup_error)) > + prior_commit_error(thd); > return wakeup_error; > } > } > The only small objection I have to this patch is that if we get first > one error and then a wait_for_prior_commit error, then we will not get > the second error. > I would suggest that instead of checking for if > (!thd->get_stmt_da()->is_set()) instead add a new flag > 'got_prior_commit_error' to check if we have already > got a prior commit error for this thread. > Adding bool get_prior_commit_error into THD after bool > wakeup_blocked; has no storage cost. It is actually deliberate to skip the ER_PRIOR_COMMIT_FAILED if we already gave a different error. The reason is that the ER_PRIOR_COMMIT_FAILED is not a real error - it is an internal error thrown only to make sure that a following transaction does not succeed after a prior transaction failed, which would result in inconsistent commit order. Therefore, if a worker thread already got a real error, for example duplicate key or row not found, then it seems preferable to keep that error rather than override it with a default, internal-only error. (Arguably it might make sense to not write ER_PRIOR_COMMIT_FAILED errors to the error log at all, as they do not seem very useful; they are not an error that the user should do anything about, on the contrary they indicate that the worker correctly handled an earlier error. But that would be a separate patch, probably). > It up to you to decide which solution to choose. Ok, then I will keep this, but I will add a comment explaining why the original error is preferred over ER_PRIOR_COMMIT_FAILED. > This should be pushed to 10.6 to avoid the 63*4 warnings we can get > with parallel=4. Ok, will do then. > I will try to spend some time today to try to simulate the case where > we get new calls to wait_for_prior_commit after signaling a previous > failure from it. I hope you find a way to reproduce, I did not manage to do so when I tried. It would be good to find the root cause of those repeated ER_PRIOR_COMMIT_FAILED. - Kristian.

1 0

Re: 0cc1c382dfc: MDEV-34712 Add support to sha2 and pbkdf2 key derivation in file_key_management
by Sergei Golubchik 10 Mar '25

10 Mar '25

Hi, Yuchen, On Mar 10, Yuchen Pei wrote: > revision-id: 0cc1c382dfc (mariadb-11.8.1-9-g0cc1c382dfc) > parent(s): 3a81664cb80 > author: Yuchen Pei > committer: Yuchen Pei > timestamp: 2025-03-05 13:33:53 +1100 > message: > > MDEV-34712 Add support to sha2 and pbkdf2 key derivation in file_key_management > > Add two new variables: > > file_key_management_digest={sha1|sha224|sha256|sha384|sha512} > > This specify the digest function to use in key derivation of the key > used for decryption of the keyfile > > file_key_management_use_pbkdf2=N > > This specify whether pbkdf2 is used in the key derivation, and if > so (N != 0), how many thousands iterations. > diff --git a/include/mysql/service_my_crypt.h b/include/mysql/service_my_crypt.h > index 1a4ebcfd067..3a58d0828fc 100644 > --- a/include/mysql/service_my_crypt.h > +++ b/include/mysql/service_my_crypt.h > @@ -70,6 +73,10 @@ extern struct my_crypt_service_st { > unsigned int (*my_aes_get_size)(enum my_aes_mode mode, unsigned int source_length); > unsigned int (*my_aes_ctx_size)(enum my_aes_mode mode); > int (*my_random_bytes)(unsigned char* buf, int num); > + void (*my_bytes_to_key)(const unsigned char *salt, const unsigned char *input, > + unsigned int input_len, unsigned char *key, > + unsigned char *iv, enum my_digest digest, > + unsigned int use_pbkdf2); backward compatible API change - you need to increment the service version > } *my_crypt_service; > > #ifdef MYSQL_DYNAMIC_PLUGIN > diff --git a/mysql-test/suite/encryption/t/filekeys_encfile.test b/mysql-test/suite/encryption/t/filekeys_encfile.test > index a0611a38ec4..89e13834a79 100644 > --- a/mysql-test/suite/encryption/t/filekeys_encfile.test > +++ b/mysql-test/suite/encryption/t/filekeys_encfile.test > @@ -1 +1,34 @@ > +# # The encrypted key files were generated using the following: > + > +# let $plain_keyfile=$MYSQL_TMP_DIR/keys.txt; > +# let $enc_keyfile_prefix=$MTR_SUITE_DIR/t/filekeys-data; > + > +# write_file $plain_keyfile; > +# # > +# # this is a comment > +# # > +# 1;770A8A65DA156D24EE2A093277530142 > +# 2;4D92199549E0F2EF009B4160F3582E5528A11A45017F3EF8 > +# # another comment > +# 33;B374A26A71490437AA024E4FADD5B497FDFF1A8EA6FF12F6FB65AF2720B59CCF > +# 4;18420B5CBA31CCDFFE9716E91EB61374D05914F3ADE23E03 --> ignored > + > +# 5;966050D7777350B6FD5CCB3E5F648DA45C63BEFB6DEDDFA13443F156B7D35C84 > +# 6;B5EA210C8C09EF20DB95EC584714A89F # and yet another > + > +# EOF > + > +# exec openssl enc -aes-256-cbc -md sha1 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha1.enc; > +# exec openssl enc -aes-256-cbc -md sha224 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha224.enc; > +# exec openssl enc -aes-256-cbc -md sha256 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha256.enc; > +# exec openssl enc -aes-256-cbc -md sha384 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha384.enc; > +# exec openssl enc -aes-256-cbc -md sha512 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha512.enc; > +# exec openssl enc -aes-256-cbc -md sha1 -pbkdf2 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha1-pbkdf2.enc; better say -iter 1000 instead of -pbkdf2 (to match plugin parameters) And change some tests to use a non-default value, please > +# exec openssl enc -aes-256-cbc -md sha224 -pbkdf2 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha224-pbkdf2.enc; > +# exec openssl enc -aes-256-cbc -md sha256 -pbkdf2 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha256-pbkdf2.enc; > +# exec openssl enc -aes-256-cbc -md sha384 -pbkdf2 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha384-pbkdf2.enc; > +# exec openssl enc -aes-256-cbc -md sha512 -pbkdf2 -pass pass:secret -in $plain_keyfile -out $enc_keyfile_prefix-sha512-pbkdf2.enc; > + > +# echo Done; > + > source filekeys_goodtest.inc; > diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc > index 346eff51b1b..ef74121d530 100644 > --- a/mysys_ssl/my_crypt.cc > +++ b/mysys_ssl/my_crypt.cc > @@ -382,4 +384,39 @@ int my_random_bytes(uchar *buf, int num) > return MY_AES_OK; > } > > +static inline const EVP_MD *get_digest(enum my_digest digest) > +{ > + switch (digest) > + { > + case MY_DIGEST_SHA1: return EVP_sha1(); > + case MY_DIGEST_SHA224: return EVP_sha224(); > + case MY_DIGEST_SHA256: return EVP_sha256(); > + case MY_DIGEST_SHA384: return EVP_sha384(); > + case MY_DIGEST_SHA512: return EVP_sha512(); > + default: > + { > + assert(0); > + return NULL; > + } > + } > +} > + > +void my_bytes_to_key(const unsigned char *salt, const unsigned char *input, > + uint input_len, unsigned char *key, unsigned char *iv, > + enum my_digest digest, uint use_pbkdf2) > +{ > + if (use_pbkdf2 == 0) > + EVP_BytesToKey(EVP_aes_256_cbc(), get_digest(digest), salt, > + input, input_len, 1, key, iv); > + else > + { > + uchar keyiv[KEY_LENGTH + IV_LENGTH]; > + PKCS5_PBKDF2_HMAC((const char*) input, input_len, salt, 8, > + use_pbkdf2 * 1000, get_digest(digest), I suggest not to do *1000. It doesn't help with anything and introduces a limitation that we don't need. Perhaps somebody has a policy of using 2^N iterations in PBKDF2? Also, `openssl enc -iter` doesn't do *1000 > + KEY_LENGTH + IV_LENGTH, keyiv); > + memcpy(key, keyiv, KEY_LENGTH); > + memcpy(iv, keyiv + KEY_LENGTH, IV_LENGTH); > + } > +} > + > } > diff --git a/plugin/file_key_management/file_key_management_plugin.cc b/plugin/file_key_management/file_key_management_plugin.cc > index 8702785a1fd..f37cfe05960 100644 > --- a/plugin/file_key_management/file_key_management_plugin.cc > +++ b/plugin/file_key_management/file_key_management_plugin.cc > @@ -54,10 +68,22 @@ static MYSQL_SYSVAR_ENUM(encryption_algorithm, encryption_algorithm, > "Encryption algorithm to use" recommendation, > NULL, NULL, 0, &encryption_algorithm_typelib); > > +static MYSQL_SYSVAR_ENUM(digest, digest, > + PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY, > + "Digest to use", "Digest algorithm used for decrypting the key file. The value of the openssl enc -md option" > + NULL, NULL, 0, &digest_typelib); > + > +static MYSQL_SYSVAR_UINT(use_pbkdf2, use_pbkdf2, > + PLUGIN_VAR_RQCMDARG | PLUGIN_VAR_READONLY, > + "Use pbkdf2", "Number of PBKDF2 iteration used for decrypting the key file. Use 0 to disable PBKDF2, otherwise it's a value of the openssl enc -iter option" > + NULL, NULL, 0, 0, UINT_MAX / 1000, 0); > + > static struct st_mysql_sys_var* settings[] = { > MYSQL_SYSVAR(filename), > MYSQL_SYSVAR(filekey), > MYSQL_SYSVAR(encryption_algorithm), > + MYSQL_SYSVAR(digest), > + MYSQL_SYSVAR(use_pbkdf2), > NULL > }; Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Has anyone implemented InnoDB redo log-based physical replication?
by Otto Kekäläinen 04 Mar '25

04 Mar '25

Hi, Has anyone attempted or seen any existing implementations "physical" replication between two MariaDB instances, where a secondary read-only instance would follow the primary by directly reading and applying page changes from the primary's redo logs? The secondary instance would start (and restart if necessary) using a full copy of the data from the primary instance, but while running keep itself current by reading redo logs from the primary instance, and apply pages based on the increasing LSN number. I call this "physical" replication to distinguish it from normal logical replication based on e.g. binlogs, and this would obviously only work for InnoDB. Thanks, Otto

2 2

Review request for MariaDB 2025 Q1 uploads to Debian and Ubuntu
by Otto Kekäläinen 24 Feb '25

24 Feb '25

Hi! Thanks Vicentiu for reviewing https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/102 The next uploads in line are ready for review at: https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/98 Prepare MariaDB Server 1:10.11.11-0+deb12u1 minor maintenance release for Debian stable (Bookworm) https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/106 Prepare upload to MariaDB 10.11.9 to Ubuntu 24.04 "Noble" https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/96 Prepare upload to MariaDB 10.6.19 to Ubuntu 22.04 "Jammy" https://salsa.debian.org/mariadb-team/mariadb-10.5/-/merge_requests/20 New upstream version MariaDB 10.5.28 for Debian 11 "Bullseye" Once I am done with these, I will start testing MariaDB 11.8 with the goal of uploading it into Debian in the coming weeks.

1 0

Re: [PATCH] MDEV-36099 Ensure that creation and usage of temporary tables in replication is predictable
by Kristian Nielsen 23 Feb '25

23 Feb '25

Hi Monty, > From: Monty <monty(a)mariadb.org> > > The purpose of this commit is to ensure that creation and changes of > temporary tables are properly and predicable logged to the binary > log. It also fixes some bugs where ROW logging was used in MIXED mode, > when STATEMENT would be a better (and expected) choice. Here is my review of the patch for MDEV-36099 predictable binlog format for temporary tables: Overall this seems like solid work, thanks! As Elena also remarked, this does significant changes to a very tricky and bug-ridden area of replication, that of temporary tables, this is scary stuff. But the patch does look good and relatively simple. A main change with this patch is that now all use of temporary tables will by default be binlogged in row mode. I think this is a reasonable change. There are still remaining problems with temporary tables in statement-based mode; and it also helps with parallel replication, which cannot run in parallel statement-logged temporary tables. And you provided --create-temporary-table-binlog-formats as a way to enable backwards-compatible behaviour. I think it is important to mention this prominently in the release notes. There will invariably be some users that manipulate a large number of rows with temporary tables, eg. BEGIN; CREATE TEMPORARY TABLE tmp AS SELECT a, b, c FROM large_table1 JOIN large_table2 ON ... INSERT INTO other_table SELECT b, c FROM tmp WHERE a <100; DROP TEMPORARY TABLE tmp; COMMIT; This will change from binlogging a few hundred bytes of Query events to potentially many megabytes of row events, which could cause problems in some cases. I think the change is still justified from the benefits it gives and the relative few cases where this will cause serious problems, but it is still important to try to document it as well as possible. I read the code part of the patch through in detail, but I did not spot anything in it that looks wrong. There may be things missing from the patch that are hard to spot in a review, I think that's unavoidable in a change like this. But I did try to think of possible issues without finding anything, and the patch does look quite simple compared to what it is doing. So looks good I think. Just a few questions on details I did not understand, and some minor suggestions for changes. Questions: > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index c2f842aea56..3d1df5ac42f 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -531,6 +531,7 @@ void init_update_queries(void) > CF_SCHEMA_CHANGE; > sql_command_flags[SQLCOM_CREATE_SEQUENCE]= (CF_CHANGES_DATA | > CF_REEXECUTION_FRAGILE | > + CF_FORCE_ORIGINAL_BINLOG_FORMAT | > CF_AUTO_COMMIT_TRANS | > CF_SCHEMA_CHANGE); I did not get the reason for this change. I assume some bug fix found during test case writing? > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -817,6 +817,7 @@ typedef struct system_variables > /* Flags for slow log filtering */ > ulong log_slow_rate_limit; > ulong binlog_format; ///< binlog format for this thd (see enum_binlog_format) > + ulong create_temporary_table_binlog_formats; Is there any way to make this take less space than 8 bytes (on 64-bit)? It goes into every THD, and only two bits are used. > + > + Temporary tables will be logged only on CREATE in STMT format > + or on INSERT if all changes to the table is in the binlog. > */ > if ((WSREP_EMULATE_BINLOG(thd) || mysql_bin_log.is_open()) && > - (likely(!error) || thd->transaction->stmt.modified_non_trans_table || > - thd->log_current_statement())) > + (table->s->using_binlog() || > + ((in_create_table && > + (!table->s->tmp_table || thd->binlog_create_tmp_table())))) && > + (likely(!error) || > + (!in_create_table && > + (thd->transaction->stmt.modified_non_trans_table || > + thd->log_current_statement())))) This conditional is now very hard to understand. I think I managed to understand all parts of it, except the part "thd->log_current_statement()", but that is unchanged with your patch. This if () would now benefit from being split into multiple parts, each of which explained in a comment. > diff --git a/mysql-test/main/statistics.result b/mysql-test/main/statistics.result > index bb6469c46da..ebd9cb7f36e 100644 > --- a/mysql-test/main/statistics.result > +++ b/mysql-test/main/statistics.result > @@ -1278,7 +1278,7 @@ Table Op Msg_type Msg_text > mysql.column_stats analyze error Invalid argument > ANALYZE TABLE mysql.column_stats; > Table Op Msg_type Msg_text > -mysql.column_stats analyze status OK > +mysql.column_stats analyze status Table is already up to date > SELECT * FROM mysql.table_stats; > db_name table_name cardinality > SELECT * FROM mysql.column_stats; I did not understand why this .result change happens after your patch? Minor things: > diff --git a/mysql-test/suite/rpl/t/create_or_replace.inc b/mysql-test/suite/rpl/t/create_or_replace.inc > index e8fa95cb94a..b4e91d7b412 100644 > --- a/mysql-test/suite/rpl/t/create_or_replace.inc > +++ b/mysql-test/suite/rpl/t/create_or_replace.inc > @@ -4,7 +4,24 @@ > --let $rpl_topology=1->2 > --source include/rpl_init.inc > > -# Create help tables > +select @@binlog_format, @@create_temporary_table_binlog_formats; > + > +# Copy create_temporary_table_binlog_formats from master to slave > +# This is done to get same results as with older versions of MariaDB. > +# The slave will work even if this is not done. However in mixed > +# format on slave temporary tables would not be logged to in the > +# slaves binary log > +let $format=`select @@create_temporary_table_binlog_formats`; > +connection server_2; > +--eval set @@global.create_temporary_table_binlog_formats='$format' > +# Ensure that the slave threads uses the new values. > +stop slave; > +start slave; Here, use --source include/stop_slave.inc and --source include/start_slave.inc. For consistent with other test cases. > - Rename of not binlogged temporary tables where binlogged to binary log > which caused replication to stop. s/where/were/ > + /* > + Mark if query was logged as statement. Used to check it tmp table changes > + are logged. > + */ > + bool query_binlogged_as_stmt; s/check it table/check if table/ + /* + This is set for all normal tables and for temporary tables that has + the CREATE binary logged. + + 0 table table is not in binary log (not logged temporary table) + 1 table create was logged (normal table or logged temp table) + 2 table create was logged but not all changes are in the binary log. + ROW LOGGING will be used for the table. s/that has the CREATE/that have the CREATE/ s/table table is not/table is not/ > @@ -794,6 +821,20 @@ void THD::mark_tmp_table_as_free_for_reuse(TABLE *table) > > DBUG_ASSERT(table->s->tmp_table); > > + /* > + Ensure that table changes where either binary logged or the table > + is marked as not up to date. > + */ s/where/were/ Thanks, Monty! Reviewed-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> - Kristian.

2 1

MDEV-33671: MTR Safe Process Hardcodes RLIMIT_NOFILE
by mohanadkhaled87＠gmail.com 11 Feb '25

11 Feb '25

Hello I am Mohanad and this is my first issue here I started working on this issue a while ago but only began testing it recently. As per the comment in this issue (https://jira.mariadb.org/browse/MDEV-33671?focusedCommentId=281856&page=com…) "You can run ulimit from Perl, between the fork and exec." I implemented this by using the BSD::Resource library, as it appears to be part of the core Perl libraries and does not introduce an external dependency and as a sanity check I added ```perl my $has_bsd_resource = eval { require BSD::Resource; BSD::Resource->import(qw(setrlimit getrlimit RLIMIT_NOFILE)); 1; }; ``` Would this be an okay to use ? Also, I have manually verified the implementation by printing the limit values. However, is it possible to add test files or automate testing for this issue in MTR ?

1 0

Current status on MDEV-34705 binlog-in-engine
by Kristian Nielsen 09 Feb '25

09 Feb '25

Hi Brandon (and other interested), It is great that you want to look closer at the MDEV-34705, binlog-in-engine project. I wanted to write up a high-level summary of the current status to help you get an overview. For reference, this is a project to implement a new binlog format that solves some of the complexity and performance problems with the old format that is not integrated with the storage engine transaction log (eg. InnoDB binlog). The code is being developed on the branch knielsen_binlog_in_engine on Github, https://github.com/MariaDB/server/commits/knielsen_binlog_in_engine There has been some previous design discussions on the mailing list, eg.: https://lists.mariadb.org/hyperkitty/list/developers@lists.mariadb.org/thre… https://lists.mariadb.org/hyperkitty/list/developers@lists.mariadb.org/thre… - Kristian. Current status: - Feature is off by default, enabled by --binlog-storage-engine=innodb. - Binlog files are now identified solely with their file_no, starting from 0 (no more master-bin.index). Binlog file names are hard-coded by the engine (binlog-NNNNNN.ibb), directory can be selected with --binlog-directory. - Binlog files are pre-allocated to --max-binlog-size, thus their size is fixed. Event groups and even events can span multiple binlog files. - New binlog format is page-based. On server start, binary search is used to find the current position (no need for scanning a whole binlog file). GTID start position is likewise found by binary search, no need for separate GTID index. - Binlog files are written strictly append-only, data is never updated after initial write. The InnoDB redo log is used to ensure that files are recovered into a consistent state with the engine data after a crash. - Large event groups (larger than --binlog-cache-size) are split into pieces that are binlogged "out-of-band", meaning they are written in separate records during transaction execution, before commit. The final commit record merely has a reference to the out-of-band data. Out-of-band data for rolled-back transactions remain as "garbage" until binlog purge. - The binlog dump thread re-assembles out-of-band data into consecutive event groups for the slaves. Thus, slaves see event data in the same format as before, no slave code changes needed. Out-of-band data is structured as complete binary trees for efficient reading. - Basic slave replication is working, not much is tested though. Currently GTID mode only, might make sense to not support filename/position mode for simplicity. - RESET MASTER, FLUSH BINARY LOGS, PURGE BINARY LOGS are implemented. - Initial benchmarking show good speedup (x2) in durable configuration, potentially huge (3 to sevelal hundred times faster) speedup in crash-safe configuration. A main challenge with this project is that the binlog internals are visible in so many places to users, and to other parts of the code. Here is an (incomplete) list of major issues still to implement: - The current implementation is using the normal InnoDB buffer pool and checkpoint mechanism. See the mailing list discussions for Marko's ideas for implementing a simpler integration with the InnoDB redo log. - When --sync-binlog=1, implement that binlog events are not sent to slaves until durably written to redo log. When --sync-binlog=0, implement that events are not sent until redo log is (non-durably) written (--innodb-flush-log-at-trx-commit=2). - mysqlbinlog ability to read new binlog format. The code is there, mostly a matter of finding a good way to share code between the server/InnoDB and mysqlbinlog.cc. - Disable all parts of the legacy binlog when --binlog-storage-engine is set (in current code some parts still remain active). - SHOW BINLOG EVENTS. - Binlog checksum and encryption. Will checksum and encrypt the individual pages, not use the legacy per-event checksums or encryption. - Storage engine API. The functionality is implemented, but the design of the API and interfaces might need review and cleanup, ie. maybe parts to be implemented as services (include/service_versions.h etc.) - Lots of smaller ToDo:s, some marked in comments in the code. - There is a --suite=binlog_in_engine, but a lot more testing is neeed. Some parts may be initially not supported in the first release, but implemented later: - Semi-synchronous replication. Should be straight-forward to implement later. Might make sense to use GTID to send back-and-forth instead of filename/offset. AFTER_SYNC no longer makes sense, but could consider implementing AFTER_PREPARE. - External XA, to be implemented following MDEV-32020. The out-of-band mechanism will be used so that XA COMMIT can reference the binlog data and replicate without the need for any transactions to be in XA PREPARED state on the slave or in backups.

1 0

Next step on MDEV-34705, implement binlog in InnoDB
by Kristian Nielsen 30 Jan '25

30 Jan '25

Hi Marko, Time flies, somehow it's already more than a year since our first discussions on implementing the binlog in InnoDB and avoiding the extra fsync() and complexity of the two-phase commit between InnoDB and binlog. But things have progressed, and I have now reached the point where most of the basic groundwork is implemented. Event groups are binlogged to InnoDB tablespaces. Binlog dump thread can read the binlog and send to slave, and replication is working. Large event groups are split into pieces, bounding the amount of data that needs to be atomically written in mini-transactions and at commit time. There are still many details left, but mostly in the server-layer replication code which should be manageable, just will take some time to get completed. I think now is a good time for you to take a first real look at the InnoDB part of the changes, I would really value your input. The main part of the InnoDB code is in two files: 1. handler/handler0binlog.cc for the high-level part that deals mostly with the new binlog file format and interfacing to the server layer. 2. fsp/fsp0binlog.cc for the low-level part that most tightly interacts with the InnoDB mini-transactions and buffer pool. The most interesting part for you to look at is fsp/fsp0binlog.cc (~1k lines), though I'm happy to hear comments on any part of the patch, of course. The code is pushed to GitHub in the branch knielsen_binlog_in_engine: https://github.com/MariaDB/server/commits/knielsen_binlog_in_engine and I've also attached the complete patch. This is my first major patch for InnoDB, so there will undoubtedly be a number of style changes required. But the overall structure of the code should now be close to what I imagine would be the final result, with some pending ToDo steps marked in comments in the code, and detailed in the below list, some of which we discussed a bit already. I hope you will take a look at the patch and let me know of any questions or other things you need from me. Maybe we can also find a chance to discuss further if you will come to FOSDEM start of February, or I could visit sometimes in Finland. - Kristian. ---- Some known outstanding issues in the InnoDB part: - We previously discussed removing some of the page header overhead for binlog tablespaces. Currently the code just leaves alone the first FIL_PAGE_DATA bytes (38) and the last FIL_PAGE_DATA_END (8 IIRC). - We discussed previously to write the current LSN at the start of the tablespace, and use this in recovery to handle that we have only two tablespace IDs that are reused. So we need code in recovery that checks the LSN at the start of the tablespace, and skips redo records with LSN smaller than this. - We want to avoid the double-write buffer for binlog pages, at least for the first page write (most pages will only be written as full pages). You mentioned an idea to completely avoid the double-write buffer and instead do some specific code for recovery in the uncommon case where a partial binlog page is written to disk due to low commit activity. - The flushing of binlog pages to disk currently happens in a dedicated thread in the background. I'd welcome ideas on how to do this differently. It is good to flush binlog pages quickly and re-use their buffer pool entries for something better. Also writing the pages to disk quickly (not necessarily fsync()'ing) makes the data readable by mysqlbinlog. - Checksum and encryption should use the standard InnoDB mechanism. I assume checksum is already handled in the code through using the buffer pool and mini-transactions to read/write pages. Not sure about encryption. I need to implement that the code handles checksum and decryption when reading the pages manually from the file (not through buffer pool).

2 10

2025 Q1 release - near final
by Daniel Black 29 Jan '25

29 Jan '25

The issue where this is tracked is: https://jira.mariadb.org/browse/MDEV-35789 Current blockers: https://jira.mariadb.org/issues/?filter=31019&jql=project%20in%20(mdev)%20A… The "fix version" in the above url list all are 11.4+ so all LTS versions should be stable. Branches bb-10.5-release (LTS) bb-10.6-release (LTS) bb-10.11-release (LTS) bb-11.4-release (LTS) bb-11.7-release (stable for first time) bb-11.8-release (gamma - no LTS announcement made) If there's regression particularly impacting please ensure the issue has a blocker priority, with a good business case reason why it should block, and make as blocks MDEV-35789. In addition to compiling the source yourselves: Containers: quay.ion/mariadb-foundation/mariadb-devel:{release} list: https://quay.io/repository/mariadb-foundation/mariadb-devel?tab=tags note: * pushes to *-release branch also gains the same tag as the main 10.X, 11.X tags. Check commit at the beginning of the server startup to determine actual commit. Repos for testing (packages may not last more than a few weeks at urls): https://ci.mariadb.org/ contains for deb and rpm distros a repo file in the form: bb-{version}-release-latest-{arch}-{distro}-{version}-{deb|rpm}-autobake.{sources|repo}

1 0

Parameterized queries
by E.S. Rosenberg 27 Jan '25

27 Jan '25

Hi, Recently I found out about parameterized queries in MySQL, it seems to have been supported at least since 8.0.x but it is not listed as an incompatibility between mariadb and mysql yet the package spec for COM_QUERY on mariadb does not mention any support for parameterized queries. So is this an incompatibility, some out of date docs or am I just looking at the wrong com_query page? Refs: https://dev.mysql.com/doc/dev/mysql-server/8.0.40/page_protocol_com_query.h… https://mariadb.com/kb/en/com_query/ Thanks! Eli

4 9

Re: 6a4b85ce1cc: MDEV-20281 "[ERROR] Failed to write to mysql.slow_log:" without error reason
by Sergei Golubchik 25 Jan '25

25 Jan '25

Hi, Oleksandr, Ok to push after fixing spagetty (either by fixing a typo or by changing the code pattern as suggested below). On Jan 25, Oleksandr Byelkin wrote: > revision-id: 6a4b85ce1cc (mariadb-10.5.27-202-g6a4b85ce1cc) > parent(s): d261fa5c703 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2025-01-24 17:24:17 +0100 > message: > > MDEV-20281 "[ERROR] Failed to write to mysql.slow_log:" without error reason > > Add "backup" (in case of absence issued by error) reasons for failed logging. > > @@ -998,6 +1047,9 @@ bool Log_to_csv_event_handler:: > close_log_table(thd, &open_tables_backup); > thd->time_zone_used= save_time_zone_used; > DBUG_RETURN(result); > +store_err: // spagetty, but reduce size of the code > + cause= "Can't write data (possible incorrect log table structure)"; > + goto err; > } typo. it's "spaghetti" but may be better to avoid it altogether by rewriting as cause= "..."; if (field[0]->store(...)) goto err; if (field[1]->store(...)) goto err; if (field[2]->store(...)) goto err; ... cause= NULL; Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Re: 0b403e03939: MDEV-35661 Assertion `!cache_list ||...' fails in online_alter_close_connection
by Sergei Golubchik 22 Jan '25

22 Jan '25

Hi, Nikita, On Jan 19, Nikita Malyavin wrote: > revision-id: 0b403e03939 (mariadb-11.7.1-16-g0b403e03939) > parent(s): 40e502eca4c > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2025-01-03 00:54:52 +0100 > message: > > MDEV-35661 Assertion `!cache_list ||...' fails in > online_alter_close_connection > > After MDEV-35182 (88adcbf3) fix, online_alter still wasn't fully ready > for `XA PREPARE` to be skipped. > > In fact, according to XA standard, even xa_commit() call may happen > without preceding xa_prepare(), so this stage is completely optional. > > This commit removes any relying on xa_prepare() call. The only thing > that will be done there is an equivalent of "statement-level commit", > which basically just flushes a pending rows event to an online alter > cache. > > This is done by always storing an online alter cache in the XA_data > object whenever XA is active, rather than in THD::ha_data. Why? I thought it'd be simpler to keep the cache in the THD and take it off it in commit or rollback. That is, almost as now, but commit/rollback won't rely on prepare being called before. Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Re: 33e36944232: MDEV-30111 InnoDB: Failing assertion: update->n_fields == 0 in row_ins_sec_index_entry_by_modify
by Sergei Golubchik 22 Jan '25

22 Jan '25

Hi, Alexander, On Jan 22, Alexander Barkov wrote: > revision-id: 33e36944232 (mariadb-10.6.20-19-g33e36944232) > parent(s): 25be7da2024 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2024-11-14 10:19:24 +0400 > message: > > MDEV-30111 InnoDB: Failing assertion: update->n_fields == 0 in row_ins_sec_index_entry_by_modify > > Also fixes: > > MDEV-32190 Index corruption with unique key and nopad collation (without DESC or HASH keys) > > The code in strings/strcoll.inl when comparing an empty string > to a string like 0x0001050001 did not take into account > that the leftmost weight in the latter can be zero, while > there are some more weights can follow the zero weight. > > Rewriting the code to treat the shorter string as smaller than > a longer string. > > --- a/strings/strcoll.inl > +++ b/strings/strcoll.inl > @@ -192,10 +192,10 @@ MY_FUNCTION_NAME(strnncoll)(CHARSET_INFO *cs __attribute__((unused)), > >0 >0 Two weights were scanned, check weight difference. > */ > if (!a_wlen) > - return b_wlen ? -b_weight : 0; > + return b_wlen ? -1 : 0; > > if (!b_wlen) > - return b_is_prefix ? 0 : a_weight; > + return b_is_prefix ? 0 : +1; > > if ((res= (a_weight - b_weight))) > return res; Sorry, I don't understand it. The function returns >0, 0, or <0. It shouldn't matter whether it returns -b_weight or -1, a_weight or +1. Except if the weight can be <= 0 itself. But in that case, if ((res= (a_weight - b_weight))) return res; is rather suspicious too. Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

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