- developers - lists.mariadb.org

MariaDB near 50% in WordPress usage stats
by Otto Kekäläinen 13 May '25

13 May '25

Hi all, Glad to see that MariaDB has now surpassed MySQL in WordPress usage stats at https://wordpress.org/about/stats/ Version Usage MySQL 8.0 20.77% MariaDB 10.6 19.88% MariaDB 10.11 18.60% MySQL 5.7 17.78% MySQL 5.5 7.03% MariaDB 10.5 6.77% MariaDB 10.3 2.74% MySQL 5.6 1.99% MariaDB 11.4 1.27% MariaDB 10.4 1.07% MariaDB 5.5 0.46% MariaDB 10.1 0.43% MariaDB 10.2 0.32% MySQL 5.1 0.17% MySQL 8.4 0.14% I haven't verified this myself, but I heard that MySQL 8.4 deprecated SQL_CALC_FOUND_ROWS which is heavily used in the WordPress code base, and thus the MySQL 8.4 adoption is nearly zero. Good reminder to have collaboration and joint testing with some of the largest open source projects to avoid accidentally having a new MariaDB version being incompatible with them..

2 2

Commit ff4209fa0d1c66952bc9f00587f525314464e7be not merged on main?
by Otto Kekäläinen 08 May '25

08 May '25

Hi! I was updating the patch https://salsa.debian.org/mariadb-team/mariadb-server/-/blob/debian/latest/d… in Debian when importing MariaDB 11.4.6, and I noticed that the upstream commit that changes the same lines in https://github.com/MariaDB/server/commit/ff4209fa0d1c66952bc9f00587f5253144… only landed on MariaDB '10.11' and '11.4' branch, but never on 'main' or '11.8'. Could it be that 'main' has had zero merges from older branches since April 3? Or was there maybe some mistake and one merge was overlooked and it wasn't properly applied on all newer maintained branches?

2 2

CI failures visible on GitHub (affecting contributors)
by Otto Kekäläinen 29 Apr '25

29 Apr '25

