- developers - lists.mariadb.org

Re: [Maria-developers] cf70893ac9d: MDEV-21303 Make executables MariaDB named
by Sergei Golubchik 18 Mar '20

18 Mar '20

Hi, Rasmus! See my comments/questions below: On Mar 16, Rasmus Johansson wrote: > revision-id: cf70893ac9d (mariadb-10.5.0-391-gcf70893ac9d) > parent(s): 56402e84b5b > author: Rasmus Johansson <razze(a)iki.fi> > committer: Rasmus Johansson <razze(a)iki.fi> > timestamp: 2020-03-16 11:10:25 +0000 > message: > > MDEV-21303 Make executables MariaDB named > > To change all executables to have a mariadb name I had to: > - Do name changes in every CMakeLists.txt that produces executables > - CREATE_MARIADB_SYMLINK was removed and GET_SYMLINK added by Wlad to reuse the function in other places also > - The scripts/CMakeLists.txt could make use of GET_SYMLINK instead of introducing redundant code, but I thought I'll leave that for next release > - A lot of changes to debian/.install and debian/.links files due to swapping of real executable and symlink. I did not however change the name of the manpages, so the real name is still mysql there and mariadb are symlinks. > - The Windows part needed a change now when we made the executables mariadb -named. MSI (and ZIP) do not support symlinks and to not break backward compatibility we had to include mysql named binaries also. Done by Wlad > > diff --git a/cmake/mysql_add_executable.cmake b/cmake/mysql_add_executable.cmake > index eec370d51af..f4d71ae9cef 100644 > --- a/cmake/mysql_add_executable.cmake > +++ b/cmake/mysql_add_executable.cmake > @@ -48,7 +48,7 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) > ENDIF() > > IF (ARG_WIN32) > - SET(WIN32 WIN32) > + SET(WIN32 ARG_WIN32) This looks wrong, see below > ELSE() > UNSET(WIN32) > ENDIF() > @@ -62,6 +62,7 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) > ELSE() > UNSET(EXCLUDE_FROM_ALL) > ENDIF() > + > ADD_EXECUTABLE(${target} ${WIN32} ${MACOSX_BUNDLE} ${EXCLUDE_FROM_ALL} ${sources}) here it'll pass ARG_WIN32 to ADD_EXECUTABLE. But ADD_EXECUTABLE takes an optional WIN32 keyword, not ARG_WIN32. > > # tell CPack where to install > @@ -79,16 +80,49 @@ FUNCTION (MYSQL_ADD_EXECUTABLE) > IF (COMP MATCHES ${SKIP_COMPONENTS}) > RETURN() > ENDIF() > + > IF (WITH_STRIPPED_CLIENT AND NOT target STREQUAL mysqld) this should be `STREQUAL mariadbd` now, I believe. > INSTALL(CODE "SET(CMAKE_INSTALL_DO_STRIP 1)" COMPONENT ${COMP}) > SET(reset_strip ON) > ENDIF() > - MYSQL_INSTALL_TARGETS(${target} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) > + > + IF(NOT ${mariadbname} STREQUAL "") where is mariadbname set? > + MYSQL_INSTALL_TARGETS(${mariadbname} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) > + ELSE() > + MYSQL_INSTALL_TARGETS(${target} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) > + ENDIF() > + > IF (reset_strip) > INSTALL(CODE "SET(CMAKE_INSTALL_DO_STRIP 0)" COMPONENT ${COMP}) > ENDIF() > ENDIF() > > - # create mariadb named symlink > - CREATE_MARIADB_SYMLINK(${target} ${ARG_DESTINATION} ${COMP}) > + # create MySQL named "legacy links" > + > + # Windows note: > + # Here, hardlinks are used, because cmake can't install symlinks. > + # In packages, there are won't be links, just copies. better to put the comment below in the ELSE branch, not here, far from the code that it is supposed to explain. > + GET_SYMLINK(${target} link) > + IF(link) > + IF(UNIX) > + ADD_CUSTOM_COMMAND(TARGET ${target} POST_BUILD > + COMMAND ${CMAKE_COMMAND} -E create_symlink > + ${target} ${link} > + COMMENT "Creating ${link} link" > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) > + INSTALL(PROGRAMS > + ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${link} > + DESTINATION > + ${ARG_DESTINATION} > + COMPONENT ${COMP}) > + ELSE() > + SET(link ${link}.exe) > + ADD_CUSTOM_COMMAND(TARGET ${target} POST_BUILD > + COMMAND cmake -E remove -f ${link} > + COMMAND mklink /H ${link} $<TARGET_FILE_NAME:${target}> > + COMMENT "Creating ${link} link" > + WORKING_DIRECTORY $<TARGET_FILE_DIR:${target}>) > + INSTALL(PROGRAMS $<TARGET_FILE_DIR:${target}>/${link} DESTINATION ${ARG_DESTINATION} COMPONENT ${COMP}) > + ENDIF() > + ENDIF() > ENDFUNCTION() > diff --git a/cmake/symlinks.cmake b/cmake/symlinks.cmake > index ec638bc82de..e040ff19f77 100644 > --- a/cmake/symlinks.cmake > +++ b/cmake/symlinks.cmake > @@ -9,68 +9,46 @@ macro(REGISTER_SYMLINK from to) > endmacro() > > # MariaDB names for executables > -REGISTER_SYMLINK("mysql_client_test_embedded" "mariadb-client-test-embedded") > -REGISTER_SYMLINK("mysql_client_test" "mariadb-client-test") > +REGISTER_SYMLINK("mariadb-client-test-embedded" "mysql_client_test_embedded") > +REGISTER_SYMLINK("mariadb-client-test" "mysql_client_test") ... I suppose you need to make sure that everything works with new names, without symlinks. That is all internal tools and scripts invoke other tools and scripts by their mariadb* names. A way to do it could be to comment out all REGISTER_SYMLINK lines and fix whatever that will break. grep is also a useful tool here, but with rather low signal-to-noise ratio. > + > +MACRO(GET_SYMLINK name out) > + set(${out}) > + list(FIND MARIADB_SYMLINK_FROMS ${name} _index) > if (${_index} GREATER -1) > - list(GET MARIADB_SYMLINK_TOS ${_index} mariadbname) > - endif() > - > - if (mariadbname) > - CREATE_MARIADB_SYMLINK_IN_DIR(${src} ${mariadbname} ${dir} ${comp}) > - endif() > -endmacro(CREATE_MARIADB_SYMLINK) > - > -# Add MariaDB symlinks in directory > -macro(CREATE_MARIADB_SYMLINK_IN_DIR src dest dir comp) > - if(UNIX) > - add_custom_target( > - SYM_${dest} ALL > - DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/${dest} > - ) > - > - add_custom_command(OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/${dest} POST_BUILD > - COMMAND ${CMAKE_COMMAND} -E create_symlink ${src} ${dest} > - COMMENT "mklink ${src} -> ${dest}") > - > - install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${dest} DESTINATION ${dir} COMPONENT ${comp}) > + list(GET MARIADB_SYMLINK_TOS ${_index} ${out}) > endif() > -endmacro(CREATE_MARIADB_SYMLINK_IN_DIR) > +ENDMACRO() > diff --git a/debian/mariadb-backup.install b/debian/mariadb-backup.install > index 2bc61c12079..e1967495b00 100644 > --- a/debian/mariadb-backup.install > +++ b/debian/mariadb-backup.install > @@ -1,5 +1,4 @@ > -usr/bin/mariabackup > +usr/bin/mariadb-backup > usr/bin/mbstream > usr/share/man/man1/mariabackup.1 > -usr/share/man/man1/mariadb-backup.1 this is strange. Why did you remove mariadb-backup.1 and not mariabackup.1 ? > usr/share/man/man1/mbstream.1 > diff --git a/debian/mariadb-client-10.5.install b/debian/mariadb-client-10.5.install > index 67c0c2619c3..216e4015851 100644 > --- a/debian/mariadb-client-10.5.install > +++ b/debian/mariadb-client-10.5.install > @@ -1,29 +1,17 @@ > debian/additions/innotop/innotop usr/bin/ > debian/additions/mysqlreport usr/bin/ > +usr/bin/mariadb-access > +usr/bin/mariadb-admin > usr/bin/mariadb-conv > -usr/bin/mysql_find_rows > -usr/bin/mysql_fix_extensions > -usr/bin/mysql_waitpid > -usr/bin/mysqlaccess > -usr/bin/mysqladmin > -usr/bin/mysqldump > -usr/bin/mysqldumpslow > -usr/bin/mysqlimport > -usr/bin/mysqlshow you removed mysqlshow here, and added mariadb-show to the server package. Intentional? UPD: I see that before your patch mysqlshow was in the client package, while mariadb-show symlink was in the server package. It's a bug, both should be in the client package. > +usr/bin/mariadb-dump > +usr/bin/mariadb-dumpslow > +usr/bin/mariadb-find-rows > +usr/bin/mariadb-fix-extensions > +usr/bin/mariadb-import > +usr/bin/mariadb-slap > +usr/bin/mariadb-waitpid > usr/bin/mysqlslap you did not remove mysqlslap, but added mariadb-slap > usr/bin/mytop > -usr/share/man/man1/mariadb-access.1 > -usr/share/man/man1/mariadb-admin.1 > -usr/share/man/man1/mariadb-binlog.1 > -usr/share/man/man1/mariadb-conv.1 > -usr/share/man/man1/mariadb-dump.1 > -usr/share/man/man1/mariadb-dumpslow.1 > -usr/share/man/man1/mariadb-find-rows.1 > -usr/share/man/man1/mariadb-fix-extensions.1 > -usr/share/man/man1/mariadb-import.1 > -usr/share/man/man1/mariadb-plugin.1 > -usr/share/man/man1/mariadb-slap.1 > -usr/share/man/man1/mariadb-waitpid.1 same weirdness as elsewhere, mariadb*.1 man pages are symlinks to mysql*.1 manpages. Should be vice versa. > usr/share/man/man1/mysql_find_rows.1 > usr/share/man/man1/mysql_fix_extensions.1 > usr/share/man/man1/mysql_waitpid.1 > diff --git a/debian/mariadb-client-10.5.links b/debian/mariadb-client-10.5.links > index 5d966575b76..d1bdfcfa3c8 100644 > --- a/debian/mariadb-client-10.5.links > +++ b/debian/mariadb-client-10.5.links > @@ -1,21 +1,19 @@ > -usr/bin/mysql_find_rows usr/bin/mariadb-find-rows > -usr/bin/mysql_fix_extensions usr/bin/mariadb-fix-extensions > -usr/bin/mysql_plugin usr/bin/mariadb-plugin > -usr/bin/mysql_waitpid usr/bin/mariadb-waitpid > -usr/bin/mysqlaccess usr/bin/mariadb-access > -usr/bin/mysqladmin usr/bin/mariadb-admin > -usr/bin/mysqlbinlog usr/bin/mariadb-binlog you removed mysqlbinlog, but added mariadb-binlog to the server package. Intentional? same for mysql_plugin/mariadb-plugin. UPD: mysqlbinlog was in the server package, while mariadb-binlog symlink was in the client package. So your change is correct, both will be in the server package now. Good. same for mysql_plugin/mariadb-plugin. > -usr/bin/mysqlcheck usr/bin/mariadb-analyze > -usr/bin/mysqlcheck usr/bin/mariadb-optimize > diff --git a/debian/mariadb-plugin-rocksdb.install b/debian/mariadb-plugin-rocksdb.install > index 80987612c30..0fc868a0721 100644 > --- a/debian/mariadb-plugin-rocksdb.install > +++ b/debian/mariadb-plugin-rocksdb.install > @@ -2,6 +2,5 @@ etc/mysql/conf.d/rocksdb.cnf etc/mysql/mariadb.conf.d > usr/bin/myrocks_hotbackup > usr/bin/mysql_ldb you didn't replace mysql_ldb with mariadb-ldb? > usr/lib/mysql/plugin/ha_rocksdb.so > -usr/share/man/man1/mariadb-ldb.1 > usr/share/man/man1/myrocks_hotbackup.1 > usr/share/man/man1/mysql_ldb.1 > diff --git a/debian/mariadb-server-10.5.install b/debian/mariadb-server-10.5.install > index 4a860ff57df..38d9f984276 100644 > --- a/debian/mariadb-server-10.5.install > +++ b/debian/mariadb-server-10.5.install > @@ -14,22 +14,23 @@ usr/bin/aria_pack > usr/bin/aria_read_log > usr/bin/galera_new_cluster > usr/bin/galera_recovery > +usr/bin/mariadb-binlog > +usr/bin/mariadb-convert-table-format > +usr/bin/mariadb-hotcopy > +usr/bin/mariadb-plugin > +usr/bin/mariadb-secure-installation > usr/bin/mariadb-service-convert > +usr/bin/mariadb-setpermission > +usr/bin/mariadb-show > +usr/bin/mariadb-tzinfo-to-sql > +usr/bin/mariadbd-multi > +usr/bin/mariadbd-safe > +usr/bin/mariadbd-safe duplicate line ^^^ > usr/bin/msql2mysql > usr/bin/myisam_ftdump > usr/bin/myisamchk > usr/bin/myisamlog > usr/bin/myisampack > -usr/bin/mysql_convert_table_format > -usr/bin/mysql_plugin > -usr/bin/mysql_secure_installation > -usr/bin/mysql_setpermission > -usr/bin/mysql_tzinfo_to_sql > -usr/bin/mysqlbinlog > -usr/bin/mysqld_multi > -usr/bin/mysqld_safe > -usr/bin/mysqld_safe_helper you don't install mariadbd-safe-helper. Perhaps that's where your duplicate line comes from, it's not a duplicate, it's truncated :) > -usr/bin/mysqlhotcopy > usr/bin/perror > usr/bin/replace > usr/bin/resolve_stack_dump > diff --git a/debian/mariadb-server-10.5.links b/debian/mariadb-server-10.5.links > index f2d97460371..e3a2d68541a 100644 > --- a/debian/mariadb-server-10.5.links > +++ b/debian/mariadb-server-10.5.links > @@ -1,9 +1,20 @@ > -usr/bin/mysql_convert_table_format usr/bin/mariadb-convert-table-format > -usr/bin/mysql_secure_installation usr/bin/mariadb-secure-installation > -usr/bin/mysql_setpermission usr/bin/mariadb-setpermission > -usr/bin/mysql_tzinfo_to_sql usr/bin/mariadb-tzinfo-to-sql > -usr/bin/mysqld_multi usr/bin/mariadbd-multi > -usr/bin/mysqld_safe usr/bin/mariadbd-safe > -usr/bin/mysqld_safe_helper usr/bin/mariadbd-safe-helper > -usr/bin/mysqlhotcopy usr/bin/mariadb-hotcopy > -usr/bin/mysqlshow usr/bin/mariadb-show > +usr/bin/mariadb-binlog usr/bin/mysqlbinlog > +usr/bin/mariadb-convert-table-format usr/bin/mysql_convert_table_format > +usr/bin/mariadb-hotcopy usr/bin/mysqlhotcopy > +usr/bin/mariadb-plugin usr/bin/mysql_plugin > +usr/bin/mariadb-secure-installation usr/bin/mysql_secure_installation > +usr/bin/mariadb-setpermission usr/bin/mysql_setpermission > +usr/bin/mariadb-show /usr/bin/mysqlshow this is the only line in your whole patch where you symlink by the absolute path. Looks like a typo. > +usr/bin/mariadb-tzinfo-to-sql usr/bin/mysql_tzinfo_to_sql > +usr/bin/mariadbd-multi usr/bin/mysqld_multi > +usr/bin/mariadbd-safe usr/bin/mysqld_safe > +usr/bin/mariadbd-safe-helper usr/bin/mysqld_safe_helper > diff --git a/libmariadb b/libmariadb > index 3be5897c334..ca68b114ba7 160000 > --- a/libmariadb > +++ b/libmariadb > @@ -1 +1 @@ > -Subproject commit 3be5897c3346639fa6d7195480d93108798c4917 > +Subproject commit ca68b114ba7e882e8abe8888204d675c4d8751f8 This is clearly wrong, please make sure you've removed it from your commit before pushing! > diff --git a/scripts/CMakeLists.txt b/scripts/CMakeLists.txt > index 7be46ac1985..350fbd14cf6 100644 > --- a/scripts/CMakeLists.txt > +++ b/scripts/CMakeLists.txt > @@ -244,8 +232,20 @@ SET(mysqlaccess_COMPONENT COMPONENT Client) > SET(mysql_find_rows_COMPONENT COMPONENT Client) > SET(mytop_COMPONENT Mytop) > > +MACRO(INSTALL_LINK old new destination component) > + EXECUTE_PROCESS( > + COMMAND ${CMAKE_COMMAND} -E create_symlink ${old} ${new} > + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} > + ) > + INSTALL( > + PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${new} > + DESTINATION ${destination} > + COMPONENT ${component} > + ) > +ENDMACRO() > + > IF(WIN32) > - # On Windows, some .sh and some .pl.in files are configured > + # On Windows, some .sh and some .pl.in files are configured > # The resulting files will have .pl extension (those are perl scripts) > > # Input files with pl.in extension > @@ -296,60 +296,73 @@ ELSE() > # On Unix, most of the files end up in the bin directory > SET(BIN_SCRIPTS > msql2mysql > - mysql_config > - mysql_setpermission > - mysql_secure_installation > - mysqlaccess > - mysql_convert_table_format > - mysql_find_rows > + mariadb-setpermission > + mariadb-secure-installation > + mariadb-access > + mariadb-convert-table-format > + mariadb-find-rows > mytop > - mysqlhotcopy > + mariadb-hotcopy > + mariadb-install-db > ${SERVER_SCRIPTS} > ${WSREP_SCRIPTS} > ${SYSTEMD_SCRIPTS} > ) > + > FOREACH(file ${BIN_SCRIPTS}) > - IF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh) > - CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh > - ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) > - ELSEIF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${file}) > - CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file} > - ${CMAKE_CURRENT_BINARY_DIR}/${file} COPYONLY) > - ELSE() > - MESSAGE(FATAL_ERROR "Can not find ${file}.sh or ${file} in " > - "${CMAKE_CURRENT_SOURCE_DIR}" ) > - ENDIF() > - # TODO: The following EXECUTE could be redundant as INSTALL_SCRIPT > - # macro does an INSTALL(PROGRAMS ..) that automatically sets +x on > - # the executable. > - EXECUTE_PROCESS(COMMAND chmod +x ${CMAKE_CURRENT_BINARY_DIR}/${file}) > - IF(NOT ${file}_COMPONENT) > - SET(${file}_COMPONENT Server) > - ENDIF() > - INSTALL_SCRIPT( > - ${CMAKE_CURRENT_BINARY_DIR}/${file} > - DESTINATION ${INSTALL_BINDIR} > - COMPONENT ${${file}_COMPONENT} > - ) > + # set name of executable > + list(FIND MARIADB_SYMLINK_FROMS ${file} _index) > + > + if(${_index} GREATER -1) > + list(GET MARIADB_SYMLINK_TOS ${_index} binname) > + else() > + set(binname ${file}) > + endif() 1. why are you not using GET_SYMLINK ? 2. Why INSTALL_SYMLINK here instead of the old CREATE_MARIADB_SYMLINK ? You avoid creating new targets for script symlinks, but who cares about that? > + > + if(NOT "${binname}" STREQUAL "") how can ${binname} be "" ? > + IF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${binname}.sh) > + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${binname}.sh > + ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) > + ELSEIF(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${binname}) > + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${binname} > + ${CMAKE_CURRENT_BINARY_DIR}/${file} COPYONLY) > + ELSE() > + MESSAGE(FATAL_ERROR "Can not find ${binname}.sh or ${binname} in " > + "${CMAKE_CURRENT_SOURCE_DIR}" ) > + ENDIF() > + > + IF(NOT ${file}_COMPONENT) > + SET(${file}_COMPONENT Server) > + ENDIF() > + INSTALL_SCRIPT( > + ${CMAKE_CURRENT_BINARY_DIR}/${file} > + DESTINATION ${INSTALL_BINDIR} > + COMPONENT ${${file}_COMPONENT} > + ) > + > + # Create symlink > + if (NOT ${binname} STREQUAL ${file}) > + INSTALL_LINK(${file} ${binname} ${INSTALL_BINDIR} ${${file}_COMPONENT}) > + > + # Place mysql_install_db symlink also in scripts > + if (${binname} STREQUAL "mysql_install_db") > + SET(bindir ${bindir1}) > + SET(prefix ${prefix1}) > + SET(sbindir ${sbindir1}) > + SET(scriptdir ${scriptdir1}) > + SET(libexecdir ${libexecdir1}) > + SET(pkgdatadir ${pkgdatadir1}) > + SET(pkgplugindir ${pkgplugindir1}) > + SET(localstatedir ${localstatedir1}) > + CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/mysql_install_db.sh ${CMAKE_CURRENT_BINARY_DIR}/mariadb-install-db ESCAPE_QUOTES @ONLY) > + INSTALL(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${file} DESTINATION ${INSTALL_SCRIPTDIR} COMPONENT Server) > + INSTALL(PROGRAMS ${CMAKE_CURRENT_BINARY_DIR}/${binname} DESTINATION ${INSTALL_SCRIPTDIR} COMPONENT Server) I don't understand that at all. 1. What does the comment "Place mysql_install_db symlink also in scripts" even mean? 2. you CONFIGURE_FILE mariadb-install-db twice. 3. Why two INSTALL(PROGRAM ? 4. You set all bindir/etc variables for mysql_install_db and it will affect CONFIGURE_FILE for all following scripts 5. why do you even compare ${binname} with mysql_install_db and not ${file} with mariadb-install-db ? > + endif() > + endif() > + endif() we customary always use uppercase in all cmake keywords. > ENDFOREACH() > > - SET (wsrep_sst_rsync_wan ${CMAKE_CURRENT_BINARY_DIR}/wsrep_sst_rsync_wan) > - ADD_CUSTOM_COMMAND( > - OUTPUT ${wsrep_sst_rsync_wan} > - COMMAND ${CMAKE_COMMAND} ARGS -E create_symlink > - wsrep_sst_rsync > - wsrep_sst_rsync_wan > - ) > - ADD_CUSTOM_TARGET(symlink_wsrep_sst_rsync > - ALL > - DEPENDS ${wsrep_sst_rsync_wan} > - ) > - INSTALL( > - FILES ${wsrep_sst_rsync_wan} > - DESTINATION ${INSTALL_BINDIR} > - COMPONENT Server > - ) > - > + INSTALL_LINK(wsrep_sst_rsync wsrep_sst_rsync_wan ${INSTALL_BINDIR} Server) > FOREACH(file ${WSREP_SOURCE}) > CONFIGURE_FILE(${CMAKE_CURRENT_SOURCE_DIR}/${file}.sh > ${CMAKE_CURRENT_BINARY_DIR}/${file} ESCAPE_QUOTES @ONLY) > diff --git a/win/packaging/ca/CMakeLists.txt b/win/packaging/ca/CMakeLists.txt > index 99dc7da01fb..326bab47de4 100644 > --- a/win/packaging/ca/CMakeLists.txt > +++ b/win/packaging/ca/CMakeLists.txt I didn't review windows packaging, it's between you and wlad. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 5365db70cb8: Improve update handler (long unique keys on blobs)
by Sergei Golubchik 16 Mar '20

