January 2025 - developers - lists.mariadb.org

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

03 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 1

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

03 Jan '25

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

2 5

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

01 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

1 0