From: Date: June 28 2007 9:10am Subject: Re: [PATCH] first version of proposed patch for #28760 List-Archive: http://lists.mysql.com/internals/34785 Message-Id: <32556AAB-69BE-4149-A951-4E870C3AFF0A@classdump.org> MIME-Version: 1.0 (Apple Message framework v752.3) Content-Type: text/plain; charset=US-ASCII; delsp=yes; format=flowed Content-Transfer-Encoding: 7bit Hi Chad! Sorry for the delay, I've been very busy. Now that I managed to actually test the patch again, here it is. The diff is against a mysql-5.1 bk snapshot from about an hour ago. I updated the patch to include the changes made in the meantime (including the additions of Last_*_Error columns to show_master_info). The 'delay' column goes at the very end. Here's my original message that's been sitting in my drafts folder for way too long: On Jun 11, 2007, at 5:13 PM, Chad MILLER wrote: > On 6 Jun 2007, at 06:08, Kay Roepke wrote: >> >> a) Should the delay setting be written to master.info and if it >> should, in which line should it go. >> The reason I ask is because the SSL settings are optional in that >> file and adding 'delay' at the end could cause compatibility >> problems. >> However adding it in the middle seems like a worse idea still. > > Yes, you must write it to master.info so that it's restored on > recovery. > > It doesn't understand items that are not in order, and this > includes SSL. The delay setting must go at the end, after SSL > settings, which must exist as a prerequisite. Is it true that the SSL settings are always present in master.info, even if SSL support hasn't been compiled in? It seems true to me, but I'm not 100% sure. If this is so, everything should work. I tested using the compile-dist script in BUILD/ on Mac OS 10.4.10. > You should be able to peek inside the event to see what type it > is. Lars notes that rotate-log events can be created by the I/O > thread, and so shouldn't be delayed. Ok, this patch includes code to ignore rotate-log events. It seems to work fine, I'm no longer seeing the delay I mentioned earlier. Though, as I write this, maybe it's better to look at the originating server id and decided on the basis of that? It sounds much easier and more straightforward to implement/read than to create special cases for certain events. >> d) Is the format of the patch ok, I had problems sending >> attachments to the list, so I put it in the body... > > Perhaps there were problems: I notice several places where there > are tabs, and our style guidelines forbid them in code. I noted them. Very sorry about the tabs, I thought I had fixed that nuisance. >> diff -urN mysql-5.1-bk-clean/sql/mysqld.cc mysql-5.1-bk/sql/mysqld.cc >> --- mysql-5.1-bk-clean/sql/mysqld.cc 2007-06-04 12:14:18.000000000 >> +0200 >> +++ mysql-5.1-bk/sql/mysqld.cc 2007-06-06 03:02:56.000000000 +0200 >> @@ -574,7 +574,7 @@ >> File_parser_dummy_hook file_parser_dummy_hook; >> /* replication parameters, if master_host is not NULL, we are a >> slave */ >> -uint master_port= MYSQL_PORT, master_connect_retry = 60; >> +uint master_port= MYSQL_PORT, master_connect_retry= 60, >> master_delay= 0; > > Think of the resulting diff when changing lines. Avoid removing a > line and adding a line, because a change to the removed line would > cause merge conflicts. There are a lot of developers working on > mysql source each with dozens of trees, and this is a real problem. I understand. However, in this case, the line also contains a style guideline fix, so I thought it would be appropriate. >> diff -urN mysql-5.1-bk-clean/sql/rpl_mi.cc mysql-5.1-bk/sql/rpl_mi.cc >> --- mysql-5.1-bk-clean/sql/rpl_mi.cc 2007-04-16 18:16:13.000000000 >> +0200 >> +++ mysql-5.1-bk/sql/rpl_mi.cc 2007-06-06 03:03:36.000000000 +0200 >> @@ -30,7 +30,8 @@ >> MASTER_INFO::MASTER_INFO() >> :ssl(0), fd(-1), io_thd(0), inited(0), >> abort_slave(0),slave_running(0), >> - ssl_verify_server_cert(0), slave_run_id(0) >> + ssl_verify_server_cert(0), slave_run_id(0), >> + delay(0) > > Same here. "delay(0)," before the end would avert a merge conflict. Fixed. I moved it up a line, in order to not add a comma at the end of the previous line (which would result in a two line diff anyway). >> @@ -1784,7 +1787,20 @@ >> --rli->slave_skip_counter; >> pthread_mutex_unlock(&rli->data_lock); >> if (reason == Log_event::EVENT_SKIP_NOT) >> + { >> + long now= (long)((time_t)time((time_t*) 0)); > > Ugh. With a different type, could this just be Yes, sorry. :) Below is an updated version. It has: 1) uint32 delay; 2) reads/writes delay from/to master.info (as the last line) 3) moved the delay column in show_master_info to last position 4) fixes the tab issues 5) I avoided changing lines, unless absolutely necessary or fixing style guideline violations. cheers, -k diff -urN mysql-5.1-bk-clean/sql/lex.h mysql-5.1-bk/sql/lex.h --- mysql-5.1-bk-clean/sql/lex.h 2007-04-02 10:54:27.000000000 +0200 +++ mysql-5.1-bk/sql/lex.h 2007-06-06 01:11:43.000000000 +0200 @@ -300,6 +300,7 @@ { "LOW_PRIORITY", SYM(LOW_PRIORITY)}, { "MASTER", SYM(MASTER_SYM)}, { "MASTER_CONNECT_RETRY", SYM(MASTER_CONNECT_RETRY_SYM)}, + { "MASTER_DELAY", SYM(MASTER_DELAY_SYM)}, { "MASTER_HOST", SYM(MASTER_HOST_SYM)}, { "MASTER_LOG_FILE", SYM(MASTER_LOG_FILE_SYM)}, { "MASTER_LOG_POS", SYM(MASTER_LOG_POS_SYM)}, diff -urN mysql-5.1-bk-clean/sql/mysqld.cc mysql-5.1-bk/sql/mysqld.cc --- mysql-5.1-bk-clean/sql/mysqld.cc 2007-06-28 08:37:56.000000000 +0200 +++ mysql-5.1-bk/sql/mysqld.cc 2007-06-28 08:38:25.000000000 +0200 @@ -577,7 +577,8 @@ File_parser_dummy_hook file_parser_dummy_hook; /* replication parameters, if master_host is not NULL, we are a slave */ -uint master_port= MYSQL_PORT, master_connect_retry = 60; +uint master_port= MYSQL_PORT, master_connect_retry= 60; +uint32 master_delay= 0; uint report_port= MYSQL_PORT; ulong master_retry_count=0; char *master_user, *master_password, *master_host, *master_info_file; diff -urN mysql-5.1-bk-clean/sql/rpl_mi.cc mysql-5.1-bk/sql/rpl_mi.cc --- mysql-5.1-bk-clean/sql/rpl_mi.cc 2007-06-28 08:37:57.000000000 +0200 +++ mysql-5.1-bk/sql/rpl_mi.cc 2007-06-28 08:54:20.000000000 +0200 @@ -31,6 +31,7 @@ :Slave_reporting_capability("I/O"), ssl(0), fd(-1), io_thd(0), inited(0), abort_slave(0),slave_running(0), + delay(0), ssl_verify_server_cert(0), slave_run_id(0) { host[0] = 0; user[0] = 0; password[0] = 0; @@ -68,8 +69,9 @@ strmake(mi->user, master_user, sizeof(mi->user) - 1); if (master_password) strmake(mi->password, master_password, MAX_PASSWORD_LENGTH); - mi->port = master_port; - mi->connect_retry = master_connect_retry; + mi->port= master_port; + mi->connect_retry= master_connect_retry; + mi->delay= master_delay; mi->ssl= master_ssl; if (master_ssl_ca) @@ -94,8 +96,11 @@ /* 5.1.16 added value of master_ssl_verify_server_cert */ LINE_FOR_MASTER_SSL_VERIFY_SERVER_CERT= 15, + /* added value of master_delay */ + LINE_FOR_MASTER_DELAY= 16, + /* Number of lines currently used when saving master info file */ - LINES_IN_MASTER_INFO= LINE_FOR_MASTER_SSL_VERIFY_SERVER_CERT + LINES_IN_MASTER_INFO= LINE_FOR_MASTER_DELAY }; int init_master_info(MASTER_INFO* mi, const char* master_info_fname, @@ -197,6 +202,7 @@ mi->fd = fd; int port, connect_retry, master_log_pos, lines; int ssl= 0, ssl_verify_server_cert= 0; + int delay= 0; char *first_non_digit; /* @@ -282,6 +288,10 @@ init_intvar_from_file(&ssl_verify_server_cert, &mi->file, 0)) goto errwithmsg; + if (lines >= LINE_FOR_MASTER_DELAY && + init_intvar_from_file(&delay, &mi->file, master_delay)) + goto errwithmsg; + } #ifndef HAVE_OPENSSL @@ -300,6 +310,7 @@ mi->connect_retry= (uint) connect_retry; mi->ssl= (my_bool) ssl; mi->ssl_verify_server_cert= ssl_verify_server_cert; + mi->delay= (uint32) delay; } DBUG_PRINT("master_info",("log_file_name: %s position: %ld", mi->master_log_name, @@ -381,13 +392,14 @@ my_b_seek(file, 0L); my_b_printf(file, - "%u\n%s\n%s\n%s\n%s\n%s\n%d\n%d\n%d\n%s\n%s\n%s\n%s\n%s \n%d\n", + "%u\n%s\n%s\n%s\n%s\n%s\n%d\n%d\n%d\n%s\n%s\n%s\n%s\n%s \n%d\n%u\n", LINES_IN_MASTER_INFO, mi->master_log_name, llstr(mi->master_log_pos, lbuf), mi->host, mi->user, mi->password, mi->port, mi->connect_retry, (int)(mi->ssl), mi->ssl_ca, mi->ssl_capath, mi- >ssl_cert, - mi->ssl_cipher, mi->ssl_key, mi->ssl_verify_server_cert); + mi->ssl_cipher, mi->ssl_key, mi->ssl_verify_server_cert, + mi->delay); DBUG_RETURN(-flush_io_cache(file)); } diff -urN mysql-5.1-bk-clean/sql/rpl_mi.h mysql-5.1-bk/sql/rpl_mi.h --- mysql-5.1-bk-clean/sql/rpl_mi.h 2007-06-28 08:37:57.000000000 +0200 +++ mysql-5.1-bk/sql/rpl_mi.h 2007-06-28 08:38:25.000000000 +0200 @@ -100,6 +100,8 @@ */ long clock_diff_with_master; + uint32 delay; ///< Replication delay, in seconds + }; void init_master_info_with_options(MASTER_INFO* mi); diff -urN mysql-5.1-bk-clean/sql/slave.cc mysql-5.1-bk/sql/slave.cc --- mysql-5.1-bk-clean/sql/slave.cc 2007-06-28 08:37:57.000000000 +0200 +++ mysql-5.1-bk/sql/slave.cc 2007-06-28 08:47:00.000000000 +0200 @@ -1205,6 +1205,7 @@ field_list.push_back(new Item_empty_string("Last_IO_Error", 20)); field_list.push_back(new Item_return_int("Last_SQL_Errno", 4, MYSQL_TYPE_LONG)); field_list.push_back(new Item_empty_string("Last_SQL_Error", 20)); + field_list.push_back(new Item_return_int("Delay", 10, MYSQL_TYPE_LONG)); if (protocol->send_fields(&field_list, Protocol::SEND_NUM_ROWS | Protocol::SEND_EOF)) @@ -1324,6 +1325,8 @@ // Last_SQL_Error protocol->store(mi->rli.last_error().message, &my_charset_bin); + protocol->store(mi->delay); + pthread_mutex_unlock(&mi->rli.data_lock); pthread_mutex_unlock(&mi->data_lock); @@ -1740,7 +1743,22 @@ --rli->slave_skip_counter; pthread_mutex_unlock(&rli->data_lock); if (reason == Log_event::EVENT_SKIP_NOT) + { + time_t now; + (void)time(&now); + DBUG_PRINT("info", ("ev->when= %lu now= %lu delay= %lu event= %s", + ev->when, now, rli->mi->delay, ev- >get_type_str())); + if ((rli->mi->delay != 0) && + (ev->get_type_code() != ROTATE_EVENT) && + ((now - ev->when) < rli->mi->delay)) + { + DBUG_PRINT("info", ("delaying replication event %s %lu secs", + ev->get_type_str(), rli->mi->delay - (now - ev->when))); + safe_sleep(thd,rli->mi->delay - (now - ev->when), (CHECK_KILLED_FUNC)sql_slave_killed, + (void*)rli); + } exec_res= ev->apply_event(rli); + } #ifndef DBUG_OFF /* This only prints information to the debug trace. diff -urN mysql-5.1-bk-clean/sql/slave.h mysql-5.1-bk/sql/slave.h --- mysql-5.1-bk-clean/sql/slave.h 2007-06-28 08:37:57.000000000 +0200 +++ mysql-5.1-bk/sql/slave.h 2007-06-28 08:38:25.000000000 +0200 @@ -198,6 +198,7 @@ /* the master variables are defaults read from my.cnf or command line */ extern uint master_port, master_connect_retry, report_port; +extern uint32 master_delay; extern char * master_user, *master_password, *master_host; extern char *master_info_file, *relay_log_info_file, *report_user; extern char *report_host, *report_password; diff -urN mysql-5.1-bk-clean/sql/sql_lex.h mysql-5.1-bk/sql/sql_lex.h --- mysql-5.1-bk-clean/sql/sql_lex.h 2007-06-28 08:37:57.000000000 +0200 +++ mysql-5.1-bk/sql/sql_lex.h 2007-06-28 08:38:25.000000000 +0200 @@ -186,6 +186,7 @@ { char *host, *user, *password, *log_file_name; uint port, connect_retry; + uint32 delay; ulonglong pos; ulong server_id; /* diff -urN mysql-5.1-bk-clean/sql/sql_repl.cc mysql-5.1-bk/sql/ sql_repl.cc --- mysql-5.1-bk-clean/sql/sql_repl.cc 2007-06-28 08:37:57.000000000 +0200 +++ mysql-5.1-bk/sql/sql_repl.cc 2007-06-28 08:38:26.000000000 +0200 @@ -1130,6 +1130,8 @@ mi->port = lex_mi->port; if (lex_mi->connect_retry) mi->connect_retry = lex_mi->connect_retry; + if (lex_mi->delay) + mi->delay= lex_mi->delay; if (lex_mi->ssl != LEX_MASTER_INFO::SSL_UNCHANGED) mi->ssl= (lex_mi->ssl == LEX_MASTER_INFO::SSL_ENABLE); diff -urN mysql-5.1-bk-clean/sql/sql_yacc.yy mysql-5.1-bk/sql/ sql_yacc.yy --- mysql-5.1-bk-clean/sql/sql_yacc.yy 2007-06-28 08:37:57.000000000 +0200 +++ mysql-5.1-bk/sql/sql_yacc.yy 2007-06-28 08:38:26.000000000 +0200 @@ -769,6 +769,7 @@ %token LOW_PRIORITY %token LT /* OPERATOR */ %token MASTER_CONNECT_RETRY_SYM +%token MASTER_DELAY_SYM %token MASTER_HOST_SYM %token MASTER_LOG_FILE_SYM %token MASTER_LOG_POS_SYM @@ -1490,6 +1491,11 @@ { Lex->mi.connect_retry = $3; } + | + MASTER_DELAY_SYM EQ ulong_num + { + Lex->mi.delay = $3; + } | MASTER_SSL_SYM EQ ulong_num { Lex->mi.ssl= $3 ? @@ -9956,6 +9962,7 @@ | MASTER_PASSWORD_SYM {} | MASTER_SERVER_ID_SYM {} | MASTER_CONNECT_RETRY_SYM {} + | MASTER_DELAY_SYM {} | MASTER_SSL_SYM {} | MASTER_SSL_CA_SYM {} | MASTER_SSL_CAPATH_SYM {}