Re: [PATCH] first version of proposed patch for #28760

[Kay Roepke]

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	{}