16 Mar '20

Hi, Michael! On Feb 29, Michael Widenius wrote: > commit 5365db70cb8 > Author: Michael Widenius <monty(a)mariadb.com> > Date: Mon Jan 13 18:30:13 2020 +0200 > > Improve update handler (long unique keys on blobs) > > MDEV-21606 Improve update handler (long unique keys on blobs) > MDEV-21470 MyISAM and Aria start_bulk_insert doesn't work with long unique > MDEV-21606 Bug fix for previous version of this code > MDEV-21819 2 Assertion `inited == NONE || update_handler != this' > > - Move update_handler from TABLE to handler > - Move out initialization of update handler from ha_write_row() to > prepare_for_insert() > - Fixed that INSERT DELAYED works with update handler > - Give an error if using long unique with an autoincrement column > - Added handler function to check if table has long unique hash indexes > - Disable write cache in MyISAM and Aria when using update_handler as > if cache is used, the row will not be inserted until end of statement > and update_handler would not find conflicting rows. > - Removed not used handler argument from > check_duplicate_long_entries_update() > - Syntax cleanups > - Indentation fixes > - Don't use single character indentifiers for arguments > > squash! 1ca2f0871ecfb550268ffc1a5cc23d7043d6b855 you may want to remove this line from the commit comment > > diff --git a/mysql-test/main/long_unique.result b/mysql-test/main/long_unique.result > index a4955b3e7b5..fee8e7721bf 100644 > --- a/mysql-test/main/long_unique.result > +++ b/mysql-test/main/long_unique.result > @@ -1477,4 +1477,28 @@ id select_type table type possible_keys key key_len ref rows Extra > SELECT t2.b FROM t1 JOIN t2 ON t1.d = t2.f WHERE t2.pk >= 20; > b > drop table t1,t2; > +# > +# MDEV-21470 MyISAM start_bulk_insert doesn't work with long unique > +# > +CREATE TABLE t1 (a INT, b BLOB) ENGINE=MyISAM; > +INSERT INTO t1 VALUES (1,'foo'),(2,'bar'); > +CREATE TABLE t2 (c BIT, d BLOB, UNIQUE(d)) ENGINE=MyISAM; > +INSERT INTO t2 SELECT * FROM t1; > +Warnings: > +Warning 1264 Out of range value for column 'c' at row 2 > +DROP TABLE t1, t2; > +# > +# MDEV-19338 Using AUTO_INCREMENT with long unique > +# > +CREATE TABLE t1 (pk INT, a TEXT NOT NULL DEFAULT '', PRIMARY KEY (pk), b INT AUTO_INCREMENT, UNIQUE(b), UNIQUE (a,b)) ENGINE=myisam; > +ERROR HY000: Function or expression 'AUTO_INCREMENT' cannot be used in the UNIQUE clause of `a` Quite confusing error message :( I understand where it comes from, but for a user, I'm afraid, it just won't make any sense. A reasonable message could be, for example, AUTO_INCREMENT column %`s cannot be used in the UNIQUE index %`s > +# > +# MDEV-21819 Assertion `inited == NONE || update_handler != this' > +# failed in handler::ha_write_row > +# > +CREATE OR REPLACE TABLE t1 (a INT, b BLOB, s DATE, e DATE, PERIOD FOR app(s,e), UNIQUE(b)) ENGINE=MyISAM PARTITION BY HASH(a) PARTITIONS 2; > +INSERT INTO t1 VALUES (1,'foo','2022-01-01', '2025-01-01'); > +DELETE FROM t1 FOR PORTION OF app FROM '2023-01-01' TO '2024-01-01'; > +ERROR 23000: Duplicate entry 'foo' for key 'b' > +DROP TABLE t1; > set @@GLOBAL.max_allowed_packet= @allowed_packet; > diff --git a/mysql-test/suite/versioning/r/long_unique.result b/mysql-test/suite/versioning/r/long_unique.result > new file mode 100644 > index 00000000000..da07bc66e22 > --- /dev/null > +++ b/mysql-test/suite/versioning/r/long_unique.result > @@ -0,0 +1,8 @@ > +# > +# Assertion `inited == NONE || update_handler != this' failed in > +# handler::ha_write_row > +# > +CREATE TABLE t1 (f VARCHAR(4096), s DATE, e DATE, PERIOD FOR app(s,e), UNIQUE(f)) ENGINE=MyISAM; > +INSERT INTO t1 VALUES ('foo', '2023-08-30', '2025-07-09'),('bar', '2021-01-01', '2021-12-31'); > +DELETE FROM t1 FOR PORTION OF app FROM '2023-08-29' TO '2025-07-01'; > +DROP TABLE t1; you've had almost the same test (MDEV-21819) in main/long_unique.test it's a bit strange to have to very similar tests in different suites I suggest to move MDEV-21819 test from main/long_unique.test to this file. and then move this whole file from versioning suite to the period suite. > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index 73191907aac..c7e61864366 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -4340,6 +4339,10 @@ int ha_partition::write_row(const uchar * buf) > } > m_last_part= part_id; > DBUG_PRINT("info", ("Insert in partition %u", part_id)); > + if (table->s->long_unique_table && > + m_file[part_id]->update_handler == m_file[part_id] && inited == RND) > + m_file[part_id]->prepare_for_insert(0); This looks quite incomprehensible. If the table has a long unique key and the update_handler is the handler itself you _prepare for insert_ ??? if it's an insert it should've been prepared earlier, like, normally for an insert, and it don't have to do anything with long uniques. What you actually do here (as I've found out looking firther in the patch) you prepare update_handler. That is "prepare_for_insert" is VERY confusing here, it sounds like it prepares for a normal insert and should be called at the beginning of mysql_insert(). And it's nothing like that. It would remove half of the questions if you'd call it prepare_update_handler() or, better, prepare_auxiliary_handler() (because it'll be used not only for long uniques) or something like that. Still, some questions are left: 1. why partitioning needs special code for that? 2. when a handler itself is its update_handler AND inited==RND? the handler itself can be the update_handler on INSERT, but then inited is NONE Is it UPDATE with system versioning? > + > start_part_bulk_insert(thd, part_id); > > tmp_disable_binlog(thd); /* Do not replicate the low-level changes. */ > diff --git a/sql/handler.cc b/sql/handler.cc > index 0700b1571e5..1e3f987b4e5 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -2648,6 +2649,39 @@ handler *handler::clone(const char *name, MEM_ROOT *mem_root) > return NULL; > } > > +/** > + @brief clone of current handler. you generally don't have to write @brief in these cases. Just /** clone of current handler. Creates a clone of handler used in update for unique hash key. */ > + Creates a clone of handler used in update for > + unique hash key. > +*/ > +bool handler::clone_handler_for_update() > +{ > + handler *tmp; > + DBUG_ASSERT(table->s->long_unique_table); > + > + if (update_handler != this) > + return 0; // Already done > + if (!(tmp= clone(table->s->normalized_path.str, table->in_use->mem_root))) > + return 1; > + update_handler= tmp; > + update_handler->ha_external_lock(table->in_use, F_RDLCK); Looks strange. Why F_RDLCK if it's an *update* handler? This definitely needs a comment, why you're using read lock for updates. > + return 0; > +} > + > +/** > + @brief Deletes update handler object > +*/ > +void handler::delete_update_handler() > +{ > + if (update_handler != this) > + { > + update_handler->ha_external_lock(table->in_use, F_UNLCK); > + update_handler->ha_close(); > + delete update_handler; > + } > + update_handler= this; > +} > + > LEX_CSTRING *handler::engine_name() > { > return hton_name(ht); > @@ -6481,6 +6515,8 @@ int handler::ha_reset() > DBUG_ASSERT(inited == NONE); > /* reset the bitmaps to point to defaults */ > table->default_column_bitmaps(); > + if (update_handler != this) > + delete_update_handler(); you already have an if() inside the delete_update_handler(). Either remove it from here or from there. > pushed_cond= NULL; > tracker= NULL; > mark_trx_read_write_done= 0; > @@ -6671,10 +6717,35 @@ static int check_duplicate_long_entries_update(TABLE *table, handler *h, uchar * > } > } > } > - exit: > - return error; > + return 0; > +} > + > + > +/** > + Do all initialization needed for insert > + > + @param force_update_handler Set to TRUE if we should always create an > + update handler. Needed if we don't know if we > + are going to do inserted while a scan is in "going to do inserts" > + progress. > +*/ > + > +int handler::prepare_for_insert(bool force_update_handler) > +{ > + /* Preparation for unique of blob's */ > + if (table->s->long_unique_table && (inited == RND || force_update_handler)) > + { > + /* > + When doing a scan we can't use the same handler to check > + duplicate rows. Create a new temporary one > + */ > + if (clone_handler_for_update()) > + return 1; > + } > + return 0; > } > > + > int handler::ha_write_row(const uchar *buf) > { > int error; > @@ -6977,6 +7044,11 @@ void handler::set_lock_type(enum thr_lock_type lock) > table->reginfo.lock_type= lock; > } > > +bool handler::has_long_unique() > +{ > + return table->s->long_unique_table; > +} I don't see why you need it for Aria. It can always check table->s->long_unique_table without an extra call > + > #ifdef WITH_WSREP > /** > @details > diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc > index 6a4ce266af2..e55ae4f97e6 100644 > --- a/sql/sql_delete.cc > +++ b/sql/sql_delete.cc > @@ -752,6 +752,10 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, > && !table->versioned() > && table->file->has_transactions(); > > + if (table->versioned(VERS_TIMESTAMP) || > + (table_list->has_period() && !portion_of_time_through_update)) > + table->file->prepare_for_insert(1); Ok, now I think that your idea of the default update_handler=this is rather fragile. If the default is NULL, than we can put in many places DBUG_ASSERT(update_handler != NULL); to make sure it was initialized when needed. Now we cannot do it. And there are many places in the code and non-trivial conditions when the update_handler has to be initialized and these places and conditions will change in the future. An assert would be helpful here. > + > THD_STAGE_INFO(thd, stage_updating); > while (likely(!(error=info.read_record())) && likely(!thd->killed) && > likely(!thd->is_error())) > diff --git a/sql/table.h b/sql/table.h > index 6ce92ee048e..1a0b9514d23 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -1182,6 +1181,7 @@ struct TABLE > /* Map of keys dependent on some constraint */ > key_map constraint_dependent_keys; > KEY *key_info; /* data of keys in database */ > + KEY_PART_INFO *base_key_part; /* Where key parts are stored */ again, this is *only* needed for INSERT DELAYED that very few people ever use. It should be in some INSERT DELAYED specific data structure, not in the common TABLE. By the way, why cannot you get the list of keyparts from table->key_info->key_part ? > > Field **field; /* Pointer to fields */ > Field **vfield; /* Pointer to virtual fields*/ > diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc > index 38091dae0ba..39147396359 100644 > --- a/storage/myisam/ha_myisam.cc > +++ b/storage/myisam/ha_myisam.cc > @@ -1740,7 +1740,7 @@ void ha_myisam::start_bulk_insert(ha_rows rows, uint flags) > enable_indexes() failed, thus it's essential that indexes are > disabled ONLY for an empty table. > */ > - if (file->state->records == 0 && can_enable_indexes && > + if (file->state->records == 0 && can_enable_indexes && !has_long_unique() && > (!rows || rows >= MI_MIN_ROWS_TO_DISABLE_INDEXES)) why do you disable bulk inserts for long uniques? > { > if (file->open_flag & HA_OPEN_INTERNAL_TABLE) > diff --git a/sql/table.cc b/sql/table.cc > index 718efa5767c..18a942964c3 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -1241,31 +1241,46 @@ bool parse_vcol_defs(THD *thd, MEM_ROOT *mem_root, TABLE *table, > Item *list_item; > KEY *key= 0; > uint key_index, parts= 0; > + KEY_PART_INFO *key_part= table->base_key_part; > + > for (key_index= 0; key_index < table->s->keys; key_index++) > { > - key=table->key_info + key_index; > - parts= key->user_defined_key_parts; > - if (key->key_part[parts].fieldnr == field->field_index + 1) > - break; > + /* > + We have to use key from share as this function may have changed > + table->key_info if it was ever invoked before. This could happen > + in case of INSERT DELAYED. > + */ > + key= table->s->key_info + key_index; > + if (key->algorithm == HA_KEY_ALG_LONG_HASH) > + { > + parts= key->user_defined_key_parts; > + if (key_part[parts].fieldnr == field->field_index + 1) > + break; > + key_part++; > + } Here, again, you've basically duplicated the logic of how long uniques are represented in the keys and keyparts. All for the sake of INSERT DELAYED. I'd really prefer INSERT DELAYED problems to be solved inside the Delayed_insert class. And it can use setup_keyinfo_hash() and re_setup_keyinfo_hash() functions to avoid duplicating the logic. > + key_part+= key->ext_key_parts; > } > - if (!key || key->algorithm != HA_KEY_ALG_LONG_HASH) > + if (key_index == table->s->keys) > goto end; > - KEY_PART_INFO *keypart; > - for (uint i=0; i < parts; i++) > + > + /* Correct the key & key_parts if this function has been called before */ > + key= table->key_info + key_index; > + key->key_part= key_part; > + > + for (uint i=0; i < parts; i++, key_part++) > { > - keypart= key->key_part + i; > - if (keypart->key_part_flag & HA_PART_KEY_SEG) > + if (key_part->key_part_flag & HA_PART_KEY_SEG) > { > - int length= keypart->length/keypart->field->charset()->mbmaxlen; > + int length= key_part->length/key_part->field->charset()->mbmaxlen; > list_item= new (mem_root) Item_func_left(thd, > - new (mem_root) Item_field(thd, keypart->field), > + new (mem_root) Item_field(thd, key_part->field), > new (mem_root) Item_int(thd, length)); > list_item->fix_fields(thd, NULL); > - keypart->field->vcol_info= > - table->field[keypart->field->field_index]->vcol_info; > + key_part->field->vcol_info= > + table->field[key_part->field->field_index]->vcol_info; > } > else > - list_item= new (mem_root) Item_field(thd, keypart->field); > + list_item= new (mem_root) Item_field(thd, key_part->field); > field_list->push_back(list_item, mem_root); > } > Item_func_hash *hash_item= new(mem_root)Item_func_hash(thd, *field_list); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] [Commits] 70b0c838fad: MDEV-21922: Allow packing addon fields even if they don't honour max_length_for_sort_data
by Sergey Petrunia 12 Mar '20

