- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] Rev 4245: MDEV-6336: mysqldump --master-data does not work with GTID setups in http://bazaar.launchpad.net/~maria-captains/maria/10.0
by Sergey Vojtovich 25 Jun '14

25 Jun '14

Hi Kristian, MDEV-6344 looks fine. But I don't feel confident reviewing MDEV-6336. Regards, Sergey On Mon, Jun 16, 2014 at 11:38:04AM +0200, knielsen(a)knielsen-hq.org wrote: > At http://bazaar.launchpad.net/~maria-captains/maria/10.0 > > ------------------------------------------------------------ > revno: 4245 > revision-id: knielsen(a)knielsen-hq.org-20140616093803-gub60pda6t77ch3k > parent: igor(a)askmonty.org-20140610223256-nqs0kkp7i45qzyvx > committer: knielsen(a)knielsen-hq.org > branch nick: work-10.0 > timestamp: Mon 2014-06-16 11:38:03 +0200 > message: > MDEV-6336: mysqldump --master-data does not work with GTID setups > MDEV-6344: mysqldump issues FLUSH TABLES, which gets written into binlog and replicated > > Add a --gtid option (for compatibility, the original behaviour is preserved > when --gtid is not used). > > With --gtid, --master-data and --dump-slave output the GTID position (the > old-style file/offset position is still output, but commented out). Also, a > CHANGE MASTER TO master_use_gtid=slave_pos is output to ensure a provisioned > slave is configured in GTID, as requested. > > Without --gtid, the GTID position is still output, if available, but commented > out. > > Also fix MDEV-6344, to avoid FLUSH TABLES getting into the binlog. Otherwise a > mysqldump on a slave server will silently inject a GTID which does not exist > on the master, which is highly undesirable. > === modified file 'client/client_priv.h' > --- a/client/client_priv.h 2013-09-14 01:09:36 +0000 > +++ b/client/client_priv.h 2014-06-16 09:38:03 +0000 > @@ -92,6 +92,7 @@ enum options_client > OPT_REPORT_PROGRESS, > OPT_SKIP_ANNOTATE_ROWS_EVENTS, > OPT_SSL_CRL, OPT_SSL_CRLPATH, > + OPT_USE_GTID, > OPT_MAX_CLIENT_OPTION /* should be always the last */ > }; > > > === modified file 'client/mysqldump.c' > --- a/client/mysqldump.c 2014-05-09 10:35:11 +0000 > +++ b/client/mysqldump.c 2014-06-16 09:38:03 +0000 > @@ -134,6 +134,7 @@ static ulong opt_compatible_mode= 0; > #define MYSQL_OPT_SLAVE_DATA_COMMENTED_SQL 2 > static uint opt_mysql_port= 0, opt_master_data; > static uint opt_slave_data; > +static uint opt_use_gtid; > static uint my_end_arg; > static char * opt_mysql_unix_port=0; > static int first_error=0; > @@ -346,6 +347,13 @@ static struct my_option my_long_options[ > {"force", 'f', "Continue even if we get an SQL error.", > &ignore_errors, &ignore_errors, 0, GET_BOOL, NO_ARG, > 0, 0, 0, 0, 0, 0}, > + {"gtid", OPT_USE_GTID, "Used together with --master-data=1 or --dump-slave=1." > + "When enabled, the output from those options will set the GTID position " > + "instead of the binlog file and offset; the file/offset will appear only as " > + "a comment. When disabled, the GTID position will still appear in the " > + "output, but only commented.", > + &opt_use_gtid, &opt_use_gtid, 0, GET_BOOL, NO_ARG, > + 0, 0, 0, 0, 0, 0}, > {"help", '?', "Display this help message and exit.", 0, 0, 0, GET_NO_ARG, > NO_ARG, 0, 0, 0, 0, 0, 0}, > {"hex-blob", OPT_HEXBLOB, "Dump binary strings (BINARY, " > @@ -1177,7 +1185,7 @@ check_consistent_binlog_pos(char *binlog > > if (mysql_query_with_error_report(mysql, &res, > "SHOW STATUS LIKE 'binlog_snapshot_%'")) > - return 1; > + return 0; > > found= 0; > while ((row= mysql_fetch_row(res))) > @@ -1200,6 +1208,90 @@ check_consistent_binlog_pos(char *binlog > return (found == 2); > } > > + > +/* > + Get the GTID position corresponding to a given old-style binlog position. > + This uses BINLOG_GTID_POS(). The advantage is that the GTID position can > + be obtained completely non-blocking in this way (without the need for > + FLUSH TABLES WITH READ LOCK), as the old-style position can be obtained > + with START TRANSACTION WITH CONSISTENT SNAPSHOT. > + > + Returns 0 if ok, non-zero if error. > +*/ > +static int > +get_binlog_gtid_pos(char *binlog_pos_file, char *binlog_pos_offset, > + char *out_gtid_pos) > +{ > + DYNAMIC_STRING query; > + MYSQL_RES *res; > + MYSQL_ROW row; > + int err; > + char file_buf[FN_REFLEN*2+1], offset_buf[LONGLONG_LEN*2+1]; > + size_t len_pos_file= strlen(binlog_pos_file); > + size_t len_pos_offset= strlen(binlog_pos_offset); > + > + if (len_pos_file >= FN_REFLEN || len_pos_offset > LONGLONG_LEN) > + return 0; > + mysql_real_escape_string(mysql, file_buf, binlog_pos_file, len_pos_file); > + mysql_real_escape_string(mysql, offset_buf, binlog_pos_offset, len_pos_offset); > + init_dynamic_string_checked(&query, "SELECT BINLOG_GTID_POS('", 256, 1024); > + dynstr_append_checked(&query, file_buf); > + dynstr_append_checked(&query, "', '"); > + dynstr_append_checked(&query, offset_buf); > + dynstr_append_checked(&query, "')"); > + > + err= mysql_query_with_error_report(mysql, &res, query.str); > + dynstr_free(&query); > + if (err) > + return err; > + > + err= 1; > + if ((row= mysql_fetch_row(res))) > + { > + strmake(out_gtid_pos, row[0], FN_REFLEN-1); > + err= 0; > + } > + mysql_free_result(res); > + > + return err; > +} > + > + > +/* > + Get the GTID position on a master or slave. > + The parameter MASTER is non-zero to get the position on a master > + (@@gtid_binlog_pos) or zero for a slave (@@gtid_slave_pos). > + > + This uses the @@gtid_binlog_pos or @@gtid_slave_pos, so requires FLUSH TABLES > + WITH READ LOCK or similar to be consistent. > + > + Returns 0 if ok, non-zero for error. > +*/ > +static int > +get_gtid_pos(char *out_gtid_pos, int master) > +{ > + MYSQL_RES *res; > + MYSQL_ROW row; > + int found; > + > + if (mysql_query_with_error_report(mysql, &res, > + (master ? > + "SELECT @@GLOBAL.gtid_binlog_pos" : > + "SELECT @@GLOBAL.gtid_slave_pos"))) > + return 1; > + > + found= 0; > + if ((row= mysql_fetch_row(res))) > + { > + strmake(out_gtid_pos, row[0], FN_REFLEN-1); > + found++; > + } > + mysql_free_result(res); > + > + return (found != 1); > +} > + > + > static char *my_case_str(const char *str, > uint str_len, > const char *token, > @@ -4799,12 +4891,14 @@ static int dump_selected_tables(char *db > } /* dump_selected_tables */ > > > -static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos) > +static int do_show_master_status(MYSQL *mysql_con, int consistent_binlog_pos, > + int have_mariadb_gtid, int use_gtid) > { > MYSQL_ROW row; > MYSQL_RES *UNINIT_VAR(master); > char binlog_pos_file[FN_REFLEN]; > char binlog_pos_offset[LONGLONG_LEN+1]; > + char gtid_pos[FN_REFLEN]; > char *file, *offset; > const char *comment_prefix= > (opt_master_data == MYSQL_OPT_MASTER_DATA_COMMENTED_SQL) ? "-- " : ""; > @@ -4815,6 +4909,9 @@ static int do_show_master_status(MYSQL * > return 1; > file= binlog_pos_file; > offset= binlog_pos_offset; > + if (have_mariadb_gtid && > + get_binlog_gtid_pos(binlog_pos_file, binlog_pos_offset, gtid_pos)) > + return 1; > } > else > { > @@ -4844,6 +4941,9 @@ static int do_show_master_status(MYSQL * > return 0; > } > } > + > + if (have_mariadb_gtid && get_gtid_pos(gtid_pos, 1)) > + return 1; > } > > /* SHOW MASTER STATUS reports file and position */ > @@ -4852,7 +4952,19 @@ static int do_show_master_status(MYSQL * > "recovery from\n--\n\n"); > fprintf(md_result_file, > "%sCHANGE MASTER TO MASTER_LOG_FILE='%s', MASTER_LOG_POS=%s;\n", > - comment_prefix, file, offset); > + (use_gtid ? "-- " : comment_prefix), file, offset); > + if (have_mariadb_gtid) > + { > + print_comment(md_result_file, 0, > + "\n--\n-- GTID to start replication from\n--\n\n"); > + if (use_gtid) > + fprintf(md_result_file, > + "%sCHANGE MASTER TO MASTER_USE_GTID=slave_pos;\n", > + comment_prefix); > + fprintf(md_result_file, > + "%sSET GLOBAL gtid_slave_pos='%s';\n", > + (!use_gtid ? "-- " : comment_prefix), gtid_pos); > + } > check_io(md_result_file); > > if (!consistent_binlog_pos) > @@ -4922,12 +5034,16 @@ static int add_slave_statements(void) > return(0); > } > > -static int do_show_slave_status(MYSQL *mysql_con) > +static int do_show_slave_status(MYSQL *mysql_con, int use_gtid, > + int have_mariadb_gtid) > { > MYSQL_RES *UNINIT_VAR(slave); > MYSQL_ROW row; > const char *comment_prefix= > (opt_slave_data == MYSQL_OPT_SLAVE_DATA_COMMENTED_SQL) ? "-- " : ""; > + const char *gtid_comment_prefix= (use_gtid ? comment_prefix : "-- "); > + const char *nogtid_comment_prefix= (!use_gtid ? comment_prefix : "-- "); > + int set_gtid_done= 0; > > if (mysql_query_with_error_report(mysql_con, &slave, > multi_source ? > @@ -4945,8 +5061,30 @@ static int do_show_slave_status(MYSQL *m > > while ((row= mysql_fetch_row(slave))) > { > + if (multi_source && !set_gtid_done) > + { > + char gtid_pos[FN_REFLEN]; > + if (have_mariadb_gtid && get_gtid_pos(gtid_pos, 0)) > + return 1; > + if (opt_comments) > + fprintf(md_result_file, "\n--\n-- Gtid position to start replication " > + "from\n--\n\n"); > + fprintf(md_result_file, "%sSET GLOBAL gtid_slave_pos='%s';\n", > + gtid_comment_prefix, gtid_pos); > + set_gtid_done= 1; > + } > if (row[9 + multi_source] && row[21 + multi_source]) > { > + if (use_gtid) > + { > + if (multi_source) > + fprintf(md_result_file, "%sCHANGE MASTER '%.80s' TO " > + "MASTER_USE_GTID=slave_pos;\n", gtid_comment_prefix, row[0]); > + else > + fprintf(md_result_file, "%sCHANGE MASTER TO " > + "MASTER_USE_GTID=slave_pos;\n", gtid_comment_prefix); > + } > + > /* SHOW MASTER STATUS reports file and position */ > if (opt_comments) > fprintf(md_result_file, > @@ -4955,9 +5093,9 @@ static int do_show_slave_status(MYSQL *m > > if (multi_source) > fprintf(md_result_file, "%sCHANGE MASTER '%.80s' TO ", > - comment_prefix, row[0]); > + nogtid_comment_prefix, row[0]); > else > - fprintf(md_result_file, "%sCHANGE MASTER TO ", comment_prefix); > + fprintf(md_result_file, "%sCHANGE MASTER TO ", nogtid_comment_prefix); > > if (opt_include_master_host_port) > { > @@ -5030,12 +5168,13 @@ static int do_flush_tables_read_lock(MYS > FLUSH TABLES is to lower the probability of a stage where both mysqldump > and most client connections are stalled. Of course, if a second long > update starts between the two FLUSHes, we have that bad stall. > + > + We use the LOCAL option, as we do not want the FLUSH TABLES replicated to > + other servers. > */ > return > - ( mysql_query_with_error_report(mysql_con, 0, > - ((opt_master_data != 0) ? > - "FLUSH /*!40101 LOCAL */ TABLES" : > - "FLUSH TABLES")) || > + ( mysql_query_with_error_report(mysql_con, 0, > + "FLUSH /*!40101 LOCAL */ TABLES") || > mysql_query_with_error_report(mysql_con, 0, > "FLUSH TABLES WITH READ LOCK") ); > } > @@ -5656,6 +5795,7 @@ int main(int argc, char **argv) > char bin_log_name[FN_REFLEN]; > int exit_code; > int consistent_binlog_pos= 0; > + int have_mariadb_gtid= 0; > MY_INIT(argv[0]); > > sf_leaking_memory=1; /* don't report memory leaks on early exits */ > @@ -5696,7 +5836,10 @@ int main(int argc, char **argv) > > /* Check if the server support multi source */ > if (mysql_get_server_version(mysql) >= 100000) > + { > multi_source= 2; > + have_mariadb_gtid= 1; > + } > > if (opt_slave_data && do_stop_slave_sql(mysql)) > goto err; > @@ -5743,9 +5886,11 @@ int main(int argc, char **argv) > /* Add 'STOP SLAVE to beginning of dump */ > if (opt_slave_apply && add_stop_slave()) > goto err; > - if (opt_master_data && do_show_master_status(mysql, consistent_binlog_pos)) > + if (opt_master_data && do_show_master_status(mysql, consistent_binlog_pos, > + have_mariadb_gtid, opt_use_gtid)) > goto err; > - if (opt_slave_data && do_show_slave_status(mysql)) > + if (opt_slave_data && do_show_slave_status(mysql, opt_use_gtid, > + have_mariadb_gtid)) > goto err; > if (opt_single_transaction && do_unlock_tables(mysql)) /* unlock but no commit! */ > goto err; > > === modified file 'mysql-test/r/rpl_mysqldump_slave.result' > --- a/mysql-test/r/rpl_mysqldump_slave.result 2014-02-26 14:28:07 +0000 > +++ b/mysql-test/r/rpl_mysqldump_slave.result 2014-06-16 09:38:03 +0000 > @@ -4,18 +4,59 @@ include/master-slave.inc > # New --dump-slave, --apply-slave-statements functionality > # > use test; > +-- SET GLOBAL gtid_slave_pos=''; > CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; > STOP ALL SLAVES; > +-- SET GLOBAL gtid_slave_pos=''; > CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; > START ALL SLAVES; > STOP ALL SLAVES; > +-- SET GLOBAL gtid_slave_pos=''; > CHANGE MASTER '' TO MASTER_HOST='127.0.0.1', MASTER_PORT=MASTER_MYPORT, MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; > START ALL SLAVES; > start slave; > Warnings: > Note 1254 Slave is already running > +-- SET GLOBAL gtid_slave_pos=''; > CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; > start slave; > Warnings: > Note 1254 Slave is already running > +*** Test mysqldump --dump-slave GTID functionality. > +SET gtid_seq_no = 1000; > +CREATE TABLE t1 (a INT PRIMARY KEY); > +DROP TABLE t1; > +CREATE TABLE t2 (a INT PRIMARY KEY); > +DROP TABLE t2; > + > +1. --dump-slave=1 > + > +SET GLOBAL gtid_slave_pos='0-1-1001'; > +CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos; > +-- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; > + > +2. --dump-slave=2 > + > +-- SET GLOBAL gtid_slave_pos='0-1-1001'; > +-- CHANGE MASTER '' TO MASTER_USE_GTID=slave_pos; > +-- CHANGE MASTER '' TO MASTER_LOG_FILE='master-bin.000001', MASTER_LOG_POS=BINLOG_START; > +*** Test mysqldump --master-data GTID functionality. > + > +1. --master-data=1 > + > +-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START; > +CHANGE MASTER TO MASTER_USE_GTID=slave_pos; > +SET GLOBAL gtid_slave_pos='0-2-1003'; > + > +2. --master-data=2 > + > +-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START; > +-- CHANGE MASTER TO MASTER_USE_GTID=slave_pos; > +-- SET GLOBAL gtid_slave_pos='0-2-1003'; > + > +3. --master-data --single-transaction > + > +-- CHANGE MASTER TO MASTER_LOG_FILE='slave-bin.000001', MASTER_LOG_POS=BINLOG_START; > +CHANGE MASTER TO MASTER_USE_GTID=slave_pos; > +SET GLOBAL gtid_slave_pos='0-2-1003'; > include/rpl_end.inc > > === modified file 'mysql-test/t/rpl_mysqldump_slave.test' > --- a/mysql-test/t/rpl_mysqldump_slave.test 2014-02-26 14:28:07 +0000 > +++ b/mysql-test/t/rpl_mysqldump_slave.test 2014-06-16 09:38:03 +0000 > @@ -36,4 +36,53 @@ start slave; > --exec $MYSQL_DUMP_SLAVE --compact --dump-slave no_such_db > start slave; > > + > +--echo *** Test mysqldump --dump-slave GTID functionality. > + > +--connection master > +SET gtid_seq_no = 1000; > +CREATE TABLE t1 (a INT PRIMARY KEY); > +DROP TABLE t1; > +--sync_slave_with_master > + > +--connection slave > +# Inject a local transaction on the slave to check that this is not considered > +# for --dump-slave. > +CREATE TABLE t2 (a INT PRIMARY KEY); > +DROP TABLE t2; > + > +--echo > +--echo 1. --dump-slave=1 > +--echo > +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ > +--exec $MYSQL_DUMP_SLAVE --compact --dump-slave=1 --gtid test > + > +--echo > +--echo 2. --dump-slave=2 > +--echo > +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ > +--exec $MYSQL_DUMP_SLAVE --compact --dump-slave=2 --gtid test > + > + > +--echo *** Test mysqldump --master-data GTID functionality. > +--echo > +--echo 1. --master-data=1 > +--echo > +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ > +--exec $MYSQL_DUMP_SLAVE --compact --master-data=1 --gtid test > + > +--echo > +--echo 2. --master-data=2 > +--echo > +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ > +--exec $MYSQL_DUMP_SLAVE --compact --master-data=2 --gtid test > + > +--echo > +--echo 3. --master-data --single-transaction > +--echo > +--replace_regex /MASTER_LOG_POS=[0-9]+/MASTER_LOG_POS=BINLOG_START/ > +--exec $MYSQL_DUMP_SLAVE --compact --master-data --single-transaction --gtid test > + > + > + > --source include/rpl_end.inc > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

