Re: [Maria-developers] [Commits] ef590fa: MDEV-7409 On RBR, extend the PROCESSLIST info to include at least the name of the recently used table
On 24/01/17 04:30, SachinSetiya wrote:
revision-id: ef590faa0e21d25a2dc5153f938135612b17ecbc (mariadb-10.1.20-31-gef590fa)
thd_proc_info(thd, message); + DBUG_EXECUTE_IF("should_sleep_for_mdev7409",{ + my_sleep(500000); + };);
Shouldn't these be DEBUG_SYNC_C("do_exec_row"); and then use debug_sync to generate test synchronisation.
int error= write_row(rgi, slave_exec_mode == SLAVE_EXEC_MODE_IDEMPOTENT); thd_proc_info(thd, tmp); + thd->db= tmp_db;
if (error && !thd->is_error()) { @@ -12371,32 +12380,45 @@ int Delete_rows_log_event::do_exec_row(rpl_group_info *rgi) { int error; const char *tmp= thd->get_proc_info(); - const char *message= "Delete_rows_log_event::find_row()"; + char *tmp_db= thd->db; + char *message, msg[128]; + my_snprintf(msg, sizeof(msg),"Delete_rows_log_event::find_row() on table %s", + m_table->s->table_name.str); + thd->db= m_table->s->db.str; + message= msg;
Why assign a pointer? Isn't defining message as a char[128] sufficient? Thanks for writing this feature.
Hi Daniel! On Tue, Jan 24, 2017 at 2:32 AM, Daniel Black <daniel.black@au1.ibm.com> wrote:
On 24/01/17 04:30, SachinSetiya wrote:
revision-id: ef590faa0e21d25a2dc5153f938135612b17ecbc (mariadb-10.1.20-31-gef590fa)
thd_proc_info(thd, message); + DBUG_EXECUTE_IF("should_sleep_for_mdev7409",{ + my_sleep(500000); + };);
Shouldn't these be DEBUG_SYNC_C("do_exec_row");
and then use debug_sync to generate test synchronisation.
I tried using DEBUG_SYNC and DEBUG_SYNC_C
int Write_rows_log_event::do_exec_row(rpl_group_info *rgi) { DBUG_ASSERT(m_table != NULL); const char *tmp= thd->get_proc_info(); char *tmp_db= thd->db; char *message, msg[128]; my_snprintf(msg, sizeof(msg),"Write_rows_log_event::write_row() on table %s", m_table->s->table_name.str); thd->db= m_table->s->db.str; message= msg; #ifdef WSREP_PROC_INFO my_snprintf(thd->wsrep_info, sizeof(thd->wsrep_info) - 1, "Write_rows_log_event::write_row(%lld) on table %s", (long long) wsrep_thd_trx_seqno(thd), m_table->s->table_name.str); message= thd->wsrep_info; #endif /* WSREP_PROC_INFO */ thd_proc_info(thd, message); DEBUG_SYNC_C("write_row_log_event__do_exec_row"); and in mtr test --connection slave SET DEBUG_SYNC= 'write_row_log_event__do_exec_row SIGNAL fdfdffd WAIT_FOR fsdfsdfsdf'; But mtr test is not waiting, this is because ::do_exec_row is called by SQL thread is there a way to stop SQL thread and add sync points ?
int error= write_row(rgi, slave_exec_mode == SLAVE_EXEC_MODE_IDEMPOTENT); thd_proc_info(thd, tmp); + thd->db= tmp_db;
if (error && !thd->is_error()) { @@ -12371,32 +12380,45 @@ int Delete_rows_log_event::do_exec_row(rpl_group_info *rgi) { int error; const char *tmp= thd->get_proc_info(); - const char *message= "Delete_rows_log_event::find_row()"; + char *tmp_db= thd->db; + char *message, msg[128]; + my_snprintf(msg, sizeof(msg),"Delete_rows_log_event::find_row() on table %s", + m_table->s->table_name.str); + thd->db= m_table->s->db.str; + message= msg;
Why assign a pointer? Isn't defining message as a char[128] sufficient?
Because, I do not have any better option char *message, msg[128]; my_snprintf(msg, sizeof(msg),"Write_rows_log_event::write_row() on table %s", m_table->s->table_name.str); thd->db= m_table->s->db.str; message= msg; #ifdef WSREP_PROC_INFO my_snprintf(thd->wsrep_info, sizeof(thd->wsrep_info) - 1, "Write_rows_log_event::write_row(%lld) on table %s", (long long) wsrep_thd_trx_seqno(thd), m_table->s->table_name.str); message= thd->wsrep_info; #endif /* WSREP_PROC_INFO */ thd_proc_info(thd, message); If i take message as array then I have to memcpy thd->wsrep_info into message. Because I do not want to call thd_proc_info 2 times. that is why I defined a pointer and a array.
Thanks for writing this feature.
-- Regards Sachin Setiya
Sachin Setiya <sachin.setiya@mariadb.com> writes:
On Tue, Jan 24, 2017 at 2:32 AM, Daniel Black <daniel.black@au1.ibm.com> wrote:
On 24/01/17 04:30, SachinSetiya wrote:
revision-id: ef590faa0e21d25a2dc5153f938135612b17ecbc (mariadb-10.1.20-31-gef590fa)
thd_proc_info(thd, message); + DBUG_EXECUTE_IF("should_sleep_for_mdev7409",{ + my_sleep(500000); + };);
Shouldn't these be DEBUG_SYNC_C("do_exec_row");
and then use debug_sync to generate test synchronisation.
I tried using DEBUG_SYNC and DEBUG_SYNC_C
But mtr test is not waiting, this is because ::do_exec_row is called by SQL thread is there a way to stop SQL thread and add sync points ?
You use DBUG_EXECUTE_IF() to trigger code inside the SQL thread that then does the appropriate debug_sync command. For an example, see the use of rpl_parallel_scheduled_gtid_0_x_100 in mysql-test/extra/rpl_tests/rpl_parallel.inc and sql/rpl_parallel.cc. Do not ever use sleeps as a way to synchronise in test cases (or anywhere).
+ my_snprintf(msg, sizeof(msg),"Write_rows_log_event::write_row() on table %s", + m_table->s->table_name.str);
Since you are anyway changing the message, I would put something more appropriate here. Putting internal server source code names like Write_rows_log_event::write_row() in SHOW PROCESSLIST does not seem appropriate. - Kristian.
Hi Kristian, Daniel, On Thu, Jan 26, 2017 at 2:24 PM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Sachin Setiya <sachin.setiya@mariadb.com> writes:
On Tue, Jan 24, 2017 at 2:32 AM, Daniel Black <daniel.black@au1.ibm.com> wrote:
On 24/01/17 04:30, SachinSetiya wrote:
revision-id: ef590faa0e21d25a2dc5153f938135612b17ecbc (mariadb-10.1.20-31-gef590fa)
thd_proc_info(thd, message); + DBUG_EXECUTE_IF("should_sleep_for_mdev7409",{ + my_sleep(500000); + };);
Shouldn't these be DEBUG_SYNC_C("do_exec_row");
and then use debug_sync to generate test synchronisation.
I tried using DEBUG_SYNC and DEBUG_SYNC_C
But mtr test is not waiting, this is because ::do_exec_row is called by SQL thread is there a way to stop SQL thread and add sync points ?
You use DBUG_EXECUTE_IF() to trigger code inside the SQL thread that then does the appropriate debug_sync command. For an example, see the use of rpl_parallel_scheduled_gtid_0_x_100 in mysql-test/extra/rpl_tests/rpl_parallel.inc and sql/rpl_parallel.cc.
thanks, used debug_sync_set_action, But one more problem I used debug_sync_set_action, but this changes thd->info to debug_sync point now. So Instead of printing thd->info I am just printing from echo and using condition in dbug_execute_if + DBUG_EXECUTE_IF("should_sleep_for_mdev7409",{ + if(!my_strcasecmp(system_charset_info, "test", thd->db) && + !my_strcasecmp(system_charset_info,"t1", table_name)) + debug_sync_set_action(thd, + STRING_WITH_LEN("now SIGNAL thd_info_set WAIT_FOR done")); + };); +connection slave; +SET DEBUG_SYNC='now WAIT_FOR thd_info_set'; +--echo #Write_row:- Writing row on table t1 +SET DEBUG_SYNC = 'now SIGNAL done';
Do not ever use sleeps as a way to synchronise in test cases (or anywhere).
+ my_snprintf(msg, sizeof(msg),"Write_rows_log_event::write_row() on table %s", + m_table->s->table_name.str);
Since you are anyway changing the message, I would put something more appropriate here. Putting internal server source code names like Write_rows_log_event::write_row() in SHOW PROCESSLIST does not seem appropriate.
Done.
- Kristian.
Patch link:- http://lists.askmonty.org/pipermail/commits/2017-January/010583.html Buildbot link:- https://buildbot.askmonty.org/buildbot/grid?category=main&branch=bb-10.1-sachin Regards sachin
participants (3)
-
Daniel Black -
Kristian Nielsen -
Sachin Setiya