12 Mar '20

Hi Varun, * The patch needs a testcase * Please check the comment at https://jira.mariadb.org/browse/MDEV-21922 , and add a comment in filesort_use_addons() about the use of upper bound for computations. * Ok to push after the above is addressed. On Thu, Mar 12, 2020 at 05:11:45PM +0530, Varun wrote: > revision-id: 70b0c838fada8e02d4ce9525611d6f4aeadf56d4 (mariadb-10.5.0-367-g70b0c838fad) > parent(s): 9d7ed94f6a526748eff29dae2939a3fd341f118b > author: Varun Gupta > committer: Varun Gupta > timestamp: 2020-03-12 14:41:03 +0530 > message: > > MDEV-21922: Allow packing addon fields even if they don't honour max_length_for_sort_data > > Addon fields will be packed if the length of addon fields is greater > than max_length_for_sort_data. > > --- > sql/filesort.cc | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/sql/filesort.cc b/sql/filesort.cc > index 1e19d492220..7b4e0bc31cd 100644 > --- a/sql/filesort.cc > +++ b/sql/filesort.cc > @@ -139,8 +139,6 @@ void Sort_param::try_to_pack_addons(ulong max_length_for_sort_data) > return; > > const uint sz= Addon_fields::size_of_length_field; > - if (rec_length + sz > max_length_for_sort_data) > - return; > > // Heuristic: skip packing if potential savings are less than 10 bytes. > if (m_packable_length < (10 + sz)) > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] 5d0e4ce: MySQL support added
by andrei.elkin＠pp.inet.fi 12 Mar '20

12 Mar '20

Привет, Сергей! О чем этот "суппорт", не обьяснишь одной фразой? Спасибо! Андрей psergey <sergey(a)mariadb.com> writes: > revision-id: 5d0e4ce291ae940feaa8398435158dc56a3db3c4 () > parent(s): d504887718112e211544beca0e6651d5477466e1 > author: Sergei Petrunia > committer: Sergei Petrunia > timestamp: 2020-03-12 10:26:06 +0300 > message: > > MySQL support added > > --- > .gitignore | 1 + > filesort-bench1/06-make-varchar-bench.sh | 30 +++++++++++++++-- > prepare-server.sh | 11 ++++-- > setup-server/setup-mariadb-current.sh | 6 ++-- > setup-server/setup-mysql-8.0.sh | 58 ++++++++++++++++++++++++++++++++ > 5 files changed, 98 insertions(+), 8 deletions(-) > > diff --git a/.gitignore b/.gitignore > index fcd5175..3bccc18 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -23,3 +23,4 @@ my-10.5-old.cnf > mysql-boost > mysql-8.0 > mysql8-data > +mysql-8.0-data.clean/ > diff --git a/filesort-bench1/06-make-varchar-bench.sh b/filesort-bench1/06-make-varchar-bench.sh > index 85afcfd..21da087 100644 > --- a/filesort-bench1/06-make-varchar-bench.sh > +++ b/filesort-bench1/06-make-varchar-bench.sh > @@ -29,6 +29,17 @@ create table test_run_queries ( > test_time_ms bigint, > sort_merge_passes int > ); > + > +drop view if exists session_status; > + > +set @var= IF(version() like '%8.0%', > + 'create view session_status as select * from performance_schema.session_status', > + 'create view session_status as select * from information_schema.session_status'); > + > +prepare s from @var; > +execute s; > + > + > END > > ### > @@ -55,7 +66,18 @@ create table $test_table_name ( > char_field varchar($varchar_size) character set utf8, b int > ) engine=myisam; > > -insert into $rand_table_name select 1+floor(rand() * @n_countries) from seq_1_to_$table_size; > +drop table if exists ten, one_k; > +create table ten(a int); > +insert into ten values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); > + > +create table one_k(a int); > +insert into one_k select A.a + B.a* 10 + C.a * 100 from ten A, ten B, ten C; > + > +set @a=0; > +insert into $rand_table_name > +select 1+floor(rand() * @n_countries) > +from > + (select @a:=@a+1 from one_k A, one_k B, one_k C limit $table_size) T; > insert into $test_table_name > select > (select Name from Country where id=T.a), 1234 > @@ -63,13 +85,15 @@ from $rand_table_name T ; > > drop table $rand_table_name; > analyze table $test_table_name; > +select count(*) from $test_table_name; > +show create table $test_table_name; > END > > for i in 1 2 3 4 5 6 7 8 9 10 ; do > > ### query_start.sql here: > cat <<END > -select variable_value into @query_start_smp from information_schema.session_status where variable_name like 'sort_merge_passes'; > +select variable_value into @query_start_smp from session_status where variable_name like 'sort_merge_passes'; > select current_timestamp(6) into @query_start_time; > END > ### > @@ -87,7 +111,7 @@ echo $QUERY > cat << END > set @test_name='$TEST_NAME'; > set @query_time_ms= timestampdiff(microsecond, @query_start_time, current_timestamp(6))/1000; > -select variable_value into @query_end_smp from information_schema.session_status where variable_name like 'sort_merge_passes'; > +select variable_value into @query_end_smp from session_status where variable_name like 'sort_merge_passes'; > set @query_merge_passes = @query_end_smp - @query_start_smp; > insert into test_run_queries > (table_size, varchar_size, test_ts, test_time_ms, sort_merge_passes) > diff --git a/prepare-server.sh b/prepare-server.sh > index 01dbeba..d4da8db 100755 > --- a/prepare-server.sh > +++ b/prepare-server.sh > @@ -38,6 +38,11 @@ if [ ! -d $SERVERNAME ]; then > exit 1 > fi > > +if [ ! -f $SERVERNAME-vars.sh ]; then > + echo "Can't find settings file $SERVERNAME-vars.sh." > + exit 1 > +fi > + > if [[ $USE_RAMDISK ]] ; then > echo " Using /dev/shm for data dir" > fi > @@ -49,6 +54,8 @@ sleep 5 > > DATA_DIR=$SERVERNAME-data > > +source ${SERVERNAME}-vars.sh > + > if [[ $RECOVER ]] ; then > echo "Recovering the existing datadir" > else > @@ -64,7 +71,7 @@ else > fi > > #exit 0; > -./$SERVERNAME/sql/mysqld --defaults-file=./my-${SERVERNAME}.cnf & > +$MYSQLD --defaults-file=./my-${SERVERNAME}.cnf & > > > server_attempts=0 > @@ -72,7 +79,7 @@ server_attempts=0 > while true ; do > client_attempts=0 > while true ; do > - ./$SERVERNAME/client/mysql --defaults-file=./my-${SERVERNAME}.cnf -uroot -e "create database sbtest" > + $MYSQL $MYSQL_ARGS -e "select 1" > > if [ $? -eq 0 ]; then > break > diff --git a/setup-server/setup-mariadb-current.sh b/setup-server/setup-mariadb-current.sh > index 1b49ef0..b6bc417 100755 > --- a/setup-server/setup-mariadb-current.sh > +++ b/setup-server/setup-mariadb-current.sh > @@ -85,16 +85,16 @@ innodb_buffer_pool_size=8G > > EOF > > -cat > mysql-vars.sh <<EOF > +cat > $DIRNAME-vars.sh <<EOF > MYSQL="`pwd`/$DIRNAME/client/mysql" > +MYSQLD="`pwd`/$DIRNAME/sql/mysqld" > MYSQLSLAP="`pwd`/$DIRNAME/client/mysqlslap" > MYSQL_SOCKET="--socket=$SOCKETNAME" > MYSQL_USER="-uroot" > MYSQL_ARGS="\$MYSQL_USER \$MYSQL_SOCKET" > EOF > > -source mysql-vars.sh > -cp mysql-vars.sh $DIRNAME-vars.sh > +source $DIRNAME-vars.sh > > ( > cd $HOMEDIR/$DIRNAME/sql > diff --git a/setup-server/setup-mysql-8.0.sh b/setup-server/setup-mysql-8.0.sh > new file mode 100644 > index 0000000..ffd9987 > --- /dev/null > +++ b/setup-server/setup-mysql-8.0.sh > @@ -0,0 +1,58 @@ > +#!/bin/bash > + > +HOMEDIR=`pwd` > + > +BRANCH=8.0 > +SERVER_VERSION=8.0 > +DIRNAME="mysql-$SERVER_VERSION" > + > +git clone --branch $BRANCH --depth 1 https://github.com/mysql/mysql-server.git $DIRNAME > + > +cd mysql-$SERVER_VERSION > +cmake . -DCMAKE_BUILD_TYPE=RelWithDebInfo -DDOWNLOAD_BOOST=1 -DWITH_BOOST=$HOMEDIR/mysql-boost \ > + -DENABLE_DOWNLOADS=1 -DFORCE_INSOURCE_BUILD=1 -DWITH_UNIT_TESTS=0 > + > +make -j8 > + > +cd mysql-test > +perl ./mysql-test-run alias > +cp -r var/data $HOMEDIR/$DIRNAME-data > +cp -r var/data $HOMEDIR/$DIRNAME-data.clean > +cd .. > + > + > +source_dir=`pwd` > +socket_name="`basename $source_dir`.sock" > +SOCKETNAME="/tmp/$socket_name" > + > +cat > $HOMEDIR/my-$DIRNAME.cnf <<EOF > + > +[mysqld] > +datadir=$HOMEDIR/$DIRNAME-data > + > +tmpdir=/tmp > +port=3320 > +socket=$SOCKETNAME > +#binlog-format=row > +gdb > +lc_messages_dir=../share > +server-id=12 > +bind-address=0.0.0.0 > +log-error > +secure_file_priv= > +innodb_buffer_pool_size=4G > +EOF > + > +cat > $DIRNAME-vars.sh <<EOF > +MYSQL="`pwd`/$DIRNAME/bin/mysql" > +MYSQLD="`pwd`/$DIRNAME/bin/mysqld" > +MYSQLSLAP="`pwd`/$DIRNAME/bin/mysqlslap" > +MYSQL_SOCKET="--socket=$SOCKETNAME" > +MYSQL_USER="-uroot" > +MYSQL_ARGS="\$MYSQL_USER \$MYSQL_SOCKET" > +EOF > + > +source $DIRNAME-vars.sh > + > +$MYSQLD --defaults-file=$HOMEDIR/my-$DIRNAME.cnf & > + > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

[Maria-developers] (no subject)
by Badrul Chowdhury 11 Mar '20

11 Mar '20

-- Cheers, Badrul

1 0

Re: [Maria-developers] bbe056ac3fa: support NULL fields in key
by Sergei Golubchik 11 Mar '20

11 Mar '20

Hi, Nikita! On Mar 10, Nikita Malyavin wrote: > revision-id: bbe056ac3fa (mariadb-10.5.0-273-gbbe056ac3fa) > parent(s): 7a5d3316805 > author: Nikita Malyavin <nikitamalyavin(a)gmail.com> > committer: Nikita Malyavin <nikitamalyavin(a)gmail.com> > timestamp: 2020-03-11 00:46:24 +1000 > message: > > support NULL fields in key > > --- > mysql-test/suite/period/r/overlaps.result | 18 ++++++++++++++++-- > mysql-test/suite/period/t/overlaps.test | 16 +++++++++++++++- > sql/handler.cc | 18 ++++++++++++++---- > sql/sql_table.cc | 29 +++++++---------------------- > 4 files changed, 52 insertions(+), 29 deletions(-) > > diff --git a/mysql-test/suite/period/r/overlaps.result b/mysql-test/suite/period/r/overlaps.result > index cf980afd7f0..e52b21496b5 100644 > --- a/mysql-test/suite/period/r/overlaps.result > +++ b/mysql-test/suite/period/r/overlaps.result > @@ -104,16 +104,30 @@ show create table t; > Table Create Table > t CREATE TABLE `t` ( > `id` int(11) NOT NULL, > - `u` int(11) NOT NULL, > + `u` int(11) DEFAULT NULL, > `s` date NOT NULL, > `e` date NOT NULL, > PERIOD FOR `p` (`s`, `e`), > PRIMARY KEY (`id`,`p` WITHOUT OVERLAPS), > UNIQUE KEY `u` (`u`,`p` WITHOUT OVERLAPS) > ) ENGINE=DEFAULT_ENGINE DEFAULT CHARSET=latin1 > +insert into t values (2, NULL, '2003-03-01', '2003-05-01'); > +insert into t values (2, NULL, '2003-03-01', '2003-05-01'); > +ERROR 23000: Duplicate entry '2-2003-05-01-2003-03-01' for key 'PRIMARY' This is wrong. Should be no duplicate key error above. > +insert into t values (3, NULL, '2003-03-01', '2003-05-01'); > insert into t values (1, 1, '2003-03-01', '2003-05-01'); > insert into t values (1, 2, '2003-05-01', '2003-07-01'); > -insert into t values (2, 1, '2003-05-01', '2003-07-01'); > +insert into t values (4, NULL, '2003-03-01', '2003-05-01'); > +create sequence seq start=5; > +update t set id= nextval(seq), u= nextval(seq), s='2003-05-01', e='2003-07-01' > + where u is NULL; > +select * from t; > +id u s e > +1 1 2003-03-01 2003-05-01 > +1 2 2003-05-01 2003-07-01 > +5 6 2003-05-01 2003-07-01 > +7 8 2003-05-01 2003-07-01 > +9 10 2003-05-01 2003-07-01 > create or replace table t(id int, s date, e date, > period for p(s,e)); > insert into t values (1, '2003-01-01', '2003-03-01'), > diff --git a/sql/handler.cc b/sql/handler.cc > index 917386f4392..16f57533f27 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -7015,7 +7017,8 @@ int handler::ha_check_overlaps(const uchar *old_data, const uchar* new_data) > bool key_used= false; > for (uint k= 0; k < key_parts && !key_used; k++) > key_used= bitmap_is_set(table->write_set, > - key_info.key_part[k].fieldnr - 1); > + key_info.key_part[k].fieldnr - 1) > + && !key_info.key_part[k].field->is_null_in_record(new_data); Why is that? > if (!key_used) > continue; > } > @@ -7064,8 +7067,15 @@ int handler::ha_check_overlaps(const uchar *old_data, const uchar* new_data) > error= handler->ha_index_next(record_buffer); > } > > - if (!error && table->check_period_overlaps(key_info, key_info, > - new_data, record_buffer) == 0) > + bool null_in_key= false; > + for (uint k= 0; k < key_parts && !null_in_key; k++) > + { > + null_in_key= key_info.key_part[k].field->is_null_in_record(record_buffer); > + } > + > + if (!null_in_key && !error > + && table->check_period_overlaps(key_info, key_info, > + new_data, record_buffer) == 0) That's strange. You compare keys in key_period_compare_bases(), why do you do NULL values here? > error= HA_ERR_FOUND_DUPP_KEY; > > if (error == HA_ERR_KEY_NOT_FOUND || error == HA_ERR_END_OF_FILE) Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] eea71e8b05a: MDEV-16978 Application-time periods: WITHOUT OVERLAPS
by Sergei Golubchik 10 Mar '20

10 Mar '20

