From b347396181018cedc946450cb49891f1a0aa4575 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Tue, 28 May 2019 14:20:39 +0530 Subject: MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled Problem: ======= rpl_blackhole.test fails when executed with following options mysqld=--binlog_annotate_row_events=1, mysqld=--replicate_annotate_row_events=1 Test output: ------------ worker[1] Using MTR_BUILD_THREAD 300, with reserved ports 16000..16019 rpl.rpl_blackhole_bug 'mix' [ pass ] 791 rpl.rpl_blackhole_bug 'row' [ fail ] Replicate_Wild_Ignore_Table Last_Errno 1032 Last_Error Could not execute Update_rows_v1 event on table test.t1; Can't find record in 't1', Error_code: 1032; handler error HA_ERR_END_OF_FILE; the event's master log master-bin.000001, end_log_pos 1510 Analysis: ========= Enabling "replicate_annotate_row_events" on slave, Tells the slave to write annotate rows events received from the master to its own binary log. The received annotate events are applied after the Gtid event as shown below. thd->query() will be set to the actual query received from the master, through annotate event. Annotate_rows event should not be deleted after the event is applied as the thd->query will be used to generate new Annotate_rows event during applying the subsequent Rows events. After the last Rows event has been applied, the saved Annotate_rows event (if any) will be deleted. In balckhole engine all the DML operations are noops as they donot store any data. They simply return success without doing any operation. But the existing strictly expects thd->query() to be 'NULL' to identify that row based replication is in use. This assumption will fail when row annotations are enabled as the query is not 'NULL'. Hence various row based operations like 'update', 'delete', 'index lookup' will fail when row annotations are enabled. Fix: === Extend the row based replication check to include row annotations as well. i.e Either the thd->query() is NULL or thd->query() points to query and row annotations are in use. --- storage/blackhole/ha_blackhole.cc | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) (limited to 'storage/blackhole') diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc index 01aaa9ea15f..43bcdc541a1 100644 --- a/storage/blackhole/ha_blackhole.cc +++ b/storage/blackhole/ha_blackhole.cc @@ -25,6 +25,16 @@ #include "ha_blackhole.h" #include "sql_class.h" // THD, SYSTEM_THREAD_SLAVE_SQL +static bool is_row_based_replication(THD *thd) +{ + /* + A row event which has its thd->query() == NULL or a row event which has + replicate_annotate_row_events enabled. In the later case the thd->query() + will be pointing to the query, received through replicated annotate event + from master. + */ + return ((thd->query() == NULL) || thd->variables.binlog_annotate_row_events); +} /* Static declarations for handlerton */ static handler *blackhole_create_handler(handlerton *hton, @@ -109,7 +119,8 @@ int ha_blackhole::update_row(const uchar *old_data, uchar *new_data) { DBUG_ENTER("ha_blackhole::update_row"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && + is_row_based_replication(thd)) DBUG_RETURN(0); DBUG_RETURN(HA_ERR_WRONG_COMMAND); } @@ -118,7 +129,8 @@ int ha_blackhole::delete_row(const uchar *buf) { DBUG_ENTER("ha_blackhole::delete_row"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && + is_row_based_replication(thd)) DBUG_RETURN(0); DBUG_RETURN(HA_ERR_WRONG_COMMAND); } @@ -135,7 +147,8 @@ int ha_blackhole::rnd_next(uchar *buf) int rc; DBUG_ENTER("ha_blackhole::rnd_next"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && + is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; @@ -220,7 +233,8 @@ int ha_blackhole::index_read_map(uchar * buf, const uchar * key, int rc; DBUG_ENTER("ha_blackhole::index_read"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && + is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; @@ -235,7 +249,8 @@ int ha_blackhole::index_read_idx_map(uchar * buf, uint idx, const uchar * key, int rc; DBUG_ENTER("ha_blackhole::index_read_idx"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && + is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; @@ -249,7 +264,8 @@ int ha_blackhole::index_read_last_map(uchar * buf, const uchar * key, int rc; DBUG_ENTER("ha_blackhole::index_read_last"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && + is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; -- cgit v1.2.1 From a47464d1c12d773364e78f50090b08484fe76129 Mon Sep 17 00:00:00 2001 From: Sujatha Date: Wed, 29 May 2019 17:35:29 +0530 Subject: MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled Post push fix. Simplified the earlier fixes. --- storage/blackhole/ha_blackhole.cc | 40 ++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) (limited to 'storage/blackhole') diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc index 43bcdc541a1..69182676c1e 100644 --- a/storage/blackhole/ha_blackhole.cc +++ b/storage/blackhole/ha_blackhole.cc @@ -25,15 +25,23 @@ #include "ha_blackhole.h" #include "sql_class.h" // THD, SYSTEM_THREAD_SLAVE_SQL +/** + Checks if the param 'thd' is pointing to slave applier thread and row based + replication is in use. + + A row event will have its thd->query() == NULL except in cases where + replicate_annotate_row_events is enabled. In the later case the thd->query() + will be pointing to the query, received through replicated annotate event + from master. + + @param thd pointer to a THD instance + + @return TRUE if thread is slave applier and row based replication is in use +*/ static bool is_row_based_replication(THD *thd) { - /* - A row event which has its thd->query() == NULL or a row event which has - replicate_annotate_row_events enabled. In the later case the thd->query() - will be pointing to the query, received through replicated annotate event - from master. - */ - return ((thd->query() == NULL) || thd->variables.binlog_annotate_row_events); + return thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && + (thd->query() == NULL || thd->variables.binlog_annotate_row_events); } /* Static declarations for handlerton */ @@ -119,8 +127,7 @@ int ha_blackhole::update_row(const uchar *old_data, uchar *new_data) { DBUG_ENTER("ha_blackhole::update_row"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && - is_row_based_replication(thd)) + if (is_row_based_replication(thd)) DBUG_RETURN(0); DBUG_RETURN(HA_ERR_WRONG_COMMAND); } @@ -129,8 +136,7 @@ int ha_blackhole::delete_row(const uchar *buf) { DBUG_ENTER("ha_blackhole::delete_row"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && - is_row_based_replication(thd)) + if (is_row_based_replication(thd)) DBUG_RETURN(0); DBUG_RETURN(HA_ERR_WRONG_COMMAND); } @@ -147,8 +153,7 @@ int ha_blackhole::rnd_next(uchar *buf) int rc; DBUG_ENTER("ha_blackhole::rnd_next"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && - is_row_based_replication(thd)) + if (is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; @@ -233,8 +238,7 @@ int ha_blackhole::index_read_map(uchar * buf, const uchar * key, int rc; DBUG_ENTER("ha_blackhole::index_read"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && - is_row_based_replication(thd)) + if (is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; @@ -249,8 +253,7 @@ int ha_blackhole::index_read_idx_map(uchar * buf, uint idx, const uchar * key, int rc; DBUG_ENTER("ha_blackhole::index_read_idx"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && - is_row_based_replication(thd)) + if (is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; @@ -264,8 +267,7 @@ int ha_blackhole::index_read_last_map(uchar * buf, const uchar * key, int rc; DBUG_ENTER("ha_blackhole::index_read_last"); THD *thd= ha_thd(); - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && - is_row_based_replication(thd)) + if (is_row_based_replication(thd)) rc= 0; else rc= HA_ERR_END_OF_FILE; -- cgit v1.2.1