Hi, I see that on the 11.8 branch (https://github.com/MariaDB/server/commits/11.8/) which is supposed to be fully green and passing CI as it is soon to be released as a new LTS version, actually isn't consistently green. Out of the latest 10 CI runs the jobs passing rates are: 12/!2 11/12 12/!2 12/!2 8/8 10/12 8/12 7/8 8/8 11/12 Questions: - What is the current plan to get CI to be consistently green? - How is it possible that a commit is pushed on the 11.8 branch even though CI is not passing? Isn't branch protection preventing it? - If there is a way to get commits on 11.8 branch with failures bypassing branch protection, are all developers committed to follow-up on their commits and fix them before they do any other work? Or what is the plan for how to ensure post-push test failures are fixed? - The jobs buildbot/amd64-debian-11-msan-clang-16 and continuous-integration/appveyor/branch seem to be the most common failing ones. Can they be removed now to make the CI consistently green? And apply full branch protection so that they, or any other jobs, can't be added back to unless they are actually green? - Why is the CI occasionally tunning 8 jobs instead of 12? I also see plenty of red at https://buildbot.mariadb.org/#/grid?branch=11.8. Hopefully we get as much green as possible for the start of the 11.8 maintenance so that any potential regressions in future stable maintenance releases of 11.8 series are easier to catch. Most of the issues seem to be test issues and not real MariaDB bugs. I have also done a bunch of testing on Debian QA systems for MariaDB 11.8 and I have only found a couple small issues, so overall 11.8 is looking good! - Otto

3 3

MariaDB 11.8 and Debian status - information to fellow Linux distro packagers/maintainers
by Otto Kekäläinen 27 Apr '25

27 Apr '25

Hi! Do we have any Linux distro packagers and maintainers on this mailing list? If you are working on importing MariaDB 11.8 into a Linux distro, you may want to take a peek at what was done in Debian, as it may save you some work in debugging build or test issues: * https://salsa.debian.org/mariadb-team/mariadb-server/-/commits/debian/latest * https://salsa.debian.org/mariadb-team/galera-4/-/commits/debian/latest You might also benefit from looking at work-in-progress and open Merge Requests at : * https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests In particular the MR https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/115 about a NEWS item to Debian users is likely to have useful text/summary that others might want to re-use: * https://salsa.debian.org/otto/mariadb-server/-/blob/dev/NEWS-11.8.2/debian/… Overall Debian package statuses are best viewed at: * https://tracker.debian.org/pkg/mariadb * https://tracker.debian.org/pkg/galera-4 ..and respective in Ubuntu: * https://launchpad.net/ubuntu/+source/mariadb * https://launchpad.net/ubuntu/+source/galera-4 I've fixed a lot of bugs in past weeks. The Debian tracker MariaDB bugs are down from 60 to 44, and in Ubuntu down to 3. For Galera the numbers are now down to 1 and 0. Looking forward to collaborate with maintainers of MariaDB and Galera packages in other Linux distributions! - Otto

1 0

Remaining InnoDB parts for MDEV-34705 binlog-in-engine?
by Kristian Nielsen 25 Apr '25

25 Apr '25

Hi Marko, I have been making some good progress on the new binlog implementation, I implemented all of what we discussed regarding the InnoDB side I believe, and also got a lot done on the integration with the server side code. I wanted to ask you what outstanding issues on the InnoDB side remain from your point of view, if any? The page format now is implemented in the new design, as described in a separate mail: https://lists.mariadb.org/hyperkitty/list/developers@lists.mariadb.org/thre… All the InnoDB-specific page overhead is gone, only a 4-byte CRC32 remains at the end of each page. The first page is reserved as a header page. The new binlog code no longer uses the InnoDB buffer pool, nor create any InnoDB tablespace structures (except for the two reserved tablespace IDs for the redo records). The code has its own buffer pool replacement (called the "page fifo") and own background flushing. The binlog hooks into the InnoDB crash recovery code following your suggestions, and just the single "memcpy" record is needed. Recovery is implemented (and tested somewhat), and has been carefully designed to recover in all cases (barring any bugs of course) while requiring only the two reserved tablespace IDs. I have two outstanding InnoDB-related issues I know about: 1. In order for a replication setup to be crash-safe, I must ensure that the slave does not receive an event until it has become durable on the master. Otherwise, when the master crashes and comes back up, an event might be applied on the slave that no (longer) exist on the master. For this, I need something in the InnoDB redo log code that will callback into the binlog after a specific LSN has been durably written to disk. This callback will then notify the dump threads (slave connections) somehow that the corresponding part of the binlog can now be safely read. 2. Integration with mariabackup. With the new binlog, mariabackup can backup the binlog together with the rest of the InnoDB data in a consistent way, something that I think is not possible currently? This will also remove the need for mariabackup to extract the binlog position with SHOW MASTER STATUS or for InnoDB to store the binlog position in the undo segments. When the binlog is backed up, the correct GTID position can be obtained after restoring a master backup simply by checking the binlog (SELECT @@gtid_binlog_pos). (For a slave backup, the position is similarly available from the backup of the mysql.gtid_slave_pos table, as before). I am not yet sure of the details of mariabackup support. On the high level, it will involve copying the binlog-NNNNNN.ibb into the backup along with the other InnoDB tablespace files; and applying redo records to those binlog files as part of --prepare. One important detail is to handle that we have only two reserved tablespace IDs for binlog files. The current recovery code requires that only the two most recent binlog files need recovery. We could try to ensure that the same applies duing mariabackup --prepare. An idea is to use the fact that binlog files are strictly append-only. Around the point where mariabackup decides the LSN where it stops copying the redo log that becomes the backup snapshot, we can determine the last binlog file that exists. We will check this just before and just after determining the LSN, and repeat if a new binlog file was created just at that point (which would otherwise leave doubt if the file was created before or after that LSN). Then after the snapshot LSN is determined, we can just copy all the binlog files up to and including (but not after) the snapshot point. This way mariabackup --prepare should be able to run the same binlog recovery code that runs in crash recovery, only needing to consider the two most recent of the copied binlog files. I can imagine other ways to do mariabackup of binlog files, but this is one idea that could be feasible. What do you think? Anything other outstanding issues with the new binlog implementation regarding InnoDB that you are aware of? - Kristian.

1 1

Re: d20e4e10353: MDEV-36536 Add option to not collect statistics for long char/varchars
by Sergei Golubchik 23 Apr '25

23 Apr '25

Hi, Michael, On Apr 22, Michael Widenius wrote: > revision-id: d20e4e10353 (mariadb-10.6.21-79-gd20e4e10353) > parent(s): 4f0a0dbd104 > author: Michael Widenius > committer: Michael Widenius > timestamp: 2025-04-22 10:53:08 +0300 > message: > > MDEV-36536 Add option to not collect statistics for long char/varchars > > This is needed to make it easy for users to automatically ignore long > char and varchars when using ANALYZE TABLE PERSISTENT. These fields > can cause problems as they will consume 'CHARACTERS * > MAX_CHARACTER_LENGTH * 2 * number_of_rows' space on disk during > analyze, which can easily be much bigger than the analyzed table. > > This commit adds a new user variable, analyze_max_length, default > value 4G. Any field that is bigger than this in bytes, will be ignored > by ANALYZE TABLE PERSISTENT unless it is specified in FOR COLUMNS(). > > While doing this patch, I noticed that we do not skip GEOMETRY columns > from ANALYZE TABLE, like we do with BLOB. This should be fixed when > merging to the 'main' branch. At the same time we should add a > resonable default value for analyze_max_length, probably 1024, like we > have for max_sort_length. > > diff --git a/mysql-test/main/mysqld--help.test b/mysql-test/main/mysqld--help.test > index 971983fd66c..44449cd3ad5 100644 > --- a/mysql-test/main/mysqld--help.test > +++ b/mysql-test/main/mysqld--help.test > @@ -20,14 +20,14 @@ exec $MYSQLD_BOOTSTRAP_CMD --symbolic-links=0 --log-bin=foo --lower-case-table-n > > perl; > # Variables which we don't want to display in the result file since > - # their paths may vary: > + # their paths may vary or they may use a default of 4294967295 : why? what's wrong with 4294967295? > @skipvars=qw/basedir open-files-limit general-log-file log plugin-dir plugin-maturity > log-slow-queries pid-file slow-query-log-file log-basename > datadir slave-load-tmpdir tmpdir socket thread-pool-size > large-files-support lower-case-file-system system-time-zone > collation-server character-set-server log-tc-size table-cache > table-open-cache table-open-cache-instances max-connections > - server-uid tls-version version.*/; > + server-uid tls-version version.* analyze-max-length/; > > # Plugins which may or may not be there: > @plugins=qw/innodb archive blackhole federated partition s3 > diff --git a/sql/sql_class.h b/sql/sql_class.h > index 69b021cd41d..aac433181a4 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -731,6 +731,7 @@ typedef struct system_variables > ha_rows select_limit; > ha_rows max_join_size; > ha_rows expensive_subquery_limit; > + ulong analyze_max_length; don't use ulong, please. uint or ulonglong. > ulong auto_increment_increment, auto_increment_offset; > #ifdef WITH_WSREP > /* > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index 0d4a8e4c6fe..4ae14a58feb 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -445,6 +445,20 @@ static Sys_var_double Sys_analyze_sample_percentage( > CMD_LINE(REQUIRED_ARG), VALID_RANGE(0, 100), > DEFAULT(100)); > > +/* > + The max length have to be UINT_MAX32 to not remove GEOMETRY fields > + from analyze. > +*/ > + > +static Sys_var_ulong Sys_analyze_max_length( > + "analyze_max_length", > + "Fields that require more storage than analyze_max_length and are not " better "Fields which length in bytes more than this are skipped by ANALYZE TABLE PERSISTENT unless explicitly listed in the FOR COLUMNS () clause" "require more storage" is not very clear > + "listed in ANALYZE ... FOR COLUMNS () will automatically be skipped by " > + "ANALYZE TABLE PERSISTENT", > + SESSION_VAR(analyze_max_length), > + CMD_LINE(REQUIRED_ARG), VALID_RANGE(32, UINT_MAX32), > + DEFAULT(UINT_MAX32), BLOCK_SIZE(1)); > + > static Sys_var_ulong Sys_auto_increment_increment( > "auto_increment_increment", > "Auto-increment columns are incremented by this", > Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Design of new binlog file format (MDEV-34705)
by Kristian Nielsen 21 Apr '25

21 Apr '25

As the MDEV-34705 binlog-in-engine project matures, I'm finalising the file format of the new binlog, and will present it here in case of any final comments or request for changes. The file format of course is an important part of the design, as file formats in general are more difficult to change going forward than many other aspects of the code. Following previous discussions on the list with Marko, the file format of the new binlog is now simplified, and mostly independent of InnoDB. The InnoDB buffer pool is no longer used for binlog files, and InnoDB recovery uses separate code to recover binlog pages than other tablespace pages. The binlog implementation reserves two special tablespace IDs in the redo log to denote binlog files, but otherwise binlog files share little code with other InnoDB tablespaces. 1. Binlog file names -------------------- The innodb binlog files have a fixed naming scheme: binlog-000000.ibb binlog-000001.ibb binlog-000002.ibb ... By default they are written to the data directory, but a separate directory can be configured with --binlog-directory. This, there is no longer any need for a binlog index file. New binlog files are pre-allocated to the configured --max-binlog-size in the background. Unused pages contain zeros until written to. Each binlog file is always written strictly append-only, bytes are never overwritten or written out-of-order. 2. Binlog file header --------------------- Each binlog file is page-based. The page size is 4k, fixed in the initial release, but planned to be configurable in a future release. Pages are checksummed (same algorithm as InnoDB full_crc32). The last four bytes of each page contain the CRC32, rest is available for data (no special page header is needed). The first page of the file is reserved as a file header. It contains the following data (little-endian format): Offset Length Data 0 4 "Magic" file identifier (0x010dfefe) 4 4 Log2 of page size (ie. 12 for 4k pages) 8 4 Major version number (0) 12 4 Minor version number (0) 16 8 The file number from the binlog name, consistency check 24 8 File length, in number of pages 32 8 LSN corresponding to the start of this binlog file 40 8 The "GTID state interval" N. Used to speed up GTID lookup. 48 8 File number containing out-of-band data referenced from this file. 56 8 File number containing XA data referenced from this file (for future expansion to support XA). 508 4 Extra crc32 checksum, to support down to 512 bytes page size. The idea with version numbers is that new minor versions are readable by older code, while new major versions are not and older code must refuse to read the file. The LSN in the header is used to do recovery correctly. Any redo record with smaller LSN will be ignored during recovery and not applied to this file. This way only two tablespace IDs are needed for recovery of binlog files. The GTID state interval (--innodb-binlog-state-interval) is a replacement for the GTID indexes of the legacy binlog. Every N pages in a binlog file, the current binlog GTID state is written. This allows to quickly find a GTID position for the slave to start at, by doing binary search on the GTID state records and then scanning ahead over the last few (<=N) pages to find the starting position. The first GTID state in the file stores the full state, the remaining pages only store the difference to the first state. The out-of-band (OOB) file number reference is used to manage when large transactions are written into the binlog files interleaved with other transaction data. If a large transaction starts binlogging in binlog-000005.ibb say, and then completes and writes the commit record to file binlog-000008.ibb, then the file header for file_no=8 will contain oob-reference=5, to mark that file_no=5 is needed by dump threads(slave connections) reading from file_no=8. An extra crc32 checksum is stored at offset 512 in the header page. This is to be later able to support 512-byte (or larger) configurable page size. This makes it possible to read the first 512 bytes of the file, verify the integrity, and read the actual page size at offset 4. The initial release is planned to have fixed 4k page size. 3. Binlog data page format -------------------------- Overall, the binlog contains a logical sequence of "records". Conceptually, the binlog is one single stream of such records; one record can span two files, and file boundary is not meaningful for the binlog content. Each binlog record is written as a single InnoDB mini-transaction (mtr). This means binlog records will be recovered atomically during InnoDB crash recovery, a partial record will not be found at the end of a binlog file. It also means the size of one record is limited by the maximum size of an mtr. Each record is split into one or more "chunks". A chunk always fits in one page. Each binlog page (apart from the first header page) contains one or more chunks. Then follows 0-3 filler bytes 0xff to pad any remaining space, and last the 4 bytes of CRC32 checksum. Here is the byte layout of a page: Offset Length Data 0 len+3 <type> <len1> <len2> <data 0> <data 1> ... <data len-1> len+3 A+3+B+3 <type> <len1> <len2> <data 0> <data 1> ... <data len-1> ... 4089 1 <filler (optional)> 4090 1 <filler (optional)> 4091 1 <filler (optional)> 4092 4 <CRC0> <CRC1> <CRC2> <CRC3> Thus, each chunk has a simple format with one byte denoting the chunk type, two bytes (little-endian) denoting the length, and then the data. Chunk types: 0 Empty (not yet used data, has no length field) 1 Commit record 2 GTID state record, ie. for --innodb-binlog-state-interval 3 Out-of-band binlog data for large transactions 4 Dummy record (used to fill up the last page when FLUSH BINARY LOGS) 0xff Filler byte, to pad page when there is no room for a whole chunk (no length field). Here for example is a commit record, type=1: 00001000: 41c8 0000 918e 0168 a201 0000 A......h.... 00001010: 0026 0000 0000 0000 0008 0001 0000 0000 .&.............. 00001020: 0000 0000 0000 0029 0000 0000 0000 918e .......)........ 00001030: 0168 0201 0000 00a1 0000 0000 0000 0000 .h.............. 00001040: 0007 0000 0000 0000 0004 0000 2300 0000 ............#... 00001050: 0000 0101 0000 2054 0000 0000 0603 7374 ...... T......st 00001060: 6404 0800 0800 0800 818c 0000 0000 0000 d............... 00001070: 0074 6573 7400 4352 4541 5445 2054 4142 .test.CREATE TAB 00001080: 4c45 2074 3120 2861 2049 4e54 204e 4f54 LE t1 (a INT NOT 00001090: 204e 554c 4c2c 2062 2049 4e54 204e 4f54 NULL, b INT NOT 000010a0: 204e 554c 4c2c 2063 2054 4558 542c 2050 NULL, c TEXT, P 000010b0: 5249 4d41 5259 204b 4559 2861 2c20 6229 RIMARY KEY(a, b) 000010c0: 2920 454e 4749 4e45 3d49 6e6e 6f44 42 ) ENGINE=InnoDB The 0x41 is the type, and the 0xc8 0x00 is the length=0x00c8. Then follows the raw event data, in the same format used in the existing binlog (sql/log_event.h). The type byte has two additional flag bits (hence 0x41 and not 0x01): Bit 6 (0x40) This is the last chunk of a record Bit 7 (0x80) This is a continuation chunk (not first) of a record Thus a commit record can consist of a single chunk: type=0x41 , or two chunks: type=0x01 type=0xc1 , or more than two chunks: type=0x01 type=0x81 type=0x81 ... type=0xc1 Chunks are thus used to split a record across pages (a page will not contain more than one chunk for a single record). 4. Chunk data contents ---------------------- The commit record, type=1, has the following header data, consisting of 5 64-bit numbers: num_oob Number of out-of-band records in this transaction oob_first_file_no The file_no of the first oob record oob_first_offset The offset into oob_first_file_no oob_last_file_no The file_no of the last oob record oob_last_offset The offset into oob_last_file_no For small event groups with no out-of-band data, only num_oob=0 is stored. The numbers are stored in a compressed format that saves space for small numbers, see storage/innobase/include/ut0compr_int.h After follows the raw binlog event data. For a small transaction with no out-of-band records, this is the whole event group (GTID, Query/rows, XID/Commit). For large transactions it is just the GTID event. See above for example commit record. The GTID state record, type=2, is just a list of GTIDs, similar to the existing Gtid_list_log_event: number_of_gtids domain_id_0, server_id_0, seq_no_0 domain_id_1, server_id_1, seq_no_1 ... domain_id_N-1, server_id_N-1, seq_no_N-1 Like for commit records, the numbers are stored compressed to save space for small numbers. Here is an example GTID state record containing one GTID 0-1-575 (see ut0compr_int.h for how to interpret the compressed numbers 0x8 0x0 0x8 0x11f9 as 1, 0, 1, 575): 00001000: 4205 0008 0008 f911 B....... 00001010: The OOB data record, type=3, stores one node in a binary tree of event group data. The out-of-band data is structured as binary trees in a way that can be written strictly append-only, and read efficiently by dump threads/connected slaves. The OOB node is defined by 5 numbers: index The identification of the node (nodes are numbered 0, 1, 2...) left_file_no The file_no of the left child of the node left_offset The offset into left_file_no of the left child. Zero denotes a leaf node. right_file_no The file_no of the right child of the node right_offset The offset into the right_file_no. After this follows the raw event data. 5. Conclusion ------------- So this is how the new binlog format is planned to look, barring any last-minute design change requirements or design review comments. I tried to include a good amount of details, feel free to ask for any of the specific details that I did omit, if interested. Hope this helps, - Kristian.

1 0

Re: [JIRA] (MDEV-36629) Change SLAVE to REPLICA in Server error messages
by Kristian Nielsen 18 Apr '25

18 Apr '25

Taking this discussion out of the MDEV-36629 specifics, as this is a general issue. It concerns a problem with how documentation is now being incorrectly updated: "Alex Hanshaw (Jira)" <jira(a)mariadb.org> writes: > Alex Hanshaw commented on MDEV-36629: > Understood. For context Vikas had told the Doc team "I understand the > compatibility issue - so we need to make both options work and > document only the new terminology." When the Doc team asked about the Ouch. This will need to be reverted ASAP, before more damage is done. The documentation is for documenting how the server works. All aspects of how it works. Not some distorted version, whether good- or ill-intended. We must certainly _not_ deliberately remove documentation of existing syntax and functionality! For example I see that documentation of the syntax START SLAVE was removed. This is just plain wrong. START SLAVE is working syntax in replication, and will remain so for the foreseeable future. It needs to be documented for users to be able to understand how it works. Please take this seriously, and ensure that such changes are stopped immediately, and all incorrect changes reverted. - Kristian. >> Change SLAVE to REPLICA in Server error messages >> ------------------------------------------------ >> >> Key: MDEV-36629 >> URL: https://jira.mariadb.org/browse/MDEV-36629 >> Project: MariaDB Server >> Issue Type: Task >> Components: Replication >> Reporter: Shwetha CM >> Assignee: Sergei Golubchik >> Priority: Major >> Fix For: N/A

2 1

Re: 6d4479d0e32: MDEV-36565 Assertion `src != ((void *)0)' failed in my_casedn_8bit
by Sergei Golubchik 16 Apr '25

16 Apr '25

Hi, Alexander, I don't know, this looks like a strange place to fix this. I'd say that either Item_func_xml_extractvalue shouldn't return {0, 0} or, if we'll treat it as a valid String, caseup() and casedn() should be able to handle it. But in a similarly-looking bug MDEV-35945 Assertion `src != ((void *)0)' failed in my_caseup_8bit you fixed it to use { "", 0 } for an empty string. It's commit c69fb1a6273. Then I'd suggest to do the same here. Let's avoid {0, 0} ? On Apr 16, Alexander Barkov wrote: > revision-id: 6d4479d0e32 (mariadb-11.4.5-25-g6d4479d0e32) > parent(s): d3c9a2ee212 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2025-04-15 19:30:44 +0400 > message: > > MDEV-36565 Assertion `src != ((void *)0)' failed in my_casedn_8bit > > diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc > index 2e3c33059d3..5df25c7f237 100644 > --- a/sql/item_strfunc.cc > +++ b/sql/item_strfunc.cc > @@ -1998,7 +1998,16 @@ String *Item_str_conv::val_str(String *str) > goto err; > > null_value= false; > - len= converter(collation.collation, (char*) res->ptr(), res->length(), > + /* > + Avoid passing nullptr as the second parameter to converter(): > + - val_str() of some Items (e.g. Item_func_xml_extractvalue) can return > + Strings with {Ptr=0,str_length=0} meaning an empty string. > + - But my_charset_handler_st virtual function caseup() and casedn() > + (which "converter" point to) do not expect {src=nullptr,srclen=0} input. > + */ > + DBUG_ASSERT(res->ptr() != nullptr || res->length() == 0); > + len= !res->ptr() ? 0/*Avoid passing nullptr*/ : > + converter(collation.collation, (char*) res->ptr(), res->length(), > (char*) str->ptr(), alloced_length); > DBUG_ASSERT(len <= alloced_length); > str->set_charset(collation.collation); > Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

1 0

Re: MDEV-36290: ALTER TABLE with multi-master can cause data loss (PR #3967)
by Kristian Nielsen 16 Apr '25

16 Apr '25

Brandon Nesterenko <notifications(a)github.com> writes: > MDEV-36290: ALTER TABLE with multi-master can cause data loss > https://github.com/MariaDB/server/pull/3967 Hi Brandon, Here is my initial review of the MDEV-36290 draft patch. Overall, it is clear that this is not something that's appropriate for 10.6. It implements a new way to apply row events, based on column names (if available) rather than column number. It is something for next dev version, 12.x. > commit de27bfbafc22601d53ddc0567401fcb26d28a91c > Author: Brandon Nesterenko <brandon.nesterenko(a)mariadb.com> > Date: Thu Apr 3 12:37:09 2025 -0600 > > MDEV-36290: ALTER TABLE with multi-master can cause data loss Please do not use the term "data loss" in the description (I know it is not you who introduced the term ;-). The issue here is conflicting queries run in parallel against multiple masters in a multi-master setup. In general, the support of multi-master in asynchronous replication requires the user to ensure that no conflicting queries are run, or to ensure that such conflicts will not cause undesirable behaviour. The behaviour addressed here is just one of many ways that conflicting ALTER TABLE could cause different data on slave and master. Using the term "data loss" can cause misunderstanding about what the problem is and what the change does. Of course, every case that is handled more correctly is in principle better. In this case, we introduce the facility to configure (--binlog_row_metadata=FULL) replication so that it is possible to replicate between tables on master and slave with different order of the columns. Not sure how useful such an option is in practice, seems to be very few users that will really need this, and such need has to be weighed against the increase in complexity in the code and potential for bugs. With that said, the approach in the patch generally looks reasonable. A few initial detail comments below, as you mentioned this is still a draft patch. As you also mentioned to me earlier, the logic in the patch is not trivial. I did not yet try to understand in detail all parts of the logic and the different corner cases. > + unsigned char *get_optional_metadata_str() > + { > + return m_optional_metadata; > + } > + > + unsigned int get_optional_metadata_len() > + { > + return m_optional_metadata_len; > + } > + Generally I do not like such do-nothing accessor functions. They just add abstraction and reduce grepability without doing anything useful. That's a style preference of course. > + /* > + Maps column index from master to slave. This is determined using the field > + names (provied by optional metadata when the master is configured with > + binlog_row_metadata=FULL). > + */ > + std::map<uint, uint> master_to_slave_index_map; > + > + /* > + When using field names to map from master->slave columns, this keeps track > + of column indices on the master which aren't present on the slave. > + > + It is used to skip columns when checking type-conversions and unpacking > + row data. > + */ > + std::set<uint> master_unmatched_cols; Is using std::map/std::set going to be expensive here, this is operations that will happen on every row-based event (when --binlog-row-metadata=FULL)? The std:: functions can imply a lot of memory allocations that are not very visible, and we usually try to avoid them in performance-relevant server code. master_to_slave_index_map is just a lookup table of numbers 0..#columns-1, right? So it doesn't need to be an associative array, a normal array is sufficient, and doesn't need to be re-allocated each event. And master_unmatched_cols is again just a set of numbers 0..#columns-1, so a bitmap could be used. - Kristian.

1 0