maria-developers team mailing list archive

[Sachin Setiya <sachin.setiya@xxxxxxxxxxx>]

Hi Kristian!

On Sun, Jan 8, 2017 at 4:09 PM, Kristian Nielsen
<knielsen@xxxxxxxxxxxxxxx> wrote:
> Sachin Setiya <sachin.setiya@xxxxxxxxxxx> writes:
>
>> I am stuck at one problem in Mdev-10664. Suppose there is multisource
>> replication('master1' and 'master2' )
>> and we want to  update status var, How to know which master_info to
>> update ?. Does slave threads have current
>> replication channel name? (I was not able to find any , I have looked
>> in rpl_group_info)
>
> rgi->rli->mi
Thanks.
>
>> diff --git a/mysql-test/suite/multi_source/multi_parallel.test b/mysql-test/suite/multi_source/multi_parallel.test
>> new file mode 100644
>> index 0000000..e27345d
>> --- /dev/null
>> +++ b/mysql-test/suite/multi_source/multi_parallel.test
>
>> +--replace_result $SERVER_MYPORT_1 MYPORT_1
>> +eval change master 'master1' to
>> +master_port=$SERVER_MYPORT_1,
>> +master_host='127.0.0.1',
>> +master_user='root',
>> +master_heartbeat_period = 25;
>> +
>> +
>> +
>> +--replace_result $SERVER_MYPORT_2 MYPORT_2
>> +eval change master 'master2' to
>> +master_port=$SERVER_MYPORT_2,
>> +master_host='127.0.0.1',
>> +master_user='root',
>> +master_heartbeat_period=35;
>
> Why do you set heartbeat_period here?
>
I was copying from other tests in multi_source, so I copied it without
thinking, sorry
Reverted.
>> +show variables like 'slave%';
>
> Why do you need to show all these variables? It is very likely to cause the
> test to fail in some environments where the output may differ for whatever
> reason.
>
Sorry for this,I forgot to remove it , I was experimenting with
slave_parallel_mode and slave_parallel threads , so I added this
To my surprise slave_parallel_mode is replication channel specific ,
while slave_parallel threadsis a global variable, Why So ?
And in mariadb documentation I was unable to find which variable is
replication channel specific and which variable is global one.
Reverted
>> +# Cleanup
>> +stop all slaves;
>> +--source include/wait_for_slave_to_stop.inc
>
> This will only wait for one slave to stop, won't it?
>
yup, changed.
>> diff --git a/sql/log_event.cc b/sql/log_event.cc
>> index ced2626..c041116 100644
>> --- a/sql/log_event.cc
>> +++ b/sql/log_event.cc
>> @@ -6780,6 +6780,17 @@ Gtid_log_event::do_apply_event(rpl_group_info *rgi)
>>    }
>>
>>    DBUG_ASSERT((bits & OPTION_GTID_BEGIN) == 0);
>> +  mysql_mutex_lock(&LOCK_active_mi);
>
> Ouch! You are now taking an expensive global lock for each and every
> transaction :-( Surely this will hurt parallel replication performance.
>
Actually I was thinking the same, But I do not know how to get the master info
So I used the same approch as in method show_slave_ddl_stmts.
I no longer use mutex.
> This seems completely unnecessary. For example, if no flags are set you are
> not doing anything here, so no lock needed at all. And even if a flag is
> set, there is no need to take LOCK_active_mi, the Master_info cannot go away
> in the middle of do_apply_event(). According to my understanding,
> incremention a status variable should not require taking a global mutex at
> all.
Yep, because if there is DDL statement or non transnational statement
it is executed alone.
So no need of mutex.
>
>> +  Master_info *mi= master_info_index->get_master_info(&thd->variables.default_master_connection,
>> +                   Sql_condition::WARN_LEVEL_NOTE);
>> +  if (mi)
>> +  {
>> +    if (flags2 & FL_DDL)
>> +      mi->total_ddl_statements++;
>> +    if (!(flags & FL_TRANSACTIONAL))
>> +      mi->total_non_trans_statements++;
>
> The name "statements" is very confusing here, since you are counting event
> groups, not individual statements.
>
Changed It to event total_ddl_events , total_non_trans_events
> Did you consider writing documentation for the feature? Writing the
> documentation to explain to users exactly how the feature work could help
> clarify such issues for yourself also, before coding...
>
Never tried it , Will try it for next bug/task.

>> diff --git a/sql/mysqld.cc b/sql/mysqld.cc
>> index adb1b27..5fcaf34 100644
>> --- a/sql/mysqld.cc
>> +++ b/sql/mysqld.cc
>
>> @@ -8473,6 +8525,8 @@ SHOW_VAR status_vars[]= {
>>    {"Slave_retried_transactions",(char*)&slave_retried_transactions, SHOW_LONG},
>>    {"Slave_running",            (char*) &show_slave_running,     SHOW_SIMPLE_FUNC},
>>    {"Slave_skipped_errors",     (char*) &slave_skipped_errors, SHOW_LONGLONG},
>> +  {"Slave_DDL_statements",     (char*) &show_slave_ddl_stmts, SHOW_SIMPLE_FUNC},
>> +  {"Slave_Non_transactional_statements", (char *) &show_slave_non_trans_stmts, SHOW_SIMPLE_FUNC},
>
> Again, this is not statements, it is event groups. Or "transactions" if you
> like, which is a term more familiar to users, though a "non-transactional
> transaction" is also not perfect terminology.
>
Used Event.
>  - Kristian.



-- 
Regards
sachin

commit d079dbb04cab34e63d4cef140735e543492178db
Author: Sachin Setiya <sachinsetia1001@xxxxxxxxx>
Date:   Sun Jan 8 19:35:20 2017 +0530

    Mdev-10664 Add statuses about optimistic parallel replication stalls
    
    In this commit we are adding replication channel specific status variables
    Slave_DDL_Events, Slave_Non_Transactional_Events.
    
    Slave_DDL_Events:- No of DDL transactions executed by slave in replication
    channel X.
    Slave_Non_Transactional_Events:- No of non-trans events group executed by
    slave in replication channel X.
    
    Patch Credit:- Kristian Nielsen

diff --git a/mysql-test/suite/multi_source/multi_parallel.result b/mysql-test/suite/multi_source/multi_parallel.result
new file mode 100644
index 0000000..5cbbaaf
--- /dev/null
+++ b/mysql-test/suite/multi_source/multi_parallel.result
@@ -0,0 +1,130 @@
+set global slave_parallel_threads=10;
+change master 'master1' to
+master_port=MYPORT_1,
+master_host='127.0.0.1',
+master_user='root';
+change master 'master2' to
+master_port=MYPORT_2,
+master_host='127.0.0.1',
+master_user='root';
+start all slaves;
+Warnings:
+Note	1937	SLAVE 'master2' started
+Note	1937	SLAVE 'master1' started
+set default_master_connection = 'master1';
+include/wait_for_slave_to_start.inc
+set default_master_connection = 'master2';
+include/wait_for_slave_to_start.inc
+## Slave status variable
+set default_master_connection = 'master1';
+show status like 'slave_running';
+Variable_name	Value
+Slave_running	ON
+set default_master_connection = 'master2';
+show status like 'slave_running';
+Variable_name	Value
+Slave_running	ON
+#master 1
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+create table t1(a int primary key);
+insert into t1 values(1);
+insert into t1 values(2);
+drop table t1;
+set default_master_connection = 'master1';
+show status like 'Slave_DDL_Events';
+Variable_name	Value
+Slave_ddl_events	20
+show status like 'Slave_Non_Transactional_Events';
+Variable_name	Value
+Slave_non_transactional_events	40
+#master 2
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+create table t2(a int primary key);
+insert into t2 values(1);
+insert into t2 values(2);
+drop table t2;
+set default_master_connection = 'master2';
+show status like 'Slave_DDL_Events';
+Variable_name	Value
+Slave_ddl_events	18
+show status like 'Slave_Non_Transactional_Events';
+Variable_name	Value
+Slave_non_transactional_events	36
+stop all slaves;
+Warnings:
+Note	1938	SLAVE 'master2' stopped
+Note	1938	SLAVE 'master1' stopped
+set default_master_connection = 'master1';
+include/wait_for_slave_to_stop.inc
+set default_master_connection = 'master2';
+include/wait_for_slave_to_stop.inc
+set global slave_parallel_threads=0;
+include/reset_master_slave.inc
+include/reset_master_slave.inc
+include/reset_master_slave.inc
diff --git a/mysql-test/suite/multi_source/multi_parallel.test b/mysql-test/suite/multi_source/multi_parallel.test
new file mode 100644
index 0000000..b9570e5
--- /dev/null
+++ b/mysql-test/suite/multi_source/multi_parallel.test
@@ -0,0 +1,113 @@
+#multi source parallel replication
+
+--source include/not_embedded.inc
+--let $rpl_server_count= 0
+
+--connect (master1,127.0.0.1,root,,,$SERVER_MYPORT_1)
+--connect (master2,127.0.0.1,root,,,$SERVER_MYPORT_2)
+--connect (slave,127.0.0.1,root,,,$SERVER_MYPORT_3)
+
+#save state
+--let $par_thd= `select @@slave_parallel_threads;`
+
+set global slave_parallel_threads=10;
+
+--replace_result $SERVER_MYPORT_1 MYPORT_1
+eval change master 'master1' to
+master_port=$SERVER_MYPORT_1,
+master_host='127.0.0.1',
+master_user='root';
+
+--replace_result $SERVER_MYPORT_2 MYPORT_2
+eval change master 'master2' to
+master_port=$SERVER_MYPORT_2,
+master_host='127.0.0.1',
+master_user='root';
+
+
+#start all slaves
+
+start all slaves;
+
+set default_master_connection = 'master1';
+--source include/wait_for_slave_to_start.inc
+
+set default_master_connection = 'master2';
+--source include/wait_for_slave_to_start.inc
+
+--echo ## Slave status variable
+
+set default_master_connection = 'master1';
+show status like 'slave_running';
+
+set default_master_connection = 'master2';
+show status like 'slave_running';
+
+
+--echo #master 1
+--connection master1
+
+--let $counter=10
+while ($counter)
+{
+  #DDL statement
+  create table t1(a int primary key);
+
+  #non trans update statement
+  insert into t1 values(1);
+  insert into t1 values(2);
+
+  drop table t1;
+  --dec $counter
+}
+--save_master_pos
+
+--connection slave
+
+--sync_with_master 0,'master1'
+set default_master_connection = 'master1';
+show status like 'Slave_DDL_Events';
+show status like 'Slave_Non_Transactional_Events';
+
+--echo #master 2
+--connection master2
+
+--let $counter=9
+while ($counter)
+{
+  #DDL statement
+  create table t2(a int primary key);
+
+  #non trans update statement
+  insert into t2 values(1);
+  insert into t2 values(2);
+
+  drop table t2;
+  --dec $counter
+}
+--save_master_pos
+
+--connection slave
+--sync_with_master 0,'master2'
+set default_master_connection = 'master2';
+show status like 'Slave_DDL_Events';
+show status like 'Slave_Non_Transactional_Events';
+
+# Cleanup
+stop all slaves;
+set default_master_connection = 'master1';
+--source include/wait_for_slave_to_stop.inc
+
+set default_master_connection = 'master2';
+--source include/wait_for_slave_to_stop.inc
+
+--eval set global slave_parallel_threads=$par_thd
+
+--source include/reset_master_slave.inc
+--disconnect slave
+--connection master1
+--source include/reset_master_slave.inc
+--disconnect master1
+--connection master2
+--source include/reset_master_slave.inc
+--disconnect master2
diff --git a/sql/log_event.cc b/sql/log_event.cc
index ced2626..f0e4d43 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -6780,6 +6780,13 @@ Gtid_log_event::do_apply_event(rpl_group_info *rgi)
   }
 
   DBUG_ASSERT((bits & OPTION_GTID_BEGIN) == 0);
+
+  Master_info *mi=rgi->rli->mi;
+  if (flags2 & FL_DDL)
+    mi->total_ddl_events++;
+  if (!(flags & FL_TRANSACTIONAL))
+    mi->total_non_trans_events++;
+
   if (flags2 & FL_STANDALONE)
     return 0;
 
diff --git a/sql/mysqld.cc b/sql/mysqld.cc
index adb1b27..e435a89 100644
--- a/sql/mysqld.cc
+++ b/sql/mysqld.cc
@@ -7805,7 +7805,59 @@ static int show_heartbeat_period(THD *thd, SHOW_VAR *var, char *buff,
   return 0;
 }
 
+static long show_slave_ddl_events(THD *thd, SHOW_VAR *var, char *buff,
+                                 enum enum_var_type scope)
+{
+  Master_info *mi= NULL;
+  longlong UNINIT_VAR(tmp);
+
+  var->type= SHOW_LONGLONG;
+  var->value= buff;
+
+  mysql_mutex_lock(&LOCK_active_mi);
+  if (master_info_index)
+  {
+    mi= master_info_index->
+      get_master_info(&thd->variables.default_master_connection,
+                    Sql_condition::WARN_LEVEL_NOTE);
+    if (mi)
+      tmp= mi->total_ddl_events;
+  }
+  mysql_mutex_unlock(&LOCK_active_mi);
+  if (mi)
+    *((longlong *)buff)= tmp;
+  else
+    var->type= SHOW_UNDEF;
+
+  return 0;
+}
 
+static long show_slave_non_trans_events(THD *thd, SHOW_VAR *var, char *buff,
+                                 enum enum_var_type scope)
+{
+  Master_info *mi= NULL;
+  longlong UNINIT_VAR(tmp);
+
+  var->type= SHOW_LONGLONG;
+  var->value= buff;
+
+  mysql_mutex_lock(&LOCK_active_mi);
+
+  {
+    mi= master_info_index->
+      get_master_info(&thd->variables.default_master_connection,
+                    Sql_condition::WARN_LEVEL_NOTE);
+    if (mi)
+      tmp= mi->total_non_trans_events;
+  }
+  mysql_mutex_unlock(&LOCK_active_mi);
+  if (mi)
+    *((longlong *)buff)= tmp;
+  else
+    var->type= SHOW_UNDEF;
+
+  return 0;
+}
 #endif /* HAVE_REPLICATION */
 
 static int show_open_tables(THD *thd, SHOW_VAR *var, char *buff,
@@ -8473,6 +8525,8 @@ SHOW_VAR status_vars[]= {
   {"Slave_retried_transactions",(char*)&slave_retried_transactions, SHOW_LONG},
   {"Slave_running",            (char*) &show_slave_running,     SHOW_SIMPLE_FUNC},
   {"Slave_skipped_errors",     (char*) &slave_skipped_errors, SHOW_LONGLONG},
+  {"Slave_DDL_Events",     (char*) &show_slave_ddl_events, SHOW_SIMPLE_FUNC},
+  {"Slave_Non_Transactional_Events", (char *) &show_slave_non_trans_events, SHOW_SIMPLE_FUNC},
 #endif
   {"Slow_launch_threads",      (char*) &slow_launch_threads,    SHOW_LONG},
   {"Slow_queries",             (char*) offsetof(STATUS_VAR, long_query_count), SHOW_LONG_STATUS},
diff --git a/sql/rpl_mi.cc b/sql/rpl_mi.cc
index 6048d26..85a5e66 100644
--- a/sql/rpl_mi.cc
+++ b/sql/rpl_mi.cc
@@ -40,7 +40,8 @@ Master_info::Master_info(LEX_STRING *connection_name_arg,
    sync_counter(0), heartbeat_period(0), received_heartbeats(0),
    master_id(0), prev_master_id(0),
    using_gtid(USE_GTID_NO), events_queued_since_last_gtid(0),
-   gtid_reconnect_event_skip_count(0), gtid_event_seen(false)
+   gtid_reconnect_event_skip_count(0), gtid_event_seen(false),
+   total_ddl_events(0), total_non_trans_events(0)
 {
   host[0] = 0; user[0] = 0; password[0] = 0;
   ssl_ca[0]= 0; ssl_capath[0]= 0; ssl_cert[0]= 0;
diff --git a/sql/rpl_mi.h b/sql/rpl_mi.h
index 9365c06..a474aa5 100644
--- a/sql/rpl_mi.h
+++ b/sql/rpl_mi.h
@@ -303,6 +303,12 @@ class Master_info : public Slave_reporting_capability
 
   /* The parallel replication mode. */
   enum_slave_parallel_mode parallel_mode;
+
+  /* No of DDL statements */
+  long total_ddl_events;
+
+  /* No of non-transactional statements*/
+  long total_non_trans_events;
 };
 
 int init_master_info(Master_info* mi, const char* master_info_fname,