January 2024 - developers - lists.mariadb.org

[Maria-developers] MariaDB // Temporal tables
by Robert Palm 02 Oct '24

02 Oct '24

2 2

Feedback plugin status?
by Otto Kekäläinen 16 Apr '24

16 Apr '24

Hi! I was about to merge a long overdue MR in Debian to enable the Feedback plugin (https://salsa.debian.org/mariadb-team/mariadb-server/-/merge_requests/16) However I noticed all the links at https://mariadb.org/about/#usage-statistics are broken. The wiki page at https://mariadb.com/kb/en/feedback-plugin/ says that the plugin is 'currently not working'. What does this mean? What is the path to get it working again? Is there a need for somebody to rewrite the stats server part? Thanks, - Otto

3 4

Review of MDEV-7850: Extend GTID Binlog Events with Thread Id
by Kristian Nielsen 31 Jan '24

31 Jan '24

> commit c37b2087b4abe576f1b0391c8d379dba6299dcb5 (origin/bb-11.4-MDEV-7850) > Author: Brandon Nesterenko <brandon.nesterenko(a)mariadb.com> > Date: Mon Jul 10 18:53:19 2023 +0300 > > MDEV-7850: Extend GTID Binlog Events with Thread Id > > This patch augments Gtid_log_event with the user thread-id. > In particular that compensates for the loss of this info in > Rows_log_events. Here's my review of this patch (which was already pushed, but there are some misunderstandings that should be fixed; Monty asked me about it as he had been told that I had reviewed the patch, but that is not the case (until now)): > diff --git a/sql/log_event.cc b/sql/log_event.cc > index 41ddc038594..f167284ad1d 100644 > --- a/sql/log_event.cc > +++ b/sql/log_event.cc > @@ -1901,11 +1904,12 @@ Query_log_event::begin_event(String *packet, ulong ev_offset, > /* > Currently we only need to replace GTID event. > The length of GTID differs depending on whether it contains commit id. > + And/or thread id. > */ > - DBUG_ASSERT(data_len == LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN || > - data_len == LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 2); > - if (data_len != LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN && > - data_len != LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 2) > + DBUG_ASSERT(data_len >= LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN && > + data_len <= LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 2 + 9); > + if (data_len < LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN || > + data_len > LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 2 + 9) This change of the asserts/checks doesn't make any sense. They were there originally, because we are hand-crafting a BEGIN query event, and there were only two cases of the length, so two fixed-sized events were crafted by two different code paths. And the asserts are there to safe-guard if the length would change. But now this check was already wrong, as more optional info has been added to the GTID event without updating the code for Query_log_event::begin_event() (XID, flags_extra, extra_engines). But the new code in this patch is generic and can handle any length GTID event, so these checks no longer make any sense, and should just be removed (rather than attempt to correct them). > @@ -1926,9 +1930,19 @@ Query_log_event::begin_event(String *packet, ulong ev_offset, > } > else > { > - DBUG_ASSERT(data_len == LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 2); > - /* Put in an empty time_zone_str to take up the extra 2 bytes. */ > - int2store(q + Q_STATUS_VARS_LEN_OFFSET, 2); > + DBUG_ASSERT(data_len <= LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 11); This assert should not be there. The length can be larger than 11, and the code below works fine for arbitrary length. > + DBUG_ASSERT(data_len >= LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 2); > + > + /* Put in an empty time_zone_str to take up the extra 2 plus the number of > + dummy_bytes. */ > + size_t dummy_bytes= > + data_len - (LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN + 2); > + > + int2store(q + Q_STATUS_VARS_LEN_OFFSET, dummy_bytes + 2); > + for (size_t i= 0; i < dummy_bytes; i++) > + q[Q_DATA_OFFSET + i]= Q_DUMMY; > + q+= dummy_bytes; > + > q[Q_DATA_OFFSET]= Q_TIME_ZONE_CODE; > q[Q_DATA_OFFSET+1]= 0; /* Zero length for empty time_zone_str */ > q[Q_DATA_OFFSET+2]= 0; /* Zero terminator for empty db */ This new code with Q_DUMMY is generic and independent of the length of the GTID event. That's good! But it needs a comment explaining (that the Q_DUMMY is unknown to the old slave, which will make the old slave just skip the rest of the user vars, which are just there to fill up the extra bytes). And now the code that adds the dummy empty time_zone_str is redundant, and should just be removed, as should the code for the case of data_len == LOG_EVENT_HEADER_LEN + GTID_HEADER_LEN. Since now any extra bytes in the GTID can just be replaced by Q_DUMMY. This way, the code will be fixed to work for all the possible GTID lengths, and be shorter and simpler (and a lot less confusing). > @@ -1584,6 +1584,9 @@ Query_log_event::Query_log_event(const uchar *buf, uint event_len, > sa_seq_no = uint8korr(pos); > pos+= 8; > } > + } > + case Q_DUMMY: > + { > break; > } > default: > /* That's why you must write status vars in growing order of code */ > DBUG_PRINT("info",("Query_log_event has unknown status vars (first has code: %u), skipping the rest of them", (uint) *(pos-1))); > pos= (const uchar*) end; // Break loop There's no point in having this code. Q_DUMMY is supposed to be higher than any known status var type, so it will be ignored anyway in the `default` case. And besides, server that knows about Q_DUMMY can never receive it in the first place. (But if this is really wanted, then don't delete the `break` statement in the prior switch case, that's likely to introduce a bug the next time a case is added. And assign Q_DUMMY the next free value, not the value 255). > diff --git a/sql/log_event.h b/sql/log_event.h > index 473506fafed..d324642ad5e 100644 > --- a/sql/log_event.h > +++ b/sql/log_event.h > @@ -324,6 +324,7 @@ class String; > > #define Q_HRNOW 128 > #define Q_XID 129 > +#define Q_DUMMY 255 This needs a good comment, explaining how Q_DUMMY is a code that will be unknown to any server version, and can be used to make the slave skip this (and any following) user vars. diff --git a/mysql-test/suite/rpl/t/rpl_gtid_thread_id.test b/mysql-test/suite/rpl/t/rpl_gtid_thread_id.test new file mode 100644 index 00000000000..b8ed1865da9 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_gtid_thread_id.test @@ -0,0 +1,125 @@ +# +# Verify that GTID log events are written into the binary log along with the +# id of the thread which executed the transaction. On the primary, this is the <snip> Disclaimer: I did not review this testcase. - Kristian.

2 3

Slow or hanging parallel replication in 10.6
by Kristian Nielsen 30 Jan '24

30 Jan '24

As we are discussing possible problems with XA parallel replication being slow or hanging in 10.6, I just want to remind of these recently fixed bugs: MDEV-31482: Lock wait timeout with INSERT-SELECT, autoinc, and statement-based replication This can cause slow or hanging parallel replication, as it hangs for --innodb-lock-wait-timeout MDEV-32096 Parallel replication lags because innobase_kill_query() may fail to interrupt a lock wait This can cause show or hanging parallel replication MDEV-31655: Parallel replication deadlock victim preference code errorneously removed This can cause parallel replication to be slow or fail with "too many retries" These bugs were fixed in 10.6.16. These bugs are not related to XA. But the symptoms of these bugs are that optimistic parallel replication occasionally is slow, hangs, or breaks under high load. So it is important not to mistakenly assume that these symptoms are necessarily caused by XA, even though the workload that has problems might be using external XA transactions. In versions prior to 10.6.16, these kinds of problems are likely to be caused by the above known bugs. (I got this list from a quick scan through the 10.6 commit history, there may be one or two more like them I missed. But the conclusion is the same, using optimistic parallel replication under high load in 10.6 requires using at least 10.6.16 to work). - Kristian.

1 0

PR#2779: Fix merge commit 5ea5291: No test file or result files should be executable
by Otto Kekäläinen 28 Jan '24

28 Jan '24

Hi Sanja! Did you notice my PR https://github.com/MariaDB/server/pull/2779? I tagged you about a minor bug in you previous merge. I was hoping you would notice it and review that PR, but Daniel Black jumped on it. I am asking now because it seems the exact same thing happened for mysql-test/main/lowercase_table2.result in your commit fecd78b8378 from November 8th, so I thought I'd send an extra email now to ensure you are aware of this happening. - Otto

2 3

Suggestions for the problems around replication of XA in 10.5
by Kristian Nielsen 27 Jan '24

27 Jan '24

Hi Monty, As promised, here my thoughts around the issues with the implementation of replication of external XA as implemented since MariaDB 10.5. I see at least two architectural issues with the current implementation. One is that it splits transactions in two separate GTIDs in the binlog (XA PREPARE and XA COMMIT). This breaks with the fundamental principle that replication applies transactions one after the other in strict sequence, and the replication position/GTID is the single transaction last replicated. This for example means that a mysqldump backup can no longer be used to provision a slave, since any XA PREPAREd event at the time of the dump will be missing; a testcase rpl_xa_provision.test in MDEV-32020 demonstrates this. Another architectural issue is that each XA PREPARE keeps row locks around on every slave until commit (or rollback). This means replication will break if _any_ transaction replicated after the XA PREPARE gets blocked on a lock. This can easily happen; surely in many ways in statement-based replication, and even in row-based replication without primary key as demonstrated by testcase rpl_mdev32020.test in MDEV-32020. There are other problems; for example the update of mysql.gtid_slave_pos cannot be committed crash-safe together with the transaction for XA PREPARE (since the transaction is not committed). I believe the root of the problem is architectural: external XA should be replicated only after they commit on the master. Trying to fix individual problems one by one will not address the root problem and will lead to ever increasing complexity without ever being fully successful. The current implementation appears to only address a very specific and rare use-case, where enhanced semi-synchronous replication is used with row-based binlogging to try to fail-over to a slave and preserve any external XA that was in PREPAREd state on the master before the failover. Mixed-mode replication, provisioning slaves with mysqldump, slaves not intended for failover, etc., seem to be not considered and basically broken since 10.5. Here is my idea for a design that solves most of these problems. At XA PREPARE, we can still write the events to the binlog, but without a GTID, and we do not replicate it to slaves by default. Then at XA COMMIT we binlog a commit transaction that can be replicated normally to the slaves without problems. If necessary, the events for XA COMMIT can be read from the PREPARE earlier in the binlog, eg. after server crash/restart. We already have the binlog checkpoint mechanism to ensure that required binlog files are preserved until no longer needed for transaction recovery. This way we make external XA preserved across server restart, and all normal replication features continue to work - mysqldump, mixed-mode, etc. Nice and simple. Then optionally we can support the specific usecase of being able to recover external XA PREPAREd transactions on a slave after failover. When enabled, the slave can receive the XA PREPARE events and binlog them itself, without applying. Then as part of failover, those XA PREPARE in the binlog that are still pending can be applied, leaving them in PREPAREd state on the new master. This way, _only_ the few transactions that need to be failed-over need special handling, the majority can still just replicate normally. There are different refinements and optimizations that can be added on top of this. But the point is that this is a simple implementation that is robust, correct, and crash-safe from the start, without needing to add complexity and fixes on top. I've done some initial proof-of-concept code for this, and continue to work on it on the branch knielsen_mdev32020 on github. - Kristian.

3 6

Presentation and Interest in "SIMD for SQL Articulations and Capabilities" Undertaking
by Rayan Chowdhury 25 Jan '24

25 Jan '24

Dear MariaDB Developers, I trust this email thinks that you are well. I am Rayan Chowdhury, and I'm an understudy from India as of now seeking Computer Science in NIIT University. I'm excited about the chance to contribute to MariaDB after discovering the "SIMD for SQL Expressions and Functions" project recently. I have a strong fascination with data set frameworks and streamlining, and the objectives of this undertaking adjust impeccably with my goals. Adding vectorized assessment for SQL capabilities and articulations in the MariaDB ColumnStore motor is both captivating and testing, and I'm anxious to be a piece of the work to further develop execution around here. I've started my journey by signing up for the developers' mailing list, and I'm excited to get involved in the MariaDB community. Prior to jumping into commitments, I needed to connect with express my advantage in the undertaking and look for direction on how I can get everything rolling. I'm anticipating adding to MariaDB and gaining from the accomplished designers locally. Much thanks to you for your time, and I'm amped up for the chance of cooperating on this venture. Best respects, Rayan Chowdhury CSE Student, NIIT University

2 1

Re: 1a3974f534f: MDEV-33299 Assertion `(tm->tv_usec % (int) log_10_int[6 - dec]) == 0' failed in void my_timestamp_to_binary(const timeval*, uchar*, uint)
by Sergei Golubchik 24 Jan '24

24 Jan '24

Hi, Alexander, On Jan 24, Alexander Barkov wrote: > revision-id: 1a3974f534f (mariadb-11.3.1-12-g1a3974f534f) > parent(s): 83a79ba33b0 > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2024-01-23 18:04:31 +0400 > message: > > MDEV-33299 Assertion `(tm->tv_usec % (int) log_10_int[6 - dec]) == 0' failed in void my_timestamp_to_binary(const timeval*, uchar*, uint) > > diff --git a/sql/opt_rewrite_date_cmp.cc b/sql/opt_rewrite_date_cmp.cc > index 5a6410414da..8ff0ab3847e 100644 > --- a/sql/opt_rewrite_date_cmp.cc > +++ b/sql/opt_rewrite_date_cmp.cc > @@ -288,10 +288,13 @@ Item *Date_cmp_func_rewriter::create_end_bound() > const_arg_ts.hour= 23; > const_arg_ts.minute= TIME_MAX_MINUTE; > const_arg_ts.second= TIME_MAX_SECOND; > - const_arg_ts.second_part= TIME_MAX_SECOND_PART; > + const_arg_ts.second_part= TIME_MAX_SECOND_PART - > + my_time_fraction_remainder(TIME_MAX_SECOND_PART, > + field_ref->decimals); > if (check_datetime_range(&const_arg_ts)) > return nullptr; > res= new (thd->mem_root) Item_datetime(thd); > + res->decimals= field_ref->decimals; would it work you if always set res->decimals=6 here (and below)? looks like a simpler way to make the item consistent. > res->set(&const_arg_ts); > break; Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 2

Re: af51be1106b: MDEV-32837 long unique does not work like unique key when using replace
by Sergei Golubchik 24 Jan '24

24 Jan '24

Hi, Alexander, Generally, ok to push. One request for a change and one question below: On Jan 23, Alexander Barkov wrote: > revision-id: af51be1106b (mariadb-10.5.23-57-gaf51be1106b) > parent(s): c9902a20b3a > author: Alexander Barkov > committer: Alexander Barkov > timestamp: 2024-01-17 15:51:54 +0400 > message: > > MDEV-32837 long unique does not work like unique key when using replace > > write_record() when performing REPLACE has an optimization: > - if the unique violation happened in the last unique key, then do UPDATE > - otherwise, do DELETE+INSERT > > This patch skips this optimization in case if a non-versioned table > has hash unique (i.e. UNIQUE..USING HASH) constraints. > > Rationale: > > hash unique constraints are checked before regular unique indexes, > so even if a hash unique is the last unique constraint in the > table->key_info[] array - it guarantees nothing - there still > can be regular (non-hash) contraints that violate as well. > So just go the non-optmized path with DELETE followed > by INSERT on the next loop iteration. Don't forget to update the commit comment to match the final fix. > diff --git a/mysql-test/main/long_unique_bugs_no_protocol.test b/mysql-test/main/long_unique_bugs_no_protocol.test > new file mode 100644 > index 00000000000..a6f1f8e57e8 > --- /dev/null > +++ b/mysql-test/main/long_unique_bugs_no_protocol.test > @@ -0,0 +1,61 @@ > +--source include/no_protocol.inc why no protocol? Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 1

Re: 0c0721cb2f7: MDEV-31616 Problems with a stored function EMPTY() on upgrade to 10.6.
by Sergei Golubchik 22 Jan '24

22 Jan '24

Hi, Alexey, On Dec 27, Alexey Botchkov wrote: > revision-id: 0c0721cb2f7 (mariadb-10.6.16-45-g0c0721cb2f7) > parent(s): aff5ed3988a > author: Alexey Botchkov > committer: Alexey Botchkov > timestamp: 2023-12-19 00:00:48 +0400 > message: > > MDEV-31616 Problems with a stored function EMPTY() on upgrade to 10.6. > > The IDENT_sys doesn't include keywords, so the fucntion with the typo: fucntion > keyword name can be created, but cannot be called. > Moving some keywords to keyword_funcname set so the functions with these > names are allowed. The current logic in the code is: * built-in functions use IDENT_sys (no keywords allowed) in function_call_generic * functions with keyword names are explicitly listed in function_call_conflict. * stored function invocations use IDENT_sys via function_call_generic but * stored function declaration uses sp_name (that is keyword_ident) rule so, it'd make sense to introduce IDENT_funcname and use it instead of IDENT_sys in function_call_generic. That was a good idea. But also, I expect it to make function_call_conflict redundant, because all names from there should be in IDENT_funcname. And the patch doesn't solve the issue that many keywords from IDENT_sys are not in IDENT_funcname. So it's still possible to create functions that cannot be invoked. Note, that simply changing sp_name to use IDENT_funcname is not correct, because stored procedure names have different limitations. Regards, Sergei Chief Architect, MariaDB Server and security(a)mariadb.org

2 2