2 1

Re: [Maria-developers] [Commits] Rev 4259: MDEV-6120: When slave stops with error, error message should indicate the failing GTID in http://bazaar.launchpad.net/~maria-captains/maria/10.0
by Sergey Vojtovich 25 Jun '14

25 Jun '14

Hi Kristian, looks good. One minor question below. On Thu, Jun 19, 2014 at 02:50:44PM +0200, knielsen(a)knielsen-hq.org wrote: > At http://bazaar.launchpad.net/~maria-captains/maria/10.0 > > ------------------------------------------------------------ > revno: 4259 > revision-id: knielsen(a)knielsen-hq.org-20140619125043-acw2gx7rw90y3vqp > parent: svoj(a)mariadb.org-20140618141504-kqq6ws9gm93r2b2d > committer: Kristian Nielsen <knielsen(a)knielsen-hq.org> > branch nick: work-10.0 > timestamp: Thu 2014-06-19 14:50:43 +0200 > message: > MDEV-6120: When slave stops with error, error message should indicate the failing GTID > > If replication breaks in GTID mode, it is not trivial to determine the GTID of > the failing event group. This is a problem, as such GTID is needed eg. to > explicitly set @@gtid_slave_pos to skip to after that event group, or to > compare errors on different servers, etc. > > Fix by ensuring that relevant slave errors logged to the error log include the > GTID of the event group containing the problem event. ...skip... > === added file 'mysql-test/suite/rpl/t/rpl_gtid_errorlog.test' > --- a/mysql-test/suite/rpl/t/rpl_gtid_errorlog.test 1970-01-01 00:00:00 +0000 > +++ b/mysql-test/suite/rpl/t/rpl_gtid_errorlog.test 2014-06-19 12:50:43 +0000 > @@ -0,0 +1,94 @@ > +--source include/have_debug.inc > +--source include/master-slave.inc > + > +--echo *** Test MDEV-6120, output of current GTID when a replication error is logged to the errorlog *** > +--connection master > +CREATE TABLE t1(a INT PRIMARY KEY); > +--sync_slave_with_master > + > +--connection slave > +--source include/stop_slave.inc > +CHANGE MASTER TO master_use_gtid=slave_pos; > + > +--connection master > +INSERT INTO t1 VALUES (1); > +SET gtid_seq_no=100; > +INSERT INTO t1 VALUES (2); > +INSERT INTO t1 VALUES (3); > +INSERT INTO t1 VALUES (4); > +--save_master_pos > + > +--connection slave > +SET sql_log_bin=0; > +INSERT INTO t1 VALUES (2); > +SET sql_log_bin=1; > + > +START SLAVE; > +--let $slave_sql_errno=1062 > +--source include/wait_for_slave_sql_error.inc > + > +--source include/stop_slave.inc > +# Skip the problem event from the master. > +SET GLOBAL gtid_slave_pos= "0-1-100"; > +--source include/start_slave.inc > +--sync_with_master > + > +SELECT * FROM t1 ORDER BY a; > + > +--connection master > + > +SET @dbug_save= @@debug_dbug; > +SET debug_dbug= '+d,incident_database_resync_on_replace'; > +REPLACE INTO t1 VALUES (5); > +SET debug_dbug= @dbug_save; > +--save_master_pos > + > +--connection slave > +--let $slave_sql_errno=1590 > +--source include/wait_for_slave_sql_error.inc > +# ToDo no need to switch off GTID once MDEV-4937 is fixed > +--source include/stop_slave.inc > +CHANGE MASTER TO master_use_gtid=no; > +SET sql_slave_skip_counter=1; > +--source include/start_slave.inc > +--sync_with_master > +--source include/stop_slave.inc > +CHANGE MASTER TO master_use_gtid=slave_pos; > +--source include/start_slave.inc > + > +SELECT * FROM t1 ORDER BY a; > + > + > +# Check error log for correct messages. > +let $log_error_= `SELECT @@GLOBAL.log_error`; > +if(!$log_error_) > +{ > + # MySQL Server on windows is started with --console and thus > + # does not know the location of its .err log, use default location > + let $log_error_ = $MYSQLTEST_VARDIR/log/mysqld.2.err; > +} > +let LOG_ERROR=$log_error_; > + > +perl; > + use strict; > + use warnings; > + my $log_error= $ENV{'LOG_ERROR'} or die "LOG_ERROR not set"; > + open(FILE, "<", "$log_error") or die("Unable to open $log_error: $!\n"); > + my $found_dup= 0; > + my $found_incident= 0; > + while (<FILE>) { > + $found_dup= 1 > + if /Slave SQL: Error 'Duplicate entry .* on query\. .*Query: '.*', Gtid 0-1-100, Internal MariaDB error code:/ > + or /Slave SQL: Could not execute Write_rows.*table test.t1; Duplicate entry.*, Gtid 0-1-100, Internal MariaDB error/ > + ; > + $found_incident= 1 > + if /Slave SQL: The incident LOST_EVENTS occured on the master\. Message: <none>, Internal MariaDB error code: 1590/; > + } > + print "Duplicate key found: $found_dup; incident found: $found_incident\n"; > + close(FILE); > +EOF > + > +--connection master > +DROP TABLE t1; > + > +--source include/rpl_end.inc Why didn't you like include/search_pattern_in_file.inc? It should perfectly able to match your patterns. ...skip... Regards, Sergey