Hi, Nikita! Despite what the subject says, it's the review for eea71e8b05a..7a5d3316805 That is everything in bb-10.5-MDEV-16978-without-overlaps minus the last commit (that came too late and I'll review it separately) In general it looks pretty good, just few minor comments below: On Mar 10, Nikita Malyavin wrote: > revision-id: eea71e8b05a (mariadb-10.5.0-246-geea71e8b05a) > parent(s): 6618fc29749 > author: Nikita Malyavin <nikitamalyavin(a)gmail.com> > committer: Nikita Malyavin <nikitamalyavin(a)gmail.com> > timestamp: 2020-02-21 02:38:57 +1000 > message: > > MDEV-16978 Application-time periods: WITHOUT OVERLAPS > > * The overlaps check is implemented on a handler level per row command. > It creates a separate cursor (actually, another handler instance) and > caches it inside the original handler, when ha_update_row or > ha_insert_row is issued. Cursor closes on unlocking the handler. > > * Containing the same key in index means unique constraint violation > even in usual terms. So we fetch left and right neighbours and check > that they have same key prefix, excluding from the key only the period part. > If it doesnt match, then there's no such neighbour, and the check passes. > Otherwise, we check if this neighbour intersects with the considered key. > > * The check does introduce new error and fails with ER_DUPP_KEY error. "does not introduce new error" you mean? > This might break REPLACE workflow and should be fixed separately > > diff --git a/sql/field.h b/sql/field.h > index 4a8eec35b05..e187ffeb331 100644 > --- a/sql/field.h > +++ b/sql/field.h > @@ -1444,8 +1444,9 @@ class Field: public Value_source > if (null_ptr) > null_ptr=ADD_TO_PTR(null_ptr,ptr_diff,uchar*); > } > - virtual void get_image(uchar *buff, uint length, CHARSET_INFO *cs) > - { memcpy(buff,ptr,length); } > + virtual void get_image(uchar *buff, uint length, > + const uchar *ptr_arg, CHARSET_INFO *cs) const > + { memcpy(buff,ptr_arg,length); } please, add a convenience method. void get_image(uchar *buff, uint length, CHARSET_INFO *cs) { get_image(buff, length, ptr, cs); } and the same below, where you add ptr_arg > virtual void set_image(const uchar *buff,uint length, CHARSET_INFO *cs) > { memcpy(ptr,buff,length); } > > @@ -4056,7 +4066,8 @@ class Field_varstring :public Field_longstr { > using Field_str::store; > double val_real() override; > longlong val_int() override; > - String *val_str(String *, String *) override; > + String *val_str(String *, String *) final; > + virtual String *val_str(String*,String *, const uchar*) const; This means that for the sake of indexes WITHOUT OVERLAPS (that very few people will use) and compressed blobs (that are used even less) you've added a new virtual call to Field_varstring::val_str (that is used awfully a lot) Try to avoid it, please > my_decimal *val_decimal(my_decimal *) override; > int cmp_max(const uchar *, const uchar *, uint max_length) const override; > int cmp(const uchar *a,const uchar *b) const override > diff --git a/sql/field.cc b/sql/field.cc > index 1ce49b0bdfa..82df2784057 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -9638,11 +9645,12 @@ int Field_bit::cmp_offset(my_ptrdiff_t row_offset) > } > > > -uint Field_bit::get_key_image(uchar *buff, uint length, imagetype type_arg) > +uint Field_bit::get_key_image(uchar *buff, uint length, const uchar *ptr_arg, imagetype type_arg) const > { > if (bit_len) > { > - uchar bits= get_rec_bits(bit_ptr, bit_ofs, bit_len); > + auto *bit_ptr_for_arg= ptr_arg + (bit_ptr - ptr); please, don't use auto in trivial cases like this one. it might be easier to type, but then the reviewer and whoever will in the future will edit this code will have to do type derivation in the head. > + uchar bits= get_rec_bits(bit_ptr_for_arg, bit_ofs, bit_len); > *buff++= bits; > length--; > } > diff --git a/sql/item_buff.cc b/sql/item_buff.cc > index 81949bcdae0..514ac740697 100644 > --- a/sql/item_buff.cc > +++ b/sql/item_buff.cc > @@ -195,7 +195,7 @@ bool Cached_item_field::cmp(void) > becasue of value change), then copy the new value to buffer. > */ > if (! null_value && (tmp || (tmp= (field->cmp(buff) != 0)))) > - field->get_image(buff,length,field->charset()); > + field->get_image(buff,length,field->ptr,field->charset()); not needed if you add convenience methods > return tmp; > } > > diff --git a/sql/sql_class.h b/sql/sql_class.h > index 13b2659789d..2f1b1431dc0 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -357,13 +357,15 @@ class Key :public Sql_alloc, public DDL_options { > engine_option_value *option_list; > bool generated; > bool invisible; > + bool without_overlaps; > + Lex_ident period; strictly speaking, you don't need without_overlaps property, if the period name is set it *has to be* without overlaps. but ok, whatever you prefer > > Key(enum Keytype type_par, const LEX_CSTRING *name_arg, > ha_key_alg algorithm_arg, bool generated_arg, DDL_options_st ddl_options) > :DDL_options(ddl_options), > type(type_par), key_create_info(default_key_create_info), > name(*name_arg), option_list(NULL), generated(generated_arg), > - invisible(false) > + invisible(false), without_overlaps(false) > { > key_create_info.algorithm= algorithm_arg; > } > diff --git a/sql/table.h b/sql/table.h > index 6ce92ee048e..09f03690a8c 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -1635,13 +1635,12 @@ struct TABLE > int insert_portion_of_time(THD *thd, const vers_select_conds_t &period_conds, > ha_rows *rows_inserted); > bool vers_check_update(List<Item> &items); > - > + static int check_period_overlaps(const KEY &lhs_key, const KEY &rhs_key, > + const uchar *lhs, const uchar *rhs); why did you make it a static method of TABLE? > int delete_row(); > void vers_update_fields(); > void vers_update_end(); > void find_constraint_correlated_indexes(); > - void clone_handler_for_update(); > - void delete_update_handler(); > > /** Number of additional fields used in versioned tables */ > #define VERSIONING_FIELDS 2 > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 240f001f7de..74d28ede25f 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -3959,6 +3959,28 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, > DBUG_RETURN(TRUE); > } > > + switch (key->type) { > + case Key::UNIQUE: > + if (!key->period) > + break; > + /* Fall through: > + WITHOUT OVERLAPS forces fields to be NOT NULL > + */ Why is that? > + case Key::PRIMARY: > + /* Implicitly set primary key fields to NOT NULL for ISO conf. */ > + if (!(sql_field->flags & NOT_NULL_FLAG)) > + { > + /* Implicitly set primary key fields to NOT NULL for ISO conf. */ duplicated comment > + sql_field->flags|= NOT_NULL_FLAG; > + sql_field->pack_flag&= ~FIELDFLAG_MAYBE_NULL; > + null_fields--; > + } > + break; > + default: > + // Fall through > + break; > + } > + > cols2.rewind(); > switch(key->type) { > > @@ -4536,15 +4556,13 @@ bool Column_definition::sp_prepare_create_field(THD *thd, MEM_ROOT *mem_root) > } > > > -static bool vers_prepare_keys(THD *thd, HA_CREATE_INFO *create_info, > - Alter_info *alter_info, KEY **key_info, uint key_count) > +static bool append_system_key_parts(THD *thd, HA_CREATE_INFO *create_info, > + Alter_info *alter_info, KEY **key_info, > + uint key_count) > { > - DBUG_ASSERT(create_info->versioned()); > - > - const char *row_start_field= create_info->vers_info.as_row.start; > - DBUG_ASSERT(row_start_field); > - const char *row_end_field= create_info->vers_info.as_row.end; > - DBUG_ASSERT(row_end_field); > + const auto &row_start_field= create_info->vers_info.as_row.start; > + const auto &row_end_field= create_info->vers_info.as_row.end; please, don't use auto in trivial cases like this one. it might be easier to type, but then the reviewer and whoever will in the future will edit this code will have to do type derivation in the head. > + DBUG_ASSERT(!create_info->versioned() || (row_start_field && row_end_field)); > > List_iterator<Key> key_it(alter_info->key_list); > Key *key= NULL; > @@ -4553,25 +4571,61 @@ static bool vers_prepare_keys(THD *thd, HA_CREATE_INFO *create_info, > if (key->type != Key::PRIMARY && key->type != Key::UNIQUE) > continue; > > + if (create_info->versioned()) > + { > Key_part_spec *key_part=NULL; > List_iterator<Key_part_spec> part_it(key->columns); > while ((key_part=part_it++)) > { > - if (!my_strcasecmp(system_charset_info, > - row_start_field, > - key_part->field_name.str) || > - > - !my_strcasecmp(system_charset_info, > - row_end_field, > - key_part->field_name.str)) > + if (row_start_field.streq(key_part->field_name) || > + row_end_field.streq(key_part->field_name)) > break; > } > - if (key_part) > - continue; // Key already contains Sys_start or Sys_end > + if (!key_part) > + key->columns.push_back(new Key_part_spec(&row_end_field, 0)); > + } > + } > > - Key_part_spec *key_part_sys_end_col= > - new (thd->mem_root) Key_part_spec(&create_info->vers_info.as_row.end, 0); > - key->columns.push_back(key_part_sys_end_col); > + key_it.rewind(); > + while ((key=key_it++)) please skip this loop if there's no PERIOD and skip the loop above if there's no system versioning. > + { > + if (key->without_overlaps) > + { > + if (key->type != Key::PRIMARY && key->type != Key::UNIQUE) > + { > + my_error(ER_PERIOD_WITHOUT_OVERLAPS_NON_UNIQUE, MYF(0), key->period.str); > + return true; I think this can even be a syntax error in the parser. there's no need to postpone it till here, is there? > + } > + if (!create_info->period_info.is_set() > + || !key->period.streq(create_info->period_info.name)) > + { > + my_error(ER_PERIOD_NOT_FOUND, MYF(0), key->period.str); > + return true; > + } > + if (thd->work_part_info) > + { > + // Unfortunately partitions do not support searching upper/lower bounds > + // (i.e. ha_index_read_map with KEY_OR_PREV, KEY_OR_NEXT) > + my_error(ER_FEATURE_NOT_SUPPORTED_WITH_PARTITIONING, MYF(0), > + "WITHOUT OVERLAPS"); > + return true; > + } > + 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(key->columns); > + while (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); > + return true; > + } > + } > + key->columns.push_back(new Key_part_spec(&period_end, 0)); > + key->columns.push_back(new Key_part_spec(&period_start, 0)); > + } > } > > return false; > diff --git a/sql/handler.cc b/sql/handler.cc > index 7d61252eea6..917386f4392 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -4173,6 +4173,11 @@ uint handler::get_dup_key(int error) > if (table->s->long_unique_table && table->file->errkey < table->s->keys) > DBUG_RETURN(table->file->errkey); > table->file->errkey = (uint) -1; > + if (overlaps_error_key != -1) > + { > + table->file->errkey= (uint)overlaps_error_key; > + DBUG_RETURN(table->file->errkey); > + } Why do you need overlaps_error_key? it looks like you can store the conflicting key number directly in errkey and it's somewhat confusing that this method uses table->file->errkey instead of just errkey. It's totally not clear why it does that. Looks like some historical thing from 2000. > if (error == HA_ERR_FOUND_DUPP_KEY || > error == HA_ERR_FOREIGN_DUPLICATE_KEY || > error == HA_ERR_FOUND_DUPP_UNIQUE || error == HA_ERR_NULL_IN_SPATIAL || > @@ -6563,10 +6576,12 @@ static int check_duplicate_long_entry_key(TABLE *table, handler *h, > unique constraint on long columns. > @returns 0 if no duplicate else returns error > */ > -static int check_duplicate_long_entries(TABLE *table, handler *h, > - const uchar *new_rec) > +int handler::check_duplicate_long_entries(const uchar *new_rec) > { > - table->file->errkey= -1; > + if (this->inited == RND) > + create_lookup_handler(); 1. and if inited==INDEX ? 2. why 'this->' ? 3. generally it's not a good idea to check inited==RND and lookup_handler==NULL inside a loop per row, because these values cannot change in the middle of a scan. Better to check them once, when inited is being set to RND. > + handler *h= lookup_handler ? lookup_handler : table->file; why table->file and not this? > + errkey= -1; > int result; > for (uint i= 0; i < table->s->keys; i++) > { > @@ -6935,6 +6956,142 @@ void handler::set_lock_type(enum thr_lock_type lock) > table->reginfo.lock_type= lock; > } > > +/** > + @brief clone of current handler. > + Creates a clone of handler used for unique hash key and WITHOUT OVERLAPS. > + @return error code > +*/ > +int handler::create_lookup_handler() > +{ > + if (lookup_handler) > + return 0; > + lookup_handler= clone(table_share->normalized_path.str, > + table->in_use->mem_root); > + int error= lookup_handler->ha_external_lock(table->in_use, F_RDLCK); > + return error; > +} > + > +int handler::ha_check_overlaps(const uchar *old_data, const uchar* new_data) > +{ > + DBUG_ASSERT(new_data); > + if (!table_share->period.unique_keys) > + return 0; > + if (table->versioned() && !table->vers_end_field()->is_max()) > + return 0; > + > + bool is_update= old_data != NULL; > + if (!check_overlaps_buffer) > + check_overlaps_buffer= (uchar*)alloc_root(&table_share->mem_root, > + table_share->max_unique_length > + + table_share->reclength); check_overlaps_buffer is per handler. It should be allocated in the TABLE::mem_root, not TABLE_SHARE::mem_root Also, it should probably be called lookup_buffer and check_duplicate_long_entry_key() should use it too. > + auto *record_buffer= check_overlaps_buffer + table_share->max_unique_length; > + auto *handler= this; > + // handler->inited can be NONE on INSERT > + if (handler->inited != NONE) > + { > + create_lookup_handler(); > + handler= lookup_handler; > + > + // Needs to compare record refs later is old_row_found() > + if (is_update) > + position(old_data); > + } > + > + // Save and later restore this handler's keyread > + int old_this_keyread= this->keyread; Again, why do you save/restore this->keyread? If you're using lookup_handler below, then this->keyread doesn't matter. And if handler==this below, then this->inited==NONE, which implies no keyread. Either way, handler->keyread_enabled() should always be false here. What about DBUG_ASSERT(!handler->keyread_enabled()) ? > + DBUG_ASSERT(this->ha_end_keyread() == 0); please fix all cases where you used a function with side effects inside an assert. not just the one I've commented about > + > + int error= 0; > + > + for (uint key_nr= 0; key_nr < table_share->keys && !error; key_nr++) > + { > + const KEY &key_info= table->key_info[key_nr]; > + const uint key_parts= key_info.user_defined_key_parts; > + if (!key_info.without_overlaps) > + continue; > + > + if (is_update) > + { > + bool key_used= false; > + for (uint k= 0; k < key_parts && !key_used; k++) > + key_used= bitmap_is_set(table->write_set, > + key_info.key_part[k].fieldnr - 1); > + if (!key_used) > + continue; > + } > + > + error= handler->ha_index_init(key_nr, 0); > + if (error) > + return error; we should try to minimize number of index_init/index_end, it doesn't look like a cheap operation, at least in InnoDB. but in a separate commit, because it should cover long uniques too. > + > + error= handler->ha_start_keyread(key_nr); > + DBUG_ASSERT(!error); > + > + const uint period_field_length= key_info.key_part[key_parts - 1].length; > + const uint key_base_length= key_info.key_length - 2 * period_field_length; > + > + key_copy(check_overlaps_buffer, new_data, &key_info, 0); > + > + /* Copy period_start to period_end. > + the value in period_start field is not significant, but anyway let's leave > + it defined to avoid uninitialized memory access > + */ > + memcpy(check_overlaps_buffer + key_base_length, > + check_overlaps_buffer + key_base_length + period_field_length, > + period_field_length); > + > + /* Find row with period_end > (period_start of new_data) */ > + error = handler->ha_index_read_map(record_buffer, > + check_overlaps_buffer, > + key_part_map((1 << (key_parts - 1)) - 1), > + HA_READ_AFTER_KEY); > + > + if (!error && is_update) > + { > + /* In case of update it could happen that the nearest neighbour is > + a record we are updating. It means, that there are no overlaps > + from this side. > + > + An assumption is made that during update we always have the last > + fetched row in old_data. Therefore, comparing ref's is enough > + */ > + DBUG_ASSERT(handler != this); > + DBUG_ASSERT(inited != NONE); > + DBUG_ASSERT(ref_length == handler->ref_length); > + > + handler->position(record_buffer); > + if (memcmp(ref, handler->ref, ref_length) == 0) > + error= handler->ha_index_next(record_buffer); > + } > + > + if (!error && table->check_period_overlaps(key_info, key_info, > + new_data, record_buffer) == 0) > + error= HA_ERR_FOUND_DUPP_KEY; > + > + if (error == HA_ERR_KEY_NOT_FOUND || error == HA_ERR_END_OF_FILE) > + error= 0; > + > + if (error == HA_ERR_FOUND_DUPP_KEY) > + overlaps_error_key= key_nr; > + > + int end_error= handler->ha_end_keyread(); > + DBUG_ASSERT(!end_error); > + > + end_error= handler->ha_index_end(); > + if (!error && end_error) > + error= end_error; > + } > + > + // Restore keyread of this handler, if it was enabled > + if (old_this_keyread < MAX_KEY) > + { > + error= this->ha_start_keyread(old_this_keyread); > + DBUG_ASSERT(error == 0); > + } > + > + return error; > +} > + > #ifdef WITH_WSREP > /** > @details > diff --git a/sql/table.cc b/sql/table.cc > index 718efa5767c..65fc44458f4 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -1499,6 +1500,14 @@ static size_t extra2_read_len(const uchar **extra2, const uchar *extra2_end) > return length; > } > > +static > +bool fill_unique_extra2(const uchar *extra2, size_t len, LEX_CUSTRING *section) thanks, good point. a bit confusing name, I thought it's something about UNIQUE particularly as this is what the whole patch is about :) What about read_extra2_section_once() or something like that? Or get_ or consume_ or store_ ? > +{ > + if (section->str) > + return true; > + *section= {extra2, len}; > + return false; > +} > > static > bool read_extra2(const uchar *frm_image, size_t len, extra2_fields *fields) > @@ -1725,11 +1725,26 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, > keyinfo= &first_keyinfo; > thd->mem_root= &share->mem_root; > > + auto err= [thd, share, &handler_file, &se_plugin, old_root](){ > + share->db_plugin= NULL; > + share->error= OPEN_FRM_CORRUPTED; > + share->open_errno= my_errno; > + delete handler_file; > + plugin_unlock(0, se_plugin); > + my_hash_free(&share->name_hash); > + > + if (!thd->is_error()) > + open_table_error(share, OPEN_FRM_CORRUPTED, share->open_errno); > + > + thd->mem_root= old_root; > + return HA_ERR_NOT_A_TABLE; > + }; > + Okay. I kind of see your point, but let's postpone this refactoring until after 10.5.2. I still need some time to think about it. You'll still be able to push it later. > if (write && write_frm_image(frm_image, frm_length)) > - goto err; > + DBUG_RETURN(err()); > > if (frm_length < FRM_HEADER_SIZE + FRM_FORMINFO_SIZE) > - goto err; > + DBUG_RETURN(err()); > > share->frm_version= frm_image[2]; > /* > @@ -8603,6 +8626,21 @@ void TABLE::evaluate_update_default_function() > DBUG_VOID_RETURN; > } > > +/** > + Compare two keys with periods > + @return -1, lhs precedes rhs > + 0, lhs overlaps rhs > + 1, lhs succeeds rhs > + */ > +int TABLE::check_period_overlaps(const KEY &lhs_key, const KEY &rhs_key, > + const uchar *lhs, const uchar *rhs) > +{ > + int cmp_res= key_period_compare_bases(lhs_key, rhs_key, lhs, rhs); you only use key_period_compare_bases here. I just wouldn't create a separate small function for that, but rather inlined it here. > + if (cmp_res) > + return cmp_res; > + > + return key_period_compare_periods(lhs_key, rhs_key, lhs, rhs); same for key_period_compare_periods > +} > > void TABLE::vers_update_fields() > { > diff --git a/sql/key.cc b/sql/key.cc > index 9dbb7a15726..49e97faea22 100644 > --- a/sql/key.cc > +++ b/sql/key.cc > @@ -896,3 +897,51 @@ bool key_buf_cmp(KEY *key_info, uint used_key_parts, > } > return FALSE; > } > + > + > +/** > + Compare base parts (not including the period) of keys with period > + @return -1, lhs less than rhs > + 0, lhs equals rhs > + 1, lhs more than rhs > + */ > +int key_period_compare_bases(const KEY &lhs_key, const KEY &rhs_key, > + const uchar *lhs, const uchar *rhs) > +{ > + uint base_part_nr= lhs_key.user_defined_key_parts - 2; may be, give this '2' a name, like 'period_key_parts' ? and use them everywhere, of course, not just here > + int cmp_res= 0; > + for (uint part_nr= 0; !cmp_res && part_nr < base_part_nr; part_nr++) > + { > + Field *f= lhs_key.key_part[part_nr].field; > + cmp_res= f->cmp(f->ptr_in_record(lhs), > + rhs_key.key_part[part_nr].field->ptr_in_record(rhs)); This doesn't seem to be handling NULLs (see the next review) > + } > + > + return cmp_res; > +} > + > +/** > + Compare periods of two keys > + @return -1, lhs preceeds rhs > + 0, lhs overlaps rhs > + 1, lhs succeeds rhs > + */ > +int key_period_compare_periods(const KEY &lhs_key, const KEY &rhs_key, > + const uchar *lhs, const uchar *rhs) > +{ > + uint period_start= lhs_key.user_defined_key_parts - 1; > + uint period_end= lhs_key.user_defined_key_parts - 2; > + > + const auto *f= lhs_key.key_part[period_start].field; > + const uchar *l[]= {lhs_key.key_part[period_start].field->ptr_in_record(lhs), > + rhs_key.key_part[period_start].field->ptr_in_record(rhs)}; > + > + const uchar *r[]= {lhs_key.key_part[period_end].field->ptr_in_record(lhs), > + rhs_key.key_part[period_end].field->ptr_in_record(rhs)}; I'd still prefer names like 'ls', 'le', 'rs', 're'. Now I need to look up and keep in mind that 'r[0] is left key end period' etc > + > + if (f->cmp(r[0], l[1]) <= 0) > + return -1; > + if (f->cmp(l[0], r[1]) >= 0) > + return 1; > + return 0; > +} Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] 9ae015878f1: MDEV-10047: table-based master info repository
by sujatha 10 Mar '20

10 Mar '20

revision-id: 9ae015878f11be3e3033fd1b35357ea5927c6c51 (mariadb-10.5.0-329-g9ae015878f1) parent(s): b753ac066bc26acda9deb707a31c112f1bbf9ec2 author: Sujatha committer: Sujatha timestamp: 2020-03-10 15:55:50 +0530 message: MDEV-10047: table-based master info repository Problem: ======= When we upgrade from "mysql" to "mariadb" if slave is using repositories as tables their data is completely ignored and no warning is issued in error log. Fix: === "mysql_upgrade" test should check for the presence of data in "mysql.slave_master_info" and "mysql.slave_relay_log_info" tables. When tables have some data the upgrade script should report a warning which hints users that the data in repository tables will be ignored. --- client/mysql_upgrade.c | 61 +++++++++- .../main/rpl_mysql_upgrade_slave_repo_check.result | 33 ++++++ .../main/rpl_mysql_upgrade_slave_repo_check.test | 127 +++++++++++++++++++++ 3 files changed, 220 insertions(+), 1 deletion(-) diff --git a/client/mysql_upgrade.c b/client/mysql_upgrade.c index 4e17089593f..bea82c2a112 100644 --- a/client/mysql_upgrade.c +++ b/client/mysql_upgrade.c @@ -1014,6 +1014,64 @@ static int install_used_engines(void) return 0; } +static int check_slave_repositories(void) +{ + DYNAMIC_STRING ds_result; + int row_count= 0; + int error= 0; + const char *query = "SELECT COUNT(*) AS c1 FROM mysql.slave_master_info"; + + if (init_dynamic_string(&ds_result, "", 512, 512)) + die("Out of memory"); + + run_query(query, &ds_result, TRUE); + + if (ds_result.length) + { + row_count= atoi((char *)ds_result.str); + if (row_count) + { + fprintf(stderr,"Slave info repository compatibility check:" + " Found data in `mysql`.`slave_master_info` table.\n"); + fprintf(stderr,"Warning: Content of `mysql`.`slave_master_info` table" + " will be ignored as MariaDB supports file based info " + "repository.\n"); + error= 1; + } + } + dynstr_free(&ds_result); + + query = "SELECT COUNT(*) AS c1 FROM mysql.slave_relay_log_info"; + + if (init_dynamic_string(&ds_result, "", 512, 512)) + die("Out of memory"); + + run_query(query, &ds_result, TRUE); + + if (ds_result.length) + { + row_count= atoi((char *)ds_result.str); + if (row_count) + { + fprintf(stderr, "Slave info repository compatibility check:" + " Found data in `mysql`.`slave_relay_log_info` table.\n"); + fprintf(stderr, "Warning: Content of `mysql`.`slave_relay_log_info` " + "table will be ignored as MariaDB supports file based " + "repository.\n"); + error= 1; + } + } + dynstr_free(&ds_result); + if (error) + { + fprintf(stderr,"Slave server may not possess the correct replication " + "metadata.\n"); + fprintf(stderr, "Execution of CHANGE MASTER as per " + "`mysql`.`slave_master_info` and `mysql`.`slave_relay_log_info` " + "table content is recommended.\n"); + } + return 0; +} /* Update all system tables in MySQL Server to current @@ -1225,7 +1283,8 @@ int main(int argc, char **argv) run_mysqlcheck_views() || run_sql_fix_privilege_tables() || run_mysqlcheck_fixnames() || - run_mysqlcheck_upgrade(FALSE)) + run_mysqlcheck_upgrade(FALSE) || + check_slave_repositories()) die("Upgrade failed" ); verbose("Phase %d/%d: Running 'FLUSH PRIVILEGES'", ++phase, phases_total); diff --git a/mysql-test/main/rpl_mysql_upgrade_slave_repo_check.result b/mysql-test/main/rpl_mysql_upgrade_slave_repo_check.result new file mode 100644 index 00000000000..87cc9ab5a24 --- /dev/null +++ b/mysql-test/main/rpl_mysql_upgrade_slave_repo_check.result @@ -0,0 +1,33 @@ +include/master-slave.inc +[connection master] +******************************************************************** +* Test case1: Upgrade when repository tables have data. * +* mysql_upgrade script should report warnings. * +******************************************************************** +connection master; +Slave info repository compatibility check: Found data in `mysql`.`slave_master_info` table. +Warning: Content of `mysql`.`slave_master_info` table will be ignored as MariaDB supports file based info repository. +Slave info repository compatibility check: Found data in `mysql`.`slave_relay_log_info` table. +Warning: Content of `mysql`.`slave_relay_log_info` table will be ignored as MariaDB supports file based repository. +Slave server may not possess the correct replication metadata. +Execution of CHANGE MASTER as per `mysql`.`slave_master_info` and `mysql`.`slave_relay_log_info` table content is recommended. +connection slave; +Slave info repository compatibility check: Found data in `mysql`.`slave_master_info` table. +Warning: Content of `mysql`.`slave_master_info` table will be ignored as MariaDB supports file based info repository. +Slave info repository compatibility check: Found data in `mysql`.`slave_relay_log_info` table. +Warning: Content of `mysql`.`slave_relay_log_info` table will be ignored as MariaDB supports file based repository. +Slave server may not possess the correct replication metadata. +Execution of CHANGE MASTER as per `mysql`.`slave_master_info` and `mysql`.`slave_relay_log_info` table content is recommended. +connection master; +TRUNCATE TABLE `mysql`.`slave_master_info`; +TRUNCATE TABLE `mysql`.`slave_relay_log_info`; +******************************************************************** +* Test case2: Upgrade when repository tables are empty. * +* mysql_upgrade script should not report any warning. * +******************************************************************** +connection master; +connection slave; +"====== Clean up ======" +connection master; +DROP TABLE `mysql`.`slave_master_info`, `mysql`.`slave_relay_log_info`; +include/rpl_end.inc diff --git a/mysql-test/main/rpl_mysql_upgrade_slave_repo_check.test b/mysql-test/main/rpl_mysql_upgrade_slave_repo_check.test new file mode 100644 index 00000000000..24b5f029e8d --- /dev/null +++ b/mysql-test/main/rpl_mysql_upgrade_slave_repo_check.test @@ -0,0 +1,127 @@ +# ==== Purpose ==== +# +# While upgrading from "mysql" to "mariadb" if slave info repositories are +# configured to be tables then appropriate warnings should be reported. +# +# ==== Implementation ==== +# +# Steps: +# 1 - On MariaDB server create `mysql`.`slave_master_info` and +# `mysql.slave_relay_log_info` tables to simulate upgrade from "mysql" +# to "mariadb" server. Insert data into these tables. +# 2 - Execute "mysql_upgrade" script and verify that appropriate warning +# is reported. i.e Warning is to alert user that the data present in +# repository tables will be ignored. +# 3 - Truncate these tables. This simulates repositories being file and +# the tables are empty. +# 4 - Execute "mysql_upgrade" script and verify that no warnings are +# reported. +# +# ==== References ==== +# +# MDEV-10047: table-based master info repository +# + +--source include/have_innodb.inc +--source include/mysql_upgrade_preparation.inc +--source include/have_binlog_format_mixed.inc +--source include/master-slave.inc + +--write_file $MYSQLTEST_VARDIR/tmp/slave_table_repo_init.sql +--disable_query_log +--disable_result_log +SET SQL_LOG_BIN=0; +# Table structure extracted from MySQL-5.6.47 +CREATE TABLE `mysql`.`slave_master_info` ( + `Number_of_lines` int(10) unsigned NOT NULL COMMENT 'Number of lines in the file.', + `Master_log_name` text CHARACTER SET utf8 COLLATE utf8_bin NOT NULL COMMENT 'The name of the master binary log currently being read from the master.', + `Master_log_pos` bigint(20) unsigned NOT NULL COMMENT 'The master log position of the last read event.', + `Host` char(64) CHARACTER SET utf8 COLLATE utf8_bin NOT NULL DEFAULT '' COMMENT 'The host name of the master.', + `User_name` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The user name used to connect to the master.', + `User_password` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The password used to connect to the master.', + `Port` int(10) unsigned NOT NULL COMMENT 'The network port used to connect to the master.', + `Connect_retry` int(10) unsigned NOT NULL COMMENT 'The period (in seconds) that the slave will wait before trying to reconnect to the master.', + `Enabled_ssl` tinyint(1) NOT NULL COMMENT 'Indicates whether the server supports SSL connections.', + `Ssl_ca` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The file used for the Certificate Authority (CA) certificate.', + `Ssl_capath` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The path to the Certificate Authority (CA) certificates.', + `Ssl_cert` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The name of the SSL certificate file.', + `Ssl_cipher` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The name of the cipher in use for the SSL connection.', + `Ssl_key` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The name of the SSL key file.', + `Ssl_verify_server_cert` tinyint(1) NOT NULL COMMENT 'Whether to verify the server certificate.', + `Heartbeat` float NOT NULL, + `Bind` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'Displays which interface is employed when connecting to the MySQL server', + `Ignored_server_ids` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The number of server IDs to be ignored, followed by the actual server IDs', + `Uuid` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The master server uuid.', + `Retry_count` bigint(20) unsigned NOT NULL COMMENT 'Number of reconnect attempts, to the master, before giving up.', + `Ssl_crl` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The file used for the Certificate Revocation List (CRL)', + `Ssl_crlpath` text CHARACTER SET utf8 COLLATE utf8_bin COMMENT 'The path used for Certificate Revocation List (CRL) files', + `Enabled_auto_position` tinyint(1) NOT NULL COMMENT 'Indicates whether GTIDs will be used to retrieve events from the master.', + PRIMARY KEY (`Host`,`Port`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8 STATS_PERSISTENT=0 COMMENT='Master Information'; + +INSERT INTO `mysql`.`slave_master_info` VALUES (23,'master-bin.000001', 120, 'localhost', 'root'," ", 13000, 60, 0," "," "," "," "," ",0 , 60," ", " ", '28e10fdd-6289-11ea-aab9-207918567a34',10," "," ", 0 ); + +# Table structure extracted from MySQL-5.6.47 +CREATE TABLE `mysql`.`slave_relay_log_info` ( + `Number_of_lines` int(10) unsigned NOT NULL COMMENT 'Number of lines in the file or rows in the table. Used to version table definitions.', + `Relay_log_name` text CHARACTER SET utf8 COLLATE utf8_bin NOT NULL COMMENT 'The name of the current relay log file.', + `Relay_log_pos` bigint(20) unsigned NOT NULL COMMENT 'The relay log position of the last executed event.', + `Master_log_name` text CHARACTER SET utf8 COLLATE utf8_bin NOT NULL COMMENT 'The name of the master binary log file from which the events in the relay log file were read.', + `Master_log_pos` bigint(20) unsigned NOT NULL COMMENT 'The master log position of the last executed event.', + `Sql_delay` int(11) NOT NULL COMMENT 'The number of seconds that the slave must lag behind the master.', + `Number_of_workers` int(10) unsigned NOT NULL, + `Id` int(10) unsigned NOT NULL COMMENT 'Internal Id that uniquely identifies this record.', + PRIMARY KEY (`Id`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8 STATS_PERSISTENT=0 COMMENT='Relay Log Information'; + +INSERT INTO `mysql`.`slave_relay_log_info` VALUES (7,'./slave-relay-bin.000001',4 ," ",0, 0 ,0 , 1); +SET SQL_LOG_BIN=1; +--enable_query_log +--enable_result_log +EOF + +--echo ******************************************************************** +--echo * Test case1: Upgrade when repository tables have data. * +--echo * mysql_upgrade script should report warnings. * +--echo ******************************************************************** +--connection master +--source $MYSQLTEST_VARDIR/tmp/slave_table_repo_init.sql +--exec $MYSQL_UPGRADE --skip-verbose --force --user=root > $MYSQLTEST_VARDIR/log/mysql_upgrade_master.log 2>&1 +--cat_file $MYSQLTEST_VARDIR/log/mysql_upgrade_master.log + +--connection slave +--source $MYSQLTEST_VARDIR/tmp/slave_table_repo_init.sql +--exec $MYSQL_UPGRADE --skip-verbose --force --user=root > $MYSQLTEST_VARDIR/log/mysql_upgrade_slave.log 2>&1 +--cat_file $MYSQLTEST_VARDIR/log/mysql_upgrade_slave.log + +--connection master +let $datadir= `select @@datadir`; +remove_file $datadir/mysql_upgrade_info; +TRUNCATE TABLE `mysql`.`slave_master_info`; +TRUNCATE TABLE `mysql`.`slave_relay_log_info`; +--remove_file $MYSQLTEST_VARDIR/log/mysql_upgrade_master.log +--remove_file $MYSQLTEST_VARDIR/log/mysql_upgrade_slave.log + +--echo ******************************************************************** +--echo * Test case2: Upgrade when repository tables are empty. * +--echo * mysql_upgrade script should not report any warning. * +--echo ******************************************************************** +--connection master +--exec $MYSQL_UPGRADE --skip-verbose --force --user=root > $MYSQLTEST_VARDIR/log/mysql_upgrade_master.log 2>&1 +--cat_file $MYSQLTEST_VARDIR/log/mysql_upgrade_master.log + +--connection slave +--exec $MYSQL_UPGRADE --skip-verbose --force --user=root > $MYSQLTEST_VARDIR/log/mysql_upgrade_slave.log 2>&1 +--cat_file $MYSQLTEST_VARDIR/log/mysql_upgrade_slave.log + +--echo "====== Clean up ======" +--connection master +let $datadir= `select @@datadir`; +remove_file $datadir/mysql_upgrade_info; +DROP TABLE `mysql`.`slave_master_info`, `mysql`.`slave_relay_log_info`; + +--remove_file $MYSQLTEST_VARDIR/tmp/slave_table_repo_init.sql +--remove_file $MYSQLTEST_VARDIR/log/mysql_upgrade_master.log +--remove_file $MYSQLTEST_VARDIR/log/mysql_upgrade_slave.log + +--source include/rpl_end.inc

1 0

[Maria-developers] Please review MDEV-17832 Protocol: extensions for Pluggable types and JSON, GEOMETRY
by Alexander Barkov 06 Mar '20

06 Mar '20

Hi, Sergei, Georg, Please review a fixed version of the patch for MDEV-17832. There are two files attached: - mdev-17821.v18.diff (server changes) - mdev-17821-cli.v06.diff (libmariadb changes) Comparing to the previous version, this version: 1. Adds a new structure MA_FIELD_EXTENSION 2. Moves extended data type information from MYSQL_FIELD to MYSQL_FIELD::extension in the client-server implementation. Note, in case of embedded server, the extended metadata is stored directly to MYSQL_FIELD. 3. Adds a new API function mariadb_field_metadata_attr(), to extract metadata from MYSQL_FIELD. 4. Changes the way how the metadata is packed on the wire from "easily human readable" to "easily parse-able", which: - makes the things faster - allows to transfer arbitrary binary data in the future, if needed. Every metadata chunk is now encoded as: a. chunk type (1 byte) b. chunk data length (1 byte) c. chunk data (according to #b) For now, two chunk types are implemented: - data type name (used for GEOMETRY sub-types, and for INET6) - format name (for JSON) Thanks!

3 4

[Maria-developers] Fwd: [andrei.elkin@mariadb.com] 30a3ee1b8ce: MDEV-21469: Implement crash-safe logging of the user XA
by andrei.elkin＠pp.inet.fi 04 Mar '20

04 Mar '20

Kristian, Fyi, here is the XA replication event recovery part. It's largely coded by Sujatha Sivakumar. The latest patch resides in bb-10.5-MDEV_21469. Cheers, Andrei revision-id: 30a3ee1b8ce98ea33dfc0595207bf467cdb91def (mariadb-10.5.0-285-g30a3ee1b8ce) parent(s): 77f4a1938f2a069174e07b3453fa0f99ba171a2e author: Sujatha committer: Andrei Elkin timestamp: 2020-03-04 14:45:01 +0200 message: MDEV-21469: Implement crash-safe logging of the user XA Description: Make XA PREPARE, XA COMMIT and XA ROLLBACK statements crash-safe. Implementation: In order to ensure consistent replication XA statements like XA PREPARE, XA COMMIT and XA ROLLBACK are firstly written into the binary log and then to storage engine. In a case server crashes after writing to binary log but not in storage engine it will lead to inconsistent state. In order to make both binary log and engine to be consistent crash recovery needs to be initiated. During crash recovery binary log needs to be parsed to identify the transactions which are present only in binary log and not present in engine. These transaction are resubmitted to engine to make it consistent along with binary log. --- .../r/binlog_xa_multi_binlog_crash_recovery.result | 40 +++ .../t/binlog_xa_multi_binlog_crash_recovery.test | 85 +++++ .../suite/rpl/r/rpl_xa_commit_crash_safe.result | 50 +++ .../suite/rpl/r/rpl_xa_event_apply_failure.result | 60 ++++ .../rpl/r/rpl_xa_prepare_commit_prepare.result | 48 +++ .../suite/rpl/r/rpl_xa_prepare_crash_safe.result | 62 ++++ .../rpl/r/rpl_xa_rollback_commit_crash_safe.result | 47 +++ .../suite/rpl/t/rpl_xa_commit_crash_safe.test | 98 ++++++ .../suite/rpl/t/rpl_xa_event_apply_failure.test | 119 +++++++ .../suite/rpl/t/rpl_xa_prepare_commit_prepare.test | 95 ++++++ .../suite/rpl/t/rpl_xa_prepare_crash_safe.test | 117 +++++++ .../rpl/t/rpl_xa_rollback_commit_crash_safe.test | 97 ++++++ sql/handler.cc | 29 +- sql/handler.h | 12 +- sql/log.cc | 379 +++++++++++++++++++-- sql/log.h | 8 +- sql/log_event.cc | 18 +- sql/log_event.h | 15 +- sql/mysqld.cc | 4 +- sql/xa.cc | 4 + 20 files changed, 1353 insertions(+), 34 deletions(-) diff --git a/mysql-test/suite/binlog/r/binlog_xa_multi_binlog_crash_recovery.result b/mysql-test/suite/binlog/r/binlog_xa_multi_binlog_crash_recovery.result new file mode 100644 index 00000000000..a09472aa94c --- /dev/null +++ b/mysql-test/suite/binlog/r/binlog_xa_multi_binlog_crash_recovery.result @@ -0,0 +1,40 @@ +RESET MASTER; +CREATE TABLE t1 (a INT PRIMARY KEY, b MEDIUMTEXT) ENGINE=Innodb; +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +connect con1,localhost,root,,; +SET DEBUG_SYNC= "simulate_hang_after_binlog_prepare SIGNAL con1_ready WAIT_FOR con1_go"; +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_prepare"; +XA START 'xa1'; +INSERT INTO t1 SET a=1; +XA END 'xa1'; +XA PREPARE 'xa1';; +connection default; +SET DEBUG_SYNC= "now WAIT_FOR con1_ready"; +FLUSH LOGS; +FLUSH LOGS; +FLUSH LOGS; +show binary logs; +Log_name File_size +master-bin.000001 # +master-bin.000002 # +master-bin.000003 # +master-bin.000004 # +include/show_binlog_events.inc +Log_name Pos Event_type Server_id End_log_pos Info +master-bin.000004 # Format_desc # # SERVER_VERSION, BINLOG_VERSION +master-bin.000004 # Gtid_list # # [#-#-#] +master-bin.000004 # Binlog_checkpoint # # master-bin.000001 +SET DEBUG_SYNC= "now SIGNAL con1_go"; +connection con1; +ERROR HY000: Lost connection to MySQL server during query +connection default; +XA RECOVER; +formatID gtrid_length bqual_length data +1 3 0 xa1 +XA COMMIT 'xa1'; +SELECT * FROM t1; +a b +1 NULL +connection default; +DROP TABLE t1; +SET debug_sync = 'reset'; diff --git a/mysql-test/suite/binlog/t/binlog_xa_multi_binlog_crash_recovery.test b/mysql-test/suite/binlog/t/binlog_xa_multi_binlog_crash_recovery.test new file mode 100644 index 00000000000..aa8d3d04fc7 --- /dev/null +++ b/mysql-test/suite/binlog/t/binlog_xa_multi_binlog_crash_recovery.test @@ -0,0 +1,85 @@ +# ==== Purpose ==== +# +# Test verifies that XA crash recovery works fine across multiple binary logs. +# +# ==== Implementation ==== +# +# Steps: +# 0 - Generate an explicit XA transaction. Using debug simulation hold the +# execution of XA PREPARE statement after the XA PREPARE is written to +# the binary log. With this the prepare will not be done in engine. +# 1 - By executing FLUSH LOGS generate multiple binary logs. +# 2 - Now make the server to disappear at this point. +# 3 - Restart the server. During recovery the XA PREPARE from the binary +# log will be read. It is cross checked with engine. Since it is not +# present in engine it will be executed once again. +# 4 - When server is up execute XA RECOVER to check that the XA is +# prepared in engine as well. +# 5 - XA COMMIT the transaction and check the validity of the data. +# +# ==== References ==== +# +# MDEV-21469: Implement crash-safe logging of the user XA +# + +--source include/have_innodb.inc +--source include/have_debug.inc +--source include/have_debug_sync.inc +--source include/have_log_bin.inc + +RESET MASTER; + +CREATE TABLE t1 (a INT PRIMARY KEY, b MEDIUMTEXT) ENGINE=Innodb; +CALL mtr.add_suppression("Found 1 prepared XA transactions"); + +connect(con1,localhost,root,,); +SET DEBUG_SYNC= "simulate_hang_after_binlog_prepare SIGNAL con1_ready WAIT_FOR con1_go"; +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_prepare"; +XA START 'xa1'; +INSERT INTO t1 SET a=1; +XA END 'xa1'; +--send XA PREPARE 'xa1'; + +connection default; +SET DEBUG_SYNC= "now WAIT_FOR con1_ready"; +FLUSH LOGS; +FLUSH LOGS; +FLUSH LOGS; + +--source include/show_binary_logs.inc +--let $binlog_file= master-bin.000004 +--let $binlog_start= 4 +--source include/show_binlog_events.inc + +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +wait +EOF + +SET DEBUG_SYNC= "now SIGNAL con1_go"; +--source include/wait_until_disconnected.inc + +--connection con1 +--error 2013 +--reap +--source include/wait_until_disconnected.inc + +# +# Server restart +# +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +restart +EOF + +connection default; +--enable_reconnect +--source include/wait_until_connected_again.inc + +XA RECOVER; +XA COMMIT 'xa1'; + +SELECT * FROM t1; + +# Clean up. +connection default; +DROP TABLE t1; +SET debug_sync = 'reset'; diff --git a/mysql-test/suite/rpl/r/rpl_xa_commit_crash_safe.result b/mysql-test/suite/rpl/r/rpl_xa_commit_crash_safe.result new file mode 100644 index 00000000000..27d043270ea --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_xa_commit_crash_safe.result @@ -0,0 +1,50 @@ +include/master-slave.inc +[connection master] +connect master2,localhost,root,,; +connection master; +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +connection slave; +include/stop_slave.inc +connection master1; +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; +XA PREPARE 'xa2'; +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_commit"; +XA COMMIT 'xa2'; +ERROR HY000: Lost connection to MySQL server during query +connection master1; +connection master; +connection default; +connection server_1; +connection master; +connection slave; +include/start_slave.inc +connection master; +SELECT * FROM t; +f +20 +40 +XA RECOVER; +formatID gtrid_length bqual_length data +XA COMMIT 'xa2'; +ERROR XAE04: XAER_NOTA: Unknown XID +SELECT * FROM t; +f +20 +40 +connection slave; +SELECT * FROM t; +f +20 +40 +connection master; +DROP TABLE t; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_xa_event_apply_failure.result b/mysql-test/suite/rpl/r/rpl_xa_event_apply_failure.result new file mode 100644 index 00000000000..547a0aae9a6 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_xa_event_apply_failure.result @@ -0,0 +1,60 @@ +include/master-slave.inc +[connection master] +connect master2,localhost,root,,; +connection master; +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +CALL mtr.add_suppression("Failed to execute binlog query event"); +CALL mtr.add_suppression("Recovery: Error .Out of memory.."); +CALL mtr.add_suppression("Crash recovery failed."); +CALL mtr.add_suppression("Can.t init tc log"); +CALL mtr.add_suppression("Aborting"); +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +connection slave; +include/stop_slave.inc +connection master1; +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; +XA PREPARE 'xa2'; +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_commit"; +XA COMMIT 'xa2'; +ERROR HY000: Lost connection to MySQL server during query +connection master1; +connection master; +connection default; +connection default; +connection master; +*** must be no 'xa2' commit seen, as it's still prepared: +SELECT * FROM t; +f +20 +XA RECOVER; +formatID gtrid_length bqual_length data +1 3 0 xa2 +SET GLOBAL DEBUG_DBUG=""; +SET SQL_LOG_BIN=0; +XA COMMIT 'xa2'; +SET SQL_LOG_BIN=1; +connection server_1; +connection master; +connection slave; +include/start_slave.inc +connection master; +SELECT * FROM t; +f +20 +40 +connection slave; +SELECT * FROM t; +f +20 +40 +connection master; +DROP TABLE t; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_xa_prepare_commit_prepare.result b/mysql-test/suite/rpl/r/rpl_xa_prepare_commit_prepare.result new file mode 100644 index 00000000000..9ba24716639 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_xa_prepare_commit_prepare.result @@ -0,0 +1,48 @@ +include/master-slave.inc +[connection master] +connect master2,localhost,root,,; +connection master; +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +connection slave; +include/stop_slave.inc +connection master1; +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_prepare"; +XA PREPARE 'xa2'; +ERROR HY000: Lost connection to MySQL server during query +connection master1; +connection master; +connection default; +connection server_1; +connection master; +connection slave; +include/start_slave.inc +connection master; +SELECT * FROM t; +f +20 +XA RECOVER; +formatID gtrid_length bqual_length data +1 3 0 xa2 +XA COMMIT 'xa2'; +SELECT * FROM t; +f +20 +40 +connection slave; +SELECT * FROM t; +f +20 +40 +connection master; +DROP TABLE t; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_xa_prepare_crash_safe.result b/mysql-test/suite/rpl/r/rpl_xa_prepare_crash_safe.result new file mode 100644 index 00000000000..99baf59a3c1 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_xa_prepare_crash_safe.result @@ -0,0 +1,62 @@ +include/master-slave.inc +[connection master] +connect master2,localhost,root,,; +connection master; +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +CALL mtr.add_suppression("Found 2 prepared XA transactions"); +CALL mtr.add_suppression("Found 3 prepared XA transactions"); +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +connection slave; +include/stop_slave.inc +connection master1; +use test; +xa start 'xa2'; +insert into t values (30); +xa end 'xa2'; +SET DEBUG_SYNC="simulate_hang_after_binlog_prepare SIGNAL reached WAIT_FOR go"; +xa prepare 'xa2'; +connection master2; +XA START 'xa3'; +INSERT INTO t VALUES (40); +XA END 'xa3'; +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_prepare"; +XA PREPARE 'xa3'; +ERROR HY000: Lost connection to MySQL server during query +connection master1; +ERROR HY000: Lost connection to MySQL server during query +connection master; +connection default; +connection server_1; +connection master; +connection slave; +include/start_slave.inc +connection master; +SELECT * FROM t; +f +XA RECOVER; +formatID gtrid_length bqual_length data +1 3 0 xa3 +1 3 0 xa1 +1 3 0 xa2 +XA COMMIT 'xa1'; +XA COMMIT 'xa2'; +XA COMMIT 'xa3'; +SELECT * FROM t; +f +20 +30 +40 +connection slave; +SELECT * FROM t; +f +20 +30 +40 +connection master; +DROP TABLE t; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/r/rpl_xa_rollback_commit_crash_safe.result b/mysql-test/suite/rpl/r/rpl_xa_rollback_commit_crash_safe.result new file mode 100644 index 00000000000..bc48c84e1c7 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_xa_rollback_commit_crash_safe.result @@ -0,0 +1,47 @@ +include/master-slave.inc +[connection master] +connect master2,localhost,root,,; +connection master; +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +connection slave; +include/stop_slave.inc +connection master1; +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; +XA PREPARE 'xa2'; +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_rollback"; +XA ROLLBACK 'xa2'; +ERROR HY000: Lost connection to MySQL server during query +connection master1; +connection master; +connection default; +connection server_1; +connection master; +connection slave; +include/start_slave.inc +connection master; +SELECT * FROM t; +f +20 +XA RECOVER; +formatID gtrid_length bqual_length data +XA ROLLBACK 'xa2'; +ERROR XAE04: XAER_NOTA: Unknown XID +SELECT * FROM t; +f +20 +connection slave; +SELECT * FROM t; +f +20 +connection master; +DROP TABLE t; +connection slave; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_xa_commit_crash_safe.test b/mysql-test/suite/rpl/t/rpl_xa_commit_crash_safe.test new file mode 100644 index 00000000000..b9e3b0d3d0d --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_xa_commit_crash_safe.test @@ -0,0 +1,98 @@ +# ==== Purpose ==== +# +# Test verifies that XA COMMIT statements are crash safe. +# +# ==== Implementation ==== +# +# Steps: +# 0 - Generate 2 explicit XA transactions. 'xa1' and 'xa2'. +# 'xa1' will be prepared and committed. +# 1 - For 'xa2' let the XA COMMIT be done in binary log and crash the +# server so that it is not committed in engine. +# 2 - Restart the server. The recovery code should successfully recover +# 'xa2'. The COMMIT should be executed during recovery. +# 3 - Check the data in table. Both rows should be present in table. +# 4 - Trying to commit 'xa2' should report unknown 'XA' error as COMMIT is +# already complete during recovery. +# +# ==== References ==== +# +# MDEV-21469: Implement crash-safe logging of the user XA + + +--source include/have_innodb.inc +--source include/master-slave.inc +--source include/have_debug.inc + +connect (master2,localhost,root,,); +--connection master +CALL mtr.add_suppression("Found 1 prepared XA transactions"); + +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +--sync_slave_with_master +--source include/stop_slave.inc + +--connection master1 +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; +XA PREPARE 'xa2'; + +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +wait +EOF + +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_commit"; +--error 2013 # CR_SERVER_LOST +XA COMMIT 'xa2'; +--source include/wait_until_disconnected.inc + +--connection master1 +--source include/wait_until_disconnected.inc + +--connection master +--source include/wait_until_disconnected.inc + +# +# Server restart +# +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +restart +EOF + +connection default; +--enable_reconnect +--source include/wait_until_connected_again.inc + +# rpl_end.inc needs to use the connection server_1 +connection server_1; +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection master +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection slave +--source include/start_slave.inc +--sync_with_master + +--connection master +SELECT * FROM t; +XA RECOVER; +--error 1397 # ER_XAER_NOTA +XA COMMIT 'xa2'; +SELECT * FROM t; +--sync_slave_with_master + +SELECT * FROM t; + +--connection master +DROP TABLE t; +--sync_slave_with_master +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_xa_event_apply_failure.test b/mysql-test/suite/rpl/t/rpl_xa_event_apply_failure.test new file mode 100644 index 00000000000..71d0de0fc56 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_xa_event_apply_failure.test @@ -0,0 +1,119 @@ +# ==== Purpose ==== +# +# Test verifies that if for some reason an event cannot be applied during +# recovery, appropriate error is reported. +# +# ==== Implementation ==== +# +# Steps: +# 0 - Generate 2 explicit XA transactions. 'xa1' and 'xa2'. +# 'xa1' will be prepared and committed. +# 1 - For 'xa2' let the XA COMMIT be done in binary log and crash the +# server so that it is not committed in engine. +# 2 - Restart the server. Using debug simulation point make XA COMMIT 'xa2' +# execution to fail. The server will resume anyway +# to leave the error in the errlog (see "Recovery: Error.."). +# 3 - Work around the simulated failure with Commit once again +# from a connection that turns OFF binlogging. +# Slave must catch up with the master. +# +# ==== References ==== +# +# MDEV-21469: Implement crash-safe logging of the user XA + + +--source include/have_innodb.inc +--source include/master-slave.inc +--source include/have_debug.inc + +connect (master2,localhost,root,,); +--connection master +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +CALL mtr.add_suppression("Failed to execute binlog query event"); +CALL mtr.add_suppression("Recovery: Error .Out of memory.."); +CALL mtr.add_suppression("Crash recovery failed."); +CALL mtr.add_suppression("Can.t init tc log"); +CALL mtr.add_suppression("Aborting"); + +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +--sync_slave_with_master +--source include/stop_slave.inc + +--connection master1 +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; +XA PREPARE 'xa2'; + +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +wait +EOF + +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_commit"; +--error 2013 # CR_SERVER_LOST +XA COMMIT 'xa2'; +--source include/wait_until_disconnected.inc + +--connection master1 +--source include/wait_until_disconnected.inc + +--connection master +--source include/wait_until_disconnected.inc + +# +# Server restart +# +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +restart: --debug-dbug=d,trans_xa_commit_fail +EOF + +connection default; +--source include/wait_until_disconnected.inc + +connection default; +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection master +--enable_reconnect +--echo *** must be no 'xa2' commit seen, as it's still prepared: +SELECT * FROM t; +XA RECOVER; + +# Commit it manually now to work around the extra binlog record +# by turning binlogging OFF by the connection. + +SET GLOBAL DEBUG_DBUG=""; +SET SQL_LOG_BIN=0; +--error 0 +XA COMMIT 'xa2'; +SET SQL_LOG_BIN=1; + + +# rpl_end.inc needs to use the connection server_1 +connection server_1; +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection master +--source include/wait_until_connected_again.inc + +--connection slave +--source include/start_slave.inc +--sync_with_master + +--connection master +SELECT * FROM t; + +--sync_slave_with_master +SELECT * FROM t; + +--connection master +DROP TABLE t; +--sync_slave_with_master +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_xa_prepare_commit_prepare.test b/mysql-test/suite/rpl/t/rpl_xa_prepare_commit_prepare.test new file mode 100644 index 00000000000..7b987c7f29b --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_xa_prepare_commit_prepare.test @@ -0,0 +1,95 @@ +# ==== Purpose ==== +# +# Test verifies that XA PREPARE transactions are crash safe. +# +# ==== Implementation ==== +# +# Steps: +# 0 - Generate 2 explicit XA transactions. 'xa1' and 'xa2'. +# 'xa1' will be prepared and committed. +# 1 - For 'xa2' let the XA PREPARE be done in binary log and crash the +# server so that it is not prepared in engine. +# 2 - Restart the server. The recovery code should successfully recover +# 'xa2'. +# 3 - When server is up, execute XA RECOVER and verify that 'xa2' is +# present. +# 4 - Commit the XA transaction and verify its correctness. +# +# ==== References ==== +# +# MDEV-21469: Implement crash-safe logging of the user XA + +--source include/have_innodb.inc +--source include/master-slave.inc +--source include/have_debug.inc + +connect (master2,localhost,root,,); +--connection master +CALL mtr.add_suppression("Found 1 prepared XA transactions"); + +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +--sync_slave_with_master +--source include/stop_slave.inc + +--connection master1 +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; + +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +wait +EOF + +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_prepare"; +--error 2013 # CR_SERVER_LOST +XA PREPARE 'xa2'; +--source include/wait_until_disconnected.inc + +--connection master1 +--source include/wait_until_disconnected.inc + +--connection master +--source include/wait_until_disconnected.inc + +# +# Server restart +# +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +restart +EOF + +connection default; +--enable_reconnect +--source include/wait_until_connected_again.inc + +# rpl_end.inc needs to use the connection server_1 +connection server_1; +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection master +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection slave +--source include/start_slave.inc +--sync_with_master + +--connection master +SELECT * FROM t; +XA RECOVER; +XA COMMIT 'xa2'; +SELECT * FROM t; +--sync_slave_with_master + +SELECT * FROM t; + +--connection master +DROP TABLE t; +--sync_slave_with_master +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_xa_prepare_crash_safe.test b/mysql-test/suite/rpl/t/rpl_xa_prepare_crash_safe.test new file mode 100644 index 00000000000..9d2c5cce528 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_xa_prepare_crash_safe.test @@ -0,0 +1,117 @@ +# ==== Purpose ==== +# +# Test verifies that XA PREPARE transactions are crash safe. +# +# ==== Implementation ==== +# +# Steps: +# 0 - Generate 3 explicit XA transactions. 'xa1', 'xa2' and 'xa3'. +# Using debug simulation hold the execution of second XA PREPARE +# statement after the XA PREPARE is written to the binary log. +# With this the prepare will not be done in engine. +# 1 - For 'xa3' allow the PREPARE statement to be written to binary log and +# simulate server crash. +# 2 - Restart the server. The recovery code should successfully recover +# 'xa2' and 'xa3'. +# 3 - When server is up, execute XA RECOVER and verify that 'xa2' and 'xa3' +# are present along with 'xa1'. +# 4 - Commit all the XA transactions and verify their correctness. +# +# ==== References ==== +# +# MDEV-21469: Implement crash-safe logging of the user XA + + +--source include/have_innodb.inc +--source include/master-slave.inc +--source include/have_debug.inc + +connect (master2,localhost,root,,); +--connection master +CALL mtr.add_suppression("Found 1 prepared XA transactions"); +CALL mtr.add_suppression("Found 2 prepared XA transactions"); +CALL mtr.add_suppression("Found 3 prepared XA transactions"); + +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +--sync_slave_with_master +--source include/stop_slave.inc + +--connection master1 +use test; +xa start 'xa2'; +insert into t values (30); +xa end 'xa2'; +SET DEBUG_SYNC="simulate_hang_after_binlog_prepare SIGNAL reached WAIT_FOR go"; +send xa prepare 'xa2'; + +--connection master2 +let $wait_condition= + SELECT COUNT(*) = 1 FROM INFORMATION_SCHEMA.PROCESSLIST + WHERE STATE like "debug sync point: simulate_hang_after_binlog_prepare%"; +--source include/wait_condition.inc + +XA START 'xa3'; +INSERT INTO t VALUES (40); +XA END 'xa3'; + +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +wait +EOF + +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_prepare"; +--error 2013 # CR_SERVER_LOST +XA PREPARE 'xa3'; +--source include/wait_until_disconnected.inc + +--connection master1 +--error 2013 +--reap +--source include/wait_until_disconnected.inc + +--connection master +--source include/wait_until_disconnected.inc + +# +# Server restart +# +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +restart +EOF + +connection default; +--enable_reconnect +--source include/wait_until_connected_again.inc + +# rpl_end.inc needs to use the connection server_1 +connection server_1; +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection master +--enable_reconnect +--source include/wait_until_connected_again.inc + + +--connection slave +--source include/start_slave.inc +--sync_with_master + +--connection master +SELECT * FROM t; +XA RECOVER; +XA COMMIT 'xa1'; +XA COMMIT 'xa2'; +XA COMMIT 'xa3'; +SELECT * FROM t; +--sync_slave_with_master + +SELECT * FROM t; + +--connection master +DROP TABLE t; +--sync_slave_with_master +--source include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_xa_rollback_commit_crash_safe.test b/mysql-test/suite/rpl/t/rpl_xa_rollback_commit_crash_safe.test new file mode 100644 index 00000000000..6416602da5e --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_xa_rollback_commit_crash_safe.test @@ -0,0 +1,97 @@ +# ==== Purpose ==== +# +# Test verifies that XA COMMIT statements are crash safe. +# +# ==== Implementation ==== +# +# Steps: +# 0 - Generate 2 explicit XA transactions. 'xa1' and 'xa2'. +# 'xa1' will be prepared and committed. +# 1 - For 'xa2' let the XA ROLLBACK be done in binary log and crash the +# server so that it is not committed in engine. +# 2 - Restart the server. The recovery code should successfully recover +# 'xa2'. The ROLLBACK should be executed during recovery. +# 3 - Check the data in table. Only one row should be present in table. +# 4 - Trying to rollback 'xa2' should report unknown 'XA' error as rollback +# is already complete during recovery. +# +# ==== References ==== +# +# MDEV-21469: Implement crash-safe logging of the user XA + +--source include/have_innodb.inc +--source include/master-slave.inc +--source include/have_debug.inc + +connect (master2,localhost,root,,); +--connection master +CALL mtr.add_suppression("Found 1 prepared XA transactions"); + +CREATE TABLE t ( f INT ) ENGINE=INNODB; +XA START 'xa1'; +INSERT INTO t VALUES (20); +XA END 'xa1'; +XA PREPARE 'xa1'; +XA COMMIT 'xa1'; +--sync_slave_with_master +--source include/stop_slave.inc + +--connection master1 +XA START 'xa2'; +INSERT INTO t VALUES (40); +XA END 'xa2'; +XA PREPARE 'xa2'; + +--write_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +wait +EOF + +SET GLOBAL DEBUG_DBUG="d,simulate_crash_after_binlog_rollback"; +--error 2013 # CR_SERVER_LOST +XA ROLLBACK 'xa2'; +--source include/wait_until_disconnected.inc + +--connection master1 +--source include/wait_until_disconnected.inc + +--connection master +--source include/wait_until_disconnected.inc + +# +# Server restart +# +--append_file $MYSQLTEST_VARDIR/tmp/mysqld.1.expect +restart +EOF + +connection default; +--enable_reconnect +--source include/wait_until_connected_again.inc + +# rpl_end.inc needs to use the connection server_1 +connection server_1; +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection master +--enable_reconnect +--source include/wait_until_connected_again.inc + +--connection slave +--source include/start_slave.inc +--sync_with_master + +--connection master +SELECT * FROM t; +XA RECOVER; +--error 1397 # ER_XAER_NOTA +XA ROLLBACK 'xa2'; +SELECT * FROM t; +--sync_slave_with_master + +SELECT * FROM t; + +--connection master +DROP TABLE t; +--sync_slave_with_master +--source include/rpl_end.inc diff --git a/sql/handler.cc b/sql/handler.cc index a1719f9b922..c997c52e602 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1290,6 +1290,9 @@ int ha_prepare(THD *thd) error=1; break; } + DEBUG_SYNC(thd, "simulate_hang_after_binlog_prepare"); + DBUG_EXECUTE_IF("simulate_crash_after_binlog_prepare", + DBUG_SUICIDE();); } else { @@ -1795,6 +1798,8 @@ commit_one_phase_2(THD *thd, bool all, THD_TRANS *trans, bool is_real_trans) ++count; ha_info_next= ha_info->next(); ha_info->reset(); /* keep it conveniently zero-filled */ + DBUG_EXECUTE_IF("simulate_crash_after_binlog_commit", + DBUG_SUICIDE();); } trans->ha_list= 0; trans->no_2pc=0; @@ -1908,6 +1913,8 @@ int ha_rollback_trans(THD *thd, bool all) status_var_increment(thd->status_var.ha_rollback_count); ha_info_next= ha_info->next(); ha_info->reset(); /* keep it conveniently zero-filled */ + DBUG_EXECUTE_IF("simulate_crash_after_binlog_rollback", + DBUG_SUICIDE();); } trans->ha_list= 0; trans->no_2pc=0; @@ -2107,6 +2114,7 @@ struct xarecover_st int len, found_foreign_xids, found_my_xids; XID *list; HASH *commit_list; + HASH *xa_prepared_list; bool dry_run; }; @@ -2155,7 +2163,23 @@ static my_bool xarecover_handlerton(THD *unused, plugin_ref plugin, _db_doprnt_("ignore xid %s", xid_to_str(buf, info->list+i)); }); xid_cache_insert(info->list + i, true); + XID *foreign_xid= info->list + i; info->found_foreign_xids++; + + /* + For each foreign xid prepraed in engine, check if it is present in + xa_prepared_list sent by binlog. + */ + if (info->xa_prepared_list) + { + struct xa_recovery_member *member= NULL; + if ((member= (xa_recovery_member *) + my_hash_search(info->xa_prepared_list, foreign_xid->key(), + foreign_xid->key_length()))) + { + member->in_engine_prepare= true; + } + } continue; } if (IF_WSREP(!(wsrep_emulate_bin_log && @@ -2202,12 +2226,13 @@ static my_bool xarecover_handlerton(THD *unused, plugin_ref plugin, return FALSE; } -int ha_recover(HASH *commit_list) +int ha_recover(HASH *commit_list, HASH *xa_prepared_list) { struct xarecover_st info; DBUG_ENTER("ha_recover"); info.found_foreign_xids= info.found_my_xids= 0; info.commit_list= commit_list; + info.xa_prepared_list= xa_prepared_list; info.dry_run= (info.commit_list==0 && tc_heuristic_recover==0); info.list= NULL; @@ -2254,7 +2279,7 @@ int ha_recover(HASH *commit_list) info.found_my_xids, opt_tc_log_file); DBUG_RETURN(1); } - if (info.commit_list) + if (info.commit_list && !info.found_foreign_xids) sql_print_information("Crash recovery finished."); DBUG_RETURN(0); } diff --git a/sql/handler.h b/sql/handler.h index 92c2a61ed0e..4ff08d7bd08 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -521,6 +521,8 @@ enum legacy_db_type DB_TYPE_FIRST_DYNAMIC=45, DB_TYPE_DEFAULT=127 // Must be last }; + +enum xa_binlog_state {XA_PREPARE=0, XA_COMPLETE}; /* Better name for DB_TYPE_UNKNOWN. Should be used for engines that do not have a hard-coded type value here. @@ -806,7 +808,6 @@ struct st_system_tablename const char *tablename; }; - typedef ulonglong my_xid; // this line is the same as in log_event.h #define MYSQL_XID_PREFIX "MySQLXid" #define MYSQL_XID_PREFIX_LEN 8 // must be a multiple of 8 @@ -898,6 +899,13 @@ struct xid_t { }; typedef struct xid_t XID; +struct xa_recovery_member +{ + XID xid; + enum xa_binlog_state state; + bool in_engine_prepare; +}; + /* for recover() handlerton call */ #define MIN_XID_LIST_SIZE 128 #define MAX_XID_LIST_SIZE (1024*128) @@ -4996,7 +5004,7 @@ int ha_commit_one_phase(THD *thd, bool all); int ha_commit_trans(THD *thd, bool all); int ha_rollback_trans(THD *thd, bool all); int ha_prepare(THD *thd); -int ha_recover(HASH *commit_list); +int ha_recover(HASH *commit_list, HASH *xa_recover_list); /* transactions: these functions never call handlerton functions directly */ int ha_enable_transaction(THD *thd, bool on); diff --git a/sql/log.cc b/sql/log.cc index e13f8fbc88f..ed6dc87b262 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -38,6 +38,7 @@ #include "log_event.h" // Query_log_event #include "rpl_filter.h" #include "rpl_rli.h" +#include "rpl_mi.h" #include "sql_audit.h" #include "mysqld.h" @@ -3406,6 +3407,8 @@ MYSQL_BIN_LOG::MYSQL_BIN_LOG(uint *sync_period) index_file_name[0] = 0; bzero((char*) &index_file, sizeof(index_file)); bzero((char*) &purge_index_file, sizeof(purge_index_file)); + /* non-zero is a marker to conduct xa recovery and related cleanup */ + xa_recover_list.records= 0; } void MYSQL_BIN_LOG::stop_background_thread() @@ -3467,6 +3470,11 @@ void MYSQL_BIN_LOG::cleanup() mysql_cond_destroy(&COND_xid_list); mysql_cond_destroy(&COND_binlog_background_thread); mysql_cond_destroy(&COND_binlog_background_thread_end); + if (!is_relay_log && xa_recover_list.records) + { + free_root(&mem_root, MYF(0)); + my_hash_free(&xa_recover_list); + } } /* @@ -8028,7 +8036,7 @@ MYSQL_BIN_LOG::trx_group_commit_leader(group_commit_entry *leader) /* Now we have in queue the list of transactions to be committed in order. */ } - + DBUG_ASSERT(is_open()); if (likely(is_open())) // Should always be true { @@ -9717,7 +9725,7 @@ int TC_LOG_MMAP::recover() goto err2; // OOM } - if (ha_recover(&xids)) + if (ha_recover(&xids, 0)) goto err2; my_hash_free(&xids); @@ -9758,7 +9766,7 @@ int TC_LOG::using_heuristic_recover() return 0; sql_print_information("Heuristic crash recovery mode"); - if (ha_recover(0)) + if (ha_recover(0, 0)) sql_print_error("Heuristic crash recovery failed"); sql_print_information("Please restart mysqld without --tc-heuristic-recover"); return 1; @@ -10217,14 +10225,108 @@ start_binlog_background_thread() return 0; } +/** + Auxiliary function for ::recover(). + @returns a successfully created and inserted @c xa_recovery_member + into hash @c hash_arg, + or NULL. +*/ +static xa_recovery_member* +xa_member_insert(HASH *hash_arg, xid_t *xid_arg, xa_binlog_state state_arg, + MEM_ROOT *ptr_mem_root) +{ + xa_recovery_member *member= (xa_recovery_member*) + alloc_root(ptr_mem_root, sizeof(xa_recovery_member)); + if (!member) + return NULL; + + member->xid.set(xid_arg); + member->state= state_arg; + member->in_engine_prepare= false; + return my_hash_insert(hash_arg, (uchar*) member) ? NULL : member; +} +/* Inserts or update an existing hash member with a proper state */ +static bool xa_member_replace(HASH *hash_arg, xid_t *xid_arg, bool is_prepare, + MEM_ROOT *ptr_mem_root) +{ + if(is_prepare) + { + if (!(xa_member_insert(hash_arg, xid_arg, XA_PREPARE, ptr_mem_root))) + return true; + } + else + { + /* + Search if XID is already present in recovery_list. If found + and the state is 'XA_PREPRAED' mark it as XA_COMPLETE. + Effectively, there won't be XA-prepare event group replay. + */ + xa_recovery_member* member; + if ((member= (xa_recovery_member *) + my_hash_search(hash_arg, xid_arg->key(), xid_arg->key_length()))) + { + if (member->state == XA_PREPARE) + member->state= XA_COMPLETE; + } + else // We found only XA COMMIT during recovery insert to list + { + if (!(member= xa_member_insert(hash_arg, + xid_arg, XA_COMPLETE, ptr_mem_root))) + return true; + } + } + return false; +} + +extern "C" uchar *xid_get_var_key(xid_t *entry, size_t *length, + my_bool not_used __attribute__((unused))) +{ + *length= entry->key_length(); + return (uchar*) entry->key(); +} + +/** + Performs recovery based on transaction coordinator log for 2pc. At the + time of crash, if the binary log was in active state, then recovery for + 'xid's and explicit 'XA' transactions is initiated, otherwise the gtid + binlog state is updated. For 'xid' and 'XA' based recovery following steps + are performed. + + Look for latest binlog checkpoint file. There can be two cases. The active + binary log and the latest binlog checkpoint file can be the same. + + Scan the binary log from the beginning. + From GTID_LIST and GTID_EVENTs reconstruct the gtid binlog state. + Prepare a list of 'xid's for recovery. + Prepare a list of explicit 'XA' transactions for recovery. + Recover the 'xid' transactions. + The explicit 'XA' transaction recovery is initiated once all the server + components are initialized. Please check 'execute_xa_for_recovery()'. + + Called from @c MYSQL_BIN_LOG::do_binlog_recovery() + + @param linfo Store here the found log file name and position to + the NEXT log file name in the index file. + + @param last_log_name Name of the last active binary log at the time of + crash. + + @param first_log Pointer to IO_CACHE of active binary log + @param fdle Format_description_log_event of active binary log + @param do_xa Is 2pc recovery needed for 'xid's and explicit XA + transactions. + @return indicates success or failure of recovery. + @retval 0 success + @retval 1 failure + +*/ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, IO_CACHE *first_log, Format_description_log_event *fdle, bool do_xa) { Log_event *ev= NULL; HASH xids; - MEM_ROOT mem_root; char binlog_checkpoint_name[FN_REFLEN]; bool binlog_checkpoint_found; bool first_round; @@ -10237,9 +10339,17 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, bool last_gtid_valid= false; #endif - if (! fdle->is_valid() || - (do_xa && my_hash_init(&xids, &my_charset_bin, TC_LOG_PAGE_SIZE/3, 0, - sizeof(my_xid), 0, 0, MYF(0)))) + binlog_checkpoint_name[0]= 0; + if (!fdle->is_valid() || + (do_xa && + (my_hash_init(&xids, &my_charset_bin, TC_LOG_PAGE_SIZE/3, + 0, + sizeof(my_xid), 0, 0, MYF(0)) || + my_hash_init(&xa_recover_list, + &my_charset_bin, + TC_LOG_PAGE_SIZE/3, + 0, 0, + (my_hash_get_key) xid_get_var_key, 0, MYF(0))))) goto err1; if (do_xa) @@ -10313,21 +10423,29 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, #ifdef HAVE_REPLICATION case GTID_EVENT: - if (first_round) { Gtid_log_event *gev= (Gtid_log_event *)ev; - - /* Update the binlog state with any GTID logged after Gtid_list. */ - last_gtid.domain_id= gev->domain_id; - last_gtid.server_id= gev->server_id; - last_gtid.seq_no= gev->seq_no; - last_gtid_standalone= - ((gev->flags2 & Gtid_log_event::FL_STANDALONE) ? true : false); - last_gtid_valid= true; + if (first_round) + { + /* Update the binlog state with any GTID logged after Gtid_list. */ + last_gtid.domain_id= gev->domain_id; + last_gtid.server_id= gev->server_id; + last_gtid.seq_no= gev->seq_no; + last_gtid_standalone= + ((gev->flags2 & Gtid_log_event::FL_STANDALONE) ? true : false); + last_gtid_valid= true; + } + if (do_xa && + (gev->flags2 & + (Gtid_log_event::FL_PREPARED_XA | + Gtid_log_event::FL_COMPLETED_XA)) && + xa_member_replace(&xa_recover_list, &gev->xid, + gev->flags2 & Gtid_log_event::FL_PREPARED_XA, + &mem_root)) + goto err2; + break; } - break; #endif - case START_ENCRYPTION_EVENT: { if (fdle->start_decryption((Start_encryption_log_event*) ev)) @@ -10417,10 +10535,22 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, if (do_xa) { - if (ha_recover(&xids)) + if (ha_recover(&xids, &xa_recover_list)) goto err2; - free_root(&mem_root, MYF(0)); + DBUG_ASSERT(!xa_recover_list.records || + (binlog_checkpoint_found && binlog_checkpoint_name[0] != 0)); + + if (!xa_recover_list.records) + { + free_root(&mem_root, MYF(0)); + my_hash_free(&xa_recover_list); + } + else + { + xa_binlog_checkpoint_name= strmake_root(&mem_root, binlog_checkpoint_name, + strlen(binlog_checkpoint_name)); + } my_hash_free(&xids); } return 0; @@ -10436,6 +10566,7 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, { free_root(&mem_root, MYF(0)); my_hash_free(&xids); + my_hash_free(&xa_recover_list); } err1: sql_print_error("Crash recovery failed. Either correct the problem " @@ -10445,6 +10576,214 @@ int TC_LOG_BINLOG::recover(LOG_INFO *linfo, const char *last_log_name, return 1; } +void MYSQL_BIN_LOG::execute_xa_for_recovery() +{ + if (xa_recover_list.records) + (void) recover_explicit_xa_prepare(); + free_root(&mem_root, MYF(0)); + my_hash_free(&xa_recover_list); +}; + +/** + Performs recovery of explict XA transactions. + 'xa_recover_list' contains the list of XA transactions to be recovered. + These events are replayed from the binary log to complete the recovery. + + @return indicates success or failure of recovery. + @retval false success + @retval true failure + +*/ +bool MYSQL_BIN_LOG::recover_explicit_xa_prepare() +{ +#ifndef HAVE_REPLICATION + /* Can't be supported without replication applier built in. */ + return false; +#else + bool err= true; + int error=0; + Relay_log_info *rli= NULL; + rpl_group_info *rgi; + THD *thd= new THD(0); /* Needed by start_slave_threads */ + thd->thread_stack= (char*) &thd; + thd->store_globals(); + thd->security_ctx->skip_grants(); + IO_CACHE log; + const char *errmsg; + File file; + bool enable_apply_event= false; + Log_event *ev = 0; + LOG_INFO linfo; + int recover_xa_count= xa_recover_list.records; + xa_recovery_member *member= NULL; + + //DBUG_ASSERT(!thd->rli_fake); + + if (!(rli= thd->rli_fake= new Relay_log_info(FALSE, "Recovery"))) + { + my_error(ER_OUTOFMEMORY, MYF(ME_FATAL), 1); + goto err2; + } + rli->sql_driver_thd= thd; + static LEX_CSTRING connection_name= { STRING_WITH_LEN("Recovery") }; + rli->mi= new Master_info(&connection_name, false); + if (!(rgi= thd->rgi_fake)) + rgi= thd->rgi_fake= new rpl_group_info(rli); + rgi->thd= thd; + thd->system_thread_info.rpl_sql_info= + new rpl_sql_thread_info(rli->mi->rpl_filter); + + if (rli && !rli->relay_log.description_event_for_exec) + { + rli->relay_log.description_event_for_exec= + new Format_description_log_event(4); + } + if (find_log_pos(&linfo, xa_binlog_checkpoint_name, 1)) + { + sql_print_error("Binlog file '%s' not found in binlog index, needed " + "for recovery. Aborting.", xa_binlog_checkpoint_name); + goto err2; + } + + tmp_disable_binlog(thd); + thd->variables.pseudo_slave_mode= TRUE; + for (;;) + { + if ((file= open_binlog(&log, linfo.log_file_name, &errmsg)) < 0) + { + sql_print_error("%s", errmsg); + goto err1; + } + while (recover_xa_count > 0 && + (ev= Log_event::read_log_event(&log, + rli->relay_log.description_event_for_exec, + opt_master_verify_checksum))) + { + if (!ev->is_valid()) + { + sql_print_error("Found invalid binlog query event %s" + " at %s:%lu; error %d %s", ev->get_type_str(), + linfo.log_file_name, + (ev->log_pos - ev->data_written)); + goto err1; + } + enum Log_event_type typ= ev->get_type_code(); + ev->thd= thd; + + if (typ == FORMAT_DESCRIPTION_EVENT) + enable_apply_event= true; + + if (typ == GTID_EVENT) + { + Gtid_log_event *gev= (Gtid_log_event *)ev; + if (gev->flags2 & + (Gtid_log_event::FL_PREPARED_XA | Gtid_log_event::FL_COMPLETED_XA)) + { + if ((member= + (xa_recovery_member*) my_hash_search(&xa_recover_list, + gev->xid.key(), + gev->xid.key_length()))) + { + /* Got XA PREPARE query in binlog but check member->state. If it is + marked as XA_PREPARE then this PREPARE has not seen its end + COMMIT/ROLLBACK. Check if it exists in engine in prepared state. + If so apply. + */ + if (gev->flags2 & Gtid_log_event::FL_PREPARED_XA) + { + if (member->state == XA_PREPARE) + { + // XA is prepared in binlog and not present in engine then apply + if (member->in_engine_prepare == false) + enable_apply_event= true; + else + --recover_xa_count; + } + } + else if (gev->flags2 & Gtid_log_event::FL_COMPLETED_XA) + { + if (member->state == XA_COMPLETE && + member->in_engine_prepare == true) + enable_apply_event= true; + else + --recover_xa_count; + } + } + } + } + + if (enable_apply_event) + { + if (typ == XA_PREPARE_LOG_EVENT) + thd->transaction.xid_state.set_binlogged(); + if ((err= ev->apply_event(rgi))) + { + sql_print_error("Failed to execute binlog query event of type: %s," + " at %s:%lu; error %d %s", ev->get_type_str(), + linfo.log_file_name, + (ev->log_pos - ev->data_written), + thd->get_stmt_da()->sql_errno(), + thd->get_stmt_da()->message()); + delete ev; + goto err1; + } + else if (typ == FORMAT_DESCRIPTION_EVENT) + enable_apply_event=false; + else if (thd->lex->sql_command == SQLCOM_XA_PREPARE || + thd->lex->sql_command == SQLCOM_XA_COMMIT || + thd->lex->sql_command == SQLCOM_XA_ROLLBACK) + { + --recover_xa_count; + enable_apply_event=false; + + sql_print_information("Binlog event %s at %s:%lu" + " successfully applied", + typ == XA_PREPARE_LOG_EVENT ? + static_cast<XA_prepare_log_event *>(ev)->get_query() : + static_cast<Query_log_event *>(ev)->query, + linfo.log_file_name, (ev->log_pos - ev->data_written)); + } + } + if (typ != FORMAT_DESCRIPTION_EVENT) + delete ev; + } + end_io_cache(&log); + mysql_file_close(file, MYF(MY_WME)); + file= -1; + if (unlikely((error= find_next_log(&linfo, 1)))) + { + if (error != LOG_INFO_EOF) + sql_print_error("find_log_pos() failed (error: %d)", error); + else + break; + } + } +err1: + reenable_binlog(thd); + /* + There should be no more XA transactions to recover upon successful + completion. + */ + if (recover_xa_count > 0) + goto err2; + sql_print_information("Crash recovery finished."); + err= false; +err2: + if (file >= 0) + { + end_io_cache(&log); + mysql_file_close(file, MYF(MY_WME)); + } + thd->variables.pseudo_slave_mode= FALSE; + delete rli->mi; + delete thd->system_thread_info.rpl_sql_info; + rgi->slave_close_thread_tables(thd); + thd->reset_globals(); + delete thd; + + return err; +#endif /* !HAVE_REPLICATION */ +} int MYSQL_BIN_LOG::do_binlog_recovery(const char *opt_name, bool do_xa_recovery) diff --git a/sql/log.h b/sql/log.h index 8e70d3c8f4c..9bf3248d4c9 100644 --- a/sql/log.h +++ b/sql/log.h @@ -63,6 +63,7 @@ class TC_LOG virtual int unlog(ulong cookie, my_xid xid)=0; virtual int unlog_xa_prepare(THD *thd, bool all)= 0; virtual void commit_checkpoint_notify(void *cookie)= 0; + virtual void execute_xa_for_recovery() {}; protected: /* @@ -708,6 +709,8 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG void commit_checkpoint_notify(void *cookie); int recover(LOG_INFO *linfo, const char *last_log_name, IO_CACHE *first_log, Format_description_log_event *fdle, bool do_xa); + bool recover_explicit_xa_prepare(); + int do_binlog_recovery(const char *opt_name, bool do_xa_recovery); #if !defined(MYSQL_CLIENT) @@ -932,7 +935,7 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG mysql_mutex_t* get_binlog_end_pos_lock() { return &LOCK_binlog_end_pos; } int wait_for_update_binlog_end_pos(THD* thd, struct timespec * timeout); - + void execute_xa_for_recovery(); /* Binlog position of end of the binlog. Access to this is protected by LOCK_binlog_end_pos @@ -945,6 +948,9 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG */ my_off_t binlog_end_pos; char binlog_end_pos_file[FN_REFLEN]; + MEM_ROOT mem_root; + char *xa_binlog_checkpoint_name; + HASH xa_recover_list; }; class Log_event_handler diff --git a/sql/log_event.cc b/sql/log_event.cc index ee44f7f1da4..9adfefceb97 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -18,7 +18,7 @@ #include "mariadb.h" #include "sql_priv.h" - +#include "handler.h" #ifndef MYSQL_CLIENT #include "unireg.h" #include "log_event.h" @@ -2812,9 +2812,25 @@ XA_prepare_log_event(const char* buf, buf += sizeof(temp); memcpy(&temp, buf, sizeof(temp)); m_xid.gtrid_length= uint4korr(&temp); + // Todo: validity here and elsewhere checks to be replaced by MDEV-21839 fixes + if (m_xid.gtrid_length < 0 || m_xid.gtrid_length > MAXGTRIDSIZE) + { + m_xid.formatID= -1; + return; + } buf += sizeof(temp); memcpy(&temp, buf, sizeof(temp)); m_xid.bqual_length= uint4korr(&temp); + if (m_xid.bqual_length < 0 || m_xid.bqual_length > MAXBQUALSIZE) + { + m_xid.formatID= -1; + return; + } + if (m_xid.gtrid_length + m_xid.bqual_length > XIDDATASIZE) + { + m_xid.formatID= -1; + return; + } buf += sizeof(temp); memcpy(m_xid.data, buf, m_xid.gtrid_length + m_xid.bqual_length); diff --git a/sql/log_event.h b/sql/log_event.h index a6543b70eb5..595dc9f6c3c 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -3234,6 +3234,7 @@ class XA_prepare_log_event: public Xid_apply_log_event const Format_description_log_event *description_event); ~XA_prepare_log_event() {} Log_event_type get_type_code() { return XA_PREPARE_LOG_EVENT; } + bool is_valid() const { return m_xid.formatID != -1; } int get_data_size() { return xid_subheader_no_data + m_xid.gtrid_length + m_xid.bqual_length; @@ -3241,12 +3242,7 @@ class XA_prepare_log_event: public Xid_apply_log_event #ifdef MYSQL_SERVER bool write(); -#endif - -private: -#if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION) - char query[sizeof("XA COMMIT ONE PHASE") + 1 + ser_buf_size]; - int do_commit(); +#ifdef HAVE_REPLICATION const char* get_query() { sprintf(query, @@ -3254,6 +3250,13 @@ class XA_prepare_log_event: public Xid_apply_log_event m_xid.serialize()); return query; } +#endif /* HAVE_REPLICATION */ +#endif /* MYSQL_SERVER */ + +private: +#if defined(MYSQL_SERVER) && defined(HAVE_REPLICATION) + char query[sizeof("XA COMMIT ONE PHASE") + 1 + ser_buf_size]; + int do_commit(); #endif }; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index b2f8afca7a6..f669a4ca5d8 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -5189,7 +5189,7 @@ static int init_server_components() unireg_abort(1); } - if (ha_recover(0)) + if (ha_recover(0, 0)) { unireg_abort(1); } @@ -5606,7 +5606,7 @@ int mysqld_main(int argc, char **argv) initialize_information_schema_acl(); execute_ddl_log_recovery(); - + tc_log->execute_xa_for_recovery(); /* Change EVENTS_ORIGINAL to EVENTS_OFF (the default value) as there is no point in using ORIGINAL during startup diff --git a/sql/xa.cc b/sql/xa.cc index 786d09c2b39..df7d0229157 100644 --- a/sql/xa.cc +++ b/sql/xa.cc @@ -581,6 +581,10 @@ bool trans_xa_commit(THD *thd) XID_STATE &xid_state= thd->transaction.xid_state; DBUG_ENTER("trans_xa_commit"); + DBUG_EXECUTE_IF("trans_xa_commit_fail", + {my_error(ER_OUT_OF_RESOURCES, MYF(0)); + DBUG_RETURN(TRUE);}); + if (!xid_state.is_explicit_XA() || !xid_state.xid_cache_element->xid.eq(thd->lex->xid))

1 0