MDEV-41012 Galera appliers hang with foreign key of types UUID, INET4, INET6 - #5634
MDEV-41012 Galera appliers hang with foreign key of types UUID, INET4, INET6#5634sjaakola wants to merge 3 commits into
Conversation
…, INET6 Added a deterministic test for reproducing the issue
…, INET6 Added second test for testing key collisions from transactions modifying separate rows
…, INET6 wsrep_store_key_val_for_row() built the certification key of a row by collating the column value whenever the field reports MYSQL_TYPE_STRING or MYSQL_TYPE_VAR_STRING, taking the collation from Field::charset(). The data types implemented on Field_fbt - UUID, INET6 and INET4 report MYSQL_TYPE_STRING, and their charset() is my_charset_numeric, which is latin1. Their values are however plain binary and accordingly get_innobase_type_from_mysql_type() maps them to DATA_FIXBINARY. Their keys were therefore run through latin1_swedish_ci, which folds them. That corrupts the key in two ways: 1. A key mismatch for one and the same row. The reference key that wsrep_rec_get_foreign_key() appends for the parent of a child INSERT is built from the InnoDB record and is not collated, so it no longer matched the primary key carried by the parent row's own writeset. Certification saw no dependency between a child INSERT and a concurrent parent UPDATE, and two appliers could apply them in parallel causing a hang or crash. 2. A key collision between distinct rows. The folding is many to one, so different values collapse onto one key, Certification compares keys byte for byte, so unrelated rows were treated as the same row. Concurrent transactions on them certified as a conflict and one was aborted with ER_LOCK_DEADLOCK. Fix is for wsrep_store_key_val_for_row() to skip the collation for fields that InnoDB stores as binary, using the same condition as get_innobase_type_from_mysql_type(). This is a no-op for the types that worked before.
7e981a9 to
adc2e21
Compare
janlindstrom
left a comment
janlindstrom
left a comment
There was a problem hiding this comment.
Fix and test cases looks correct, but in my understanding changing binary format requires protocol change.
Style: wsrep_store_key_val_for_row is too long and complicated, it is getting to state where it is not maintainable or understandable. Consider splitting.
Because your branch is against MariaDB repository only Buildbot testing is run and it lacks most of the debug full testing. https://github.com/mariadb-corporation/codership-galera is only repository currently where full CI is run.
Future: Maybe we should get these wsrep-only functions to separate file e.g. ha_innodb_wsrep.cc, but before anyone does that better ask Marko if he agrees/cares.
| if character set is multi byte. | ||
|
|
||
| Fields that InnoDB stores as DATA_BINARY, | ||
| DATA_FIXBINARY or DATA_BLOB must not be collated |
There was a problem hiding this comment.
Minor : DATA_BLOB is not handled in this branch see line 6845.
Major: Write-set key encoding for UUID/INET4/INET6 changes with no wsrep_protocol_version gate; in a mixed-version cluster old and new nodes compute different keys for the same row, so concurrent same-PK INSERTs both certify and the applier then fails with ER_DUP_ENTRY, dropping a node. Every other format change in this file is version-gated.
2 participants
wsrep_store_key_val_for_row() built the certification key of a row by collating the column value whenever the field reports MYSQL_TYPE_STRING or MYSQL_TYPE_VAR_STRING, taking the collation from Field::charset().
The data types implemented on Field_fbt - UUID, INET6 and INET4
report MYSQL_TYPE_STRING, and their charset() is my_charset_numeric,
which is latin1. Their values are however plain binary and accordingly
get_innobase_type_from_mysql_type() maps them to DATA_FIXBINARY. Their
keys were therefore run through latin1_swedish_ci, which
folds them. That corrupts the key in two ways:
A key mismatch for one and the same row. The reference key that
wsrep_rec_get_foreign_key() appends for the parent of a child INSERT is
built from the InnoDB record and is not collated, so it no longer
matched the primary key carried by the parent row's own writeset.
Certification saw no dependency between a child INSERT and a concurrent
parent UPDATE, and two appliers could apply them in parallel causing
a hang or crash.
A key collision between distinct rows. The folding is many to one, so
different values collapse onto one key, Certification compares keys byte
for byte, so unrelated rows were treated as the same row. Concurrent
transactions on them certified as a conflict and one was aborted with
ER_LOCK_DEADLOCK.
Fix is for wsrep_store_key_val_for_row() to skip the collation for
fields that InnoDB stores as binary, using the same condition as
get_innobase_type_from_mysql_type(). This is a no-op for the types that
worked before.