2 1

[Maria-developers] Review of MDEV-5730: enhance security using special compilation options
by Kristian Nielsen 24 Jun '14

24 Jun '14

> commit feab48657528b9bb40fb035d51bee28d93710c1e > Author: Sergei Golubchik <serg(a)mariadb.org> > Date: Mon Jun 16 21:39:09 2014 +0200 > > MDEV-5730 enhance security using special compilation options > > -Wl,-z,relro,-z,now > -pie > -fstack-protector --param=ssp-buffer-size=4 > -D_FORTIFY_SOURCE=2 The patch is ok to push with a clarified comment as requested below. I have checked the compiler/linker options added and commented some on them below. > diff --git a/CMakeLists.txt b/CMakeLists.txt > index a24f000..5b0d348 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -201,6 +201,20 @@ IF (WITH_ASAN) > ENDIF() > ENDIF() > > +OPTION(SECURITY_HARDENED "Use security-enhancing compiler features (stack protector, relro, etc)" ON) So these are on by default. This means that this patch will cause some performance impact. So having them on by default is a compromise between performance and security. You should probably include in the commit comment a discussion of why the security viewpoint was prefered over the performance viewpoint in this case (or a comment in the cmake file if you prefer). The proper way to handle a patch like this would be to measure the performance impact, eg. for single-threaded performance with some various simple sysbench loads. However, it is quite hard to do such measurements, as you need a setup that has low enough noise in results to detect sub-percentage differences of performance. I have had some success by locking processes to a single core and using `perf` measurements of cycles spent (rather than wall-clock time), but even so the results can be tricky to interpret. So it may be too much work for a small patch like this. (And even if the performance improvement can be measured, how can we measure the security impact to weight them against one another?) My personal opinion is that I do not buy into this "security" mindset much. I do not think these options improve security significantly in practise for most users. So while the performance impact is probably also not very significant, why accept it at all (by default)? But the opposite viewpoint exists and is valid as well. So I think this patch is ok if you add a discussion of why it was chosen to do it this way. (Even if the reason is "Most distros do it" - at least then it is documented what the reason is). Also, the option should mention the possible performance impact. Suggestion: "Use compiler options (stack protector, relro, etc) that may slightly enhance security of the resulting binary, at the possible cost of some small performance impact." > +IF(SECURITY_HARDENED) > + # security-enhancing flags > + MY_CHECK_AND_SET_COMPILER_FLAG("-pie -fPIC") -fPIC can have a significant impact on performance on some platforms. For example on 32-bit x86, it requires extra instructions at the head of every function, and it makes one less register available for code generation on a CPU already starved for registers. On 64-bit x86, the impact is much smaller, as the architecture was enhanced to better support -fPIC. However, I seem to recall that the server is anyway built with -fPIC, for libmysqld or something like that? In which case this has no extra impact. > + MY_CHECK_AND_SET_COMPILER_FLAG("-Wl,-z,relro,-z,now") This option seems to cause all relocations to happen at load time, rather than lazily on-demand. The impact should be higher memory usage and some extra CPU time for these relocations, many of which may never be needed. This option will have the biggest effect on the client programs probably, as server startup is already fairly heavy. Probably even for client programs the added cost will be not very significant, as even the client programs are usually called to do significant processing (eg. connect to a database and so on). I guess these options would mostly hurt small utility programs for shell scripts, like `mv` or so. > + MY_CHECK_AND_SET_COMPILER_FLAG("-fstack-protector --param=ssp-buffer-size=4") This also adds extra code and memory loads to each function, so it will have some impact to performance. It is probably the most significant impact of all the added options, though without detailed analysis this is only guesswork, of course. > + # sometimes _FORTIFY_SOURCE is predefined > + INCLUDE(CheckSymbolExists) > + CHECK_SYMBOL_EXISTS(_FORTIFY_SOURCE "" HAVE_FORTIFY_SOURCE) > + IF(NOT HAVE_FORTIFY_SOURCE) > + ADD_DEFINITIONS(-D_FORTIFY_SOURCE=2) > + ENDIF() > +ENDIF() FORTIFY_SOURCE also adds some runtime overhead, but it is not very much, it is an extra argument to memcpy() and friends about destination buffer size. Probably the way or code is written, there will be few places where this option makes any difference, so the performance impact does not seem likely to be very significant. Hope this helps, - Kristian.

3 2

[Maria-developers] [GSoC] Optimize mysql-test-runs - Setback
by Pablo Estrada 24 Jun '14

24 Jun '14

Hello Sergei, Elena and all, Today while working on the script, I found and fixed an issue: There is some faulty code code in my script that is in charge of collecting the statistics about whether a test failure was caught or not (here <https://github.com/pabloem/Kokiri/blob/master/basic_simulator.py#L393>). I looked into fixing it, and then I could see another *problem*: The *recall numbers* that I had collected previously were *too high*. The actual recall numbers, once we consider the test failures that are *not caught*, are disappointingly lower. I won't show you results yet, since I want to make sure that the code has been fixed, and I have accurate tests first. This is all for now. The strategy that I was using is a lot less effective than it seemed initially. I will send out a more detailed report with results, my opinion on the weak points of the strategy, and ideas, including a roadmap to try to improve results. Regards. All feedback is welcome. Pablo

3 18

[Maria-developers] Doubt in bin logging for CREATE VIEW
by sriram patil 24 Jun '14

24 Jun '14

Hi All, I was checking the behavior of bin logging in various methods. I found out CREATE VIEW differs a little bit than others in bin logging. The query is written into bin log if query is executed successfully. If there is any error the query is not logged. This behavior is observed across all the commands. In case of CREATE VIEW (method: mysql_create_view file: sql/sql_view.cc), while the bin logging is skipped for most of the errors by jumping to "err" label, it is not skipped in following code snippet (line number: 616) res= mysql_register_view(thd, view, mode); /* View TABLE_SHARE must be removed from the table definition cache in order to make ALTER VIEW work properly. Otherwise, we would not be able to detect meta-data changes after ALTER VIEW. */ if (!res) tdc_remove_table(thd, TDC_RT_REMOVE_ALL, view->db, view->table_name, false); if (mysql_bin_log.is_open()) { // this is where the log is written } If an error occurs in mysql_register_view, e.g. ER_OUT_OF_RESOURCES, ER_TABLE_EXISTS_ERROR, etc. the query is still logged. Is there a specific reason why it is logged even though an error occurs? Or is it a bug? Thanks & Regards, Sriram

4 9

Re: [Maria-developers] [Commits] 7d386bb: MDEV-4549 [PATCH] Clean up code working with ACL tables
by Sergey Vojtovich 23 Jun '14

23 Jun '14

Hi Sergei, ok to push. A few minor questions inline. On Mon, Jun 16, 2014 at 09:34:21AM +0200, serg(a)mariadb.org wrote: > revision-id: 7d386bbce216745ba28df728f45a0b796fcced98 > parent(s): f61f36b386e8d0c6448297bb84c92e8d9b82be6a > committer: Sergei Golubchik > branch nick: maria > timestamp: 2014-06-15 17:12:57 +0200 > message: > > MDEV-4549 [PATCH] Clean up code working with ACL tables > > enum values to index different ACL tables, instead of hard-coded numbers > (even different in diffent functions). Also factor out TABLE_LIST > initialization into a separate function. > > --- > mysql-test/r/not_embedded_server.result | 4 +- > mysql-test/r/sp-destruct.result | 3 - > .../suite/rpl/r/rpl_tmp_table_and_DDL.result | 22 +- > mysql-test/t/not_embedded_server.test | 2 +- > mysql-test/t/sp-destruct.test | 1 - > sql/sql_acl.cc | 533 +++++++++------------ > 6 files changed, 235 insertions(+), 330 deletions(-) > > diff --git a/mysql-test/r/not_embedded_server.result b/mysql-test/r/not_embedded_server.result > index 2295276..6dda855 100644 > --- a/mysql-test/r/not_embedded_server.result > +++ b/mysql-test/r/not_embedded_server.result > @@ -1,4 +1,4 @@ > -call mtr.add_suppression("Can't open and lock privilege tables: Table 'host' was not locked with LOCK TABLES"); > +call mtr.add_suppression("Can't open and lock privilege tables: Table 'roles_mapping' was not locked with LOCK TABLES"); > SHOW VARIABLES like 'slave_skip_errors'; > Variable_name Value > slave_skip_errors OFF Why did it change (here and in other tests)? I didn't notice any table order change in the code. ...skip... > diff --git a/mysql-test/r/sp-destruct.result b/mysql-test/r/sp-destruct.result > index 172e40c..fe68229 100644 > --- a/mysql-test/r/sp-destruct.result > +++ b/mysql-test/r/sp-destruct.result > @@ -128,11 +128,8 @@ CREATE FUNCTION f1() RETURNS INT RETURN 1; > RENAME TABLE mysql.procs_priv TO procs_priv_backup; > FLUSH TABLE mysql.procs_priv; > DROP FUNCTION f1; > -ERROR 42S02: Table 'mysql.procs_priv' doesn't exist > SHOW WARNINGS; > Level Code Message > -Error 1146 Table 'mysql.procs_priv' doesn't exist > -Warning 1405 Failed to revoke all privileges to dropped routine > # Restore the procs_priv table > RENAME TABLE procs_priv_backup TO mysql.procs_priv; > FLUSH TABLE mysql.procs_priv; Hmm... why? ...skip... > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 5e8e0e7..442d512 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -380,7 +380,7 @@ class ACL_PROXY_USER :public ACL_ACCESS > MYSQL_PROXIES_PRIV_PROXIED_USER, > MYSQL_PROXIES_PRIV_WITH_GRANT, > MYSQL_PROXIES_PRIV_GRANTOR, > - MYSQL_PROXIES_PRIV_TIMESTAMP } old_acl_proxy_users; > + MYSQL_PROXIES_PRIV_TIMESTAMP } proxy_table_fields; > public: > ACL_PROXY_USER () {}; > > @@ -760,6 +760,99 @@ static int traverse_role_graph_down(ACL_USER_BASE *, void *, > int (*) (ACL_USER_BASE *, ACL_ROLE *, void *)); > > /* > + Enumeration of ACL/GRANT tables in the mysql database > +*/ > +enum enum_acl_tables > +{ > + USER_TABLE, > + DB_TABLE, > + TABLES_PRIV_TABLE, > + COLUMNS_PRIV_TABLE, > +#define FIRST_OPTIONAL_TABLE HOST_TABLE > + HOST_TABLE, > + PROCS_PRIV_TABLE, > + PROXIES_PRIV_TABLE, > + ROLES_MAPPING_TABLE, > + TABLES_MAX // <== always the last > +}; > +// bits for init_priv_tables > +static const int Table_user= 1 << USER_TABLE; > +static const int Table_db= 1 << DB_TABLE; > +static const int Table_tables_priv= 1 << TABLES_PRIV_TABLE; > +static const int Table_columns_priv= 1 << COLUMNS_PRIV_TABLE; > +static const int Table_host= 1 << HOST_TABLE; > +static const int Table_procs_priv= 1 << PROCS_PRIV_TABLE; > +static const int Table_proxies_priv= 1 << PROXIES_PRIV_TABLE; > +static const int Table_roles_mapping= 1 << ROLES_MAPPING_TABLE; > + > +const LEX_STRING acl_table_names[]= // matches enum_acl_tables > +{ > + { C_STRING_WITH_LEN("user") }, > + { C_STRING_WITH_LEN("db") }, > + { C_STRING_WITH_LEN("tables_priv") }, > + { C_STRING_WITH_LEN("columns_priv") }, > + { C_STRING_WITH_LEN("host") }, > + { C_STRING_WITH_LEN("procs_priv") }, > + { C_STRING_WITH_LEN("proxies_priv") }, > + { C_STRING_WITH_LEN("roles_mapping") } > +}; > + > +/** > + Initialize a TABLE_LIST array to a set of acl/grant tables. > + All tables will be opened with the same lock type, either read or write. > + > + @retval !0 a pointer to the first initialized element in the TABLE_LIST > + @retval 0 replication filters matched. Abort operation, but return OK (!) > +*/ > +TABLE_LIST *init_priv_tables(THD *thd, TABLE_LIST *tables, > + enum thr_lock_type lock_type, int tables_to_open) Nice clean-up, I assume there was a reason not rewrite open_grant_tables() instead. > +{ > + int prev= -1; > + bzero(tables, sizeof(TABLE_LIST) * TABLES_MAX); > + for (int cur=0, mask=1; mask <= tables_to_open; cur++, mask <<= 1) > + { > + if ((tables_to_open & mask) == 0) > + continue; > + tables[cur].init_one_table(C_STRING_WITH_LEN("mysql"), > + acl_table_names[cur].str, > + acl_table_names[cur].length, > + acl_table_names[cur].str, lock_type); > + tables[cur].open_type= OT_BASE_ONLY; > + if (lock_type >= TL_WRITE_ALLOW_WRITE) > + tables[cur].updating= 1; > + if (cur >= FIRST_OPTIONAL_TABLE) > + tables[cur].open_strategy= TABLE_LIST::OPEN_IF_EXISTS; > + if (prev != -1) > + tables[cur].next_local= tables[cur].next_global= & tables[prev]; > + prev= cur; > + } > + > +#ifdef HAVE_REPLICATION > + if (lock_type >= TL_WRITE_ALLOW_WRITE && thd->slave_thread && !thd->spcont) > + { > + /* > + GRANT and REVOKE are applied the slave in/exclusion rules as they are > + some kind of updates to the mysql.% tables. > + */ > + Rpl_filter *rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter; > + if (rpl_filter->is_on() && !rpl_filter->tables_ok(0, tables)) > + return NULL; > + } > +#endif Was it a bug that mysql_grant_role() didn't have this code? open_grant_tables() does updating= 0 after tables_ok(). Is that needed? ...skip... > @@ -5783,8 +5796,9 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, > } > } > > - if (replace_routine_table(thd, grant_name, tables[1].table, *Str, > - db_name, table_name, is_proc, rights, > + if (no_such_table(tables + PROCS_PRIV_TABLE) || > + replace_routine_table(thd, grant_name, tables[PROCS_PRIV_TABLE].table, > + *Str, db_name, table_name, is_proc, rights, > revoke_grant) != 0) > { > result= TRUE; Why no_such_table() here? ...skip... > @@ -6250,7 +6232,7 @@ bool mysql_grant(THD *thd, const char *db, List <LEX_USER> &list, > ulong db_rights= rights & DB_ACLS; > if (db_rights == rights) > { > - if (replace_db_table(tables[1].table, db, *Str, db_rights, > + if (replace_db_table(tables[DB_TABLE].table, db, *Str, db_rights, > revoke_grant)) > result= -1; > } Tabs here (and in a few hunks below). > @@ -6262,9 +6244,11 @@ bool mysql_grant(THD *thd, const char *db, List <LEX_USER> &list, > } > else if (is_proxy) > { > - if (replace_proxies_priv_table (thd, tables[1].table, Str, proxied_user, > - rights & GRANT_ACL ? TRUE : FALSE, > - revoke_grant)) > + if (no_such_table(tables + PROXIES_PRIV_TABLE) || > + replace_proxies_priv_table (thd, tables[PROXIES_PRIV_TABLE].table, > + Str, proxied_user, > + rights & GRANT_ACL ? TRUE : FALSE, > + revoke_grant)) > result= -1; > } > if (Str->is_role()) Why no_such_table() here? ...skip... Regards, Sergey

2 2

[Maria-developers] [GSoC] Week Five!
by sriram patil 23 Jun '14

23 Jun '14

Hi All, Following is the link to my blog post of fifth week into GSoC. http://goo.gl/x7dtcn Suggestions and/or review are welcome! Thanks, Sriram

1 0

[Maria-developers] Java Connector Coding till 21st June
by Puneet Dewan 21 Jun '14

21 Jun '14

Hi Everyone. Test case class(ServersidePreparedStatementTest.java) was written for checking the value of statement id. This week code review was done by Massimo and Georg. There pointers were really valuable as data was not being send in a proper format to the server for execute(com_stmt_execute) method : 1.NullBitMap was set incorrectly. 2.Creation of a new class extending PacketOuptutStream e.g integer of 4 bytes can be sent.Currently if an integer is sent it is sent as a byte. 3.Also I was expecting when we do a *prepare* for a statement like insert into XYZ(id) values(?) where id is of type Integer. Then server shall be returning integer as a type. This was a false assumption.It does not return the type. So now on the basis of value set for *id as example*,decide on the type to be sent from execute. So this week will fix the above things. Looking forward for suggestions. Regards Puneet.

1 0

Re: [Maria-developers] [Commits] 22f532c: MDEV-4260 Don't create frm files for temporary tables
by Sergey Vojtovich 20 Jun '14

20 Jun '14

Hi Sergei, ok to push. One note inline. On Fri, Jun 20, 2014 at 11:29:36AM +0200, serg(a)mariadb.org wrote: > revision-id: 22f532cfb2f40132e2f7b079a3aafb109af92c26 > parent(s): fb8818c1aff9ce67b74f965c019cad93addd811e > committer: Sergei Golubchik > branch nick: maria > timestamp: 2014-06-20 11:29:12 +0200 > message: > > MDEV-4260 Don't create frm files for temporary tables > > * Don't write frm for tmp tables > * pass frm image down to open_table_uncached, when possible > * don't use truncate-by-recreate for temp tables - cannot recreate > without frm, and delete_all_rows is faster anyway > * remove HTON_FLUSH_AFTER_RENAME (BDB-ism, unused for years) > > --- > mysql-test/r/temp_table_frm.result | 20 +++++++++++ > mysql-test/t/temp_table_frm.test | 13 ++++++++ > sql/handler.h | 2 +- > sql/sql_base.cc | 14 +++++++- > sql/sql_base.h | 3 +- > sql/sql_table.cc | 27 +++------------ > sql/sql_truncate.cc | 68 +------------------------------------- > sql/unireg.cc | 3 +- > 8 files changed, 56 insertions(+), 94 deletions(-) > ...skip... > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 0558066..7d69ccb 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -5561,7 +5563,17 @@ TABLE *open_table_uncached(THD *thd, handlerton *hton, > strend(saved_cache_key)+1, tmp_path); > share->db_plugin= ha_lock_engine(thd, hton); > > - if (open_table_def(thd, share, GTS_TABLE | GTS_USE_DISCOVERY)) > + /* > + Use the frm image, if possible, open the file otherwise. > + > + The image might be unavailable in ALTER TABLE, when the discovering > + engine took over the ownership (see TABLE::read_frm_image). > + */ > + int res= frm->str > + ? res= share->init_from_binary_frm_image(thd, false, frm->str, frm->length) Excessive "res="? Regards, Sergey

2 1

[Maria-developers] [GSoC] Week Four!
by sriram patil 19 Jun '14

19 Jun '14

Hi All, Sorry for a little delay in posting the blog link this week. Following is the blog link about my fourth week into GSoC. http://goo.gl/W54cvE Suggestions and/or reviews are welcome! Thanks, Sriram

4 4