Commit bb32ac1d authored by Venkatesh Duggirala's avatar Venkatesh Duggirala

BUG#17018343 SLAVE CRASHES WHEN APPLYING ROW-BASED BINLOG ENTRIES IN CASCADING

REPLICATION

Problem: In RBR mode, merge table updates are not successfully applied on a cascading
replication.

Analysis & Fix: Every type of row event is preceded by one or more table_map_log_events
that gives the information about all the tables that are involved in the row
event. Server maintains the list in RPL_TABLE_LIST and it goes through all the
tables and checks for the compatibility between master and slave. Before
checking for the compatibility, it calls 'open_tables()' which takes the list
of all tables that needs to be locked and opened. In RBR, because of the
Table_map_log_event , we already have all the tables including base tables in
the list. But the open_tables() which is generic call takes care of appending
base tables if the list contains merge tables. There is an assumption in the
current replication layer logic that these tables (TABLE_LIST type objects) are always
added in the end of the list. Replication layer maintains the count of
tables(tables_to_lock_count) that needs to be verified for compatibility check
and runs through only those many tables from the list and rest of the objects
in linked list can be skipped. But this assumption is wrong.
open_tables()->..->add_children_to_list() adds base tables to the list immediately
after seeing the merge table in the list.

For eg: If the list passed to open_tables() is t1->t2->t3 where t3 is merge
table (and t1 and t2 are base tables), it adds t1'->t2' to the list after t3.
New table list looks like t1->t2->t3->t1'->t2'. It looks like it added at the
end of the list but that is not correct. If the list passed to open_tables()
is t3->t1->t2 where t3 is merge table (and t1 and t2 are base tables), the new
prepared list will be t3->t1'->t2'->t1->t2. Where t1' and t2' are of
TABLE_LIST objects which were added by add_children_to_list() call and replication
layer should not look into them. Here tables_to_lock_count  will not help as the
objects are added in between the list.

Fix: After investigating add_children_list() logic (which is called from open_tables()),
there is no flag/logic in it to skip adding the children to the list even if the
children are already included in the table list. Hence to fix the issue, a
logic should be added in the replication layer to skip children in the list by
checking whether  'parent_l' is non-null or not. If it is children, we will skip 'compatibility'
check for that table.

Also this patch is not removing 'tables_to_lock_count' logic for the performance issues
if there are any children at the end of the list, those can be easily skipped directly by
stopping the loop with tables_to_lock_count check.
parent c7e68606
/* /*
Copyright (c) 2000, 2015, Oracle and/or its affiliates. All rights reserved. Copyright (c) 2000, 2016, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by it under the terms of the GNU General Public License as published by
...@@ -7930,9 +7930,6 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) ...@@ -7930,9 +7930,6 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli)
/* /*
When the open and locking succeeded, we check all tables to When the open and locking succeeded, we check all tables to
ensure that they still have the correct type. ensure that they still have the correct type.
We can use a down cast here since we know that every table added
to the tables_to_lock is a RPL_TABLE_LIST.
*/ */
{ {
...@@ -7951,10 +7948,37 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) ...@@ -7951,10 +7948,37 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli)
NOTE: The base tables are added here are removed when NOTE: The base tables are added here are removed when
close_thread_tables is called. close_thread_tables is called.
*/ */
RPL_TABLE_LIST *ptr= rli->tables_to_lock; TABLE_LIST *table_list_ptr= rli->tables_to_lock;
for (uint i= 0 ; ptr && (i < rli->tables_to_lock_count); for (uint i=0 ; table_list_ptr && (i < rli->tables_to_lock_count);
ptr= static_cast<RPL_TABLE_LIST*>(ptr->next_global), i++) table_list_ptr= table_list_ptr->next_global, i++)
{ {
/*
Below if condition takes care of skipping base tables that
make up the MERGE table (which are added by open_tables()
call). They are added next to the merge table in the list.
For eg: If RPL_TABLE_LIST is t3->t1->t2 (where t1 and t2
are base tables for merge table 't3'), open_tables will modify
the list by adding t1 and t2 again immediately after t3 in the
list (*not at the end of the list*). New table_to_lock list will
look like t3->t1'->t2'->t1->t2 (where t1' and t2' are TABLE_LIST
objects added by open_tables() call). There is no flag(or logic) in
open_tables() that can skip adding these base tables to the list.
So the logic here should take care of skipping them.
tables_to_lock_count logic will take care of skipping base tables
that are added at the end of the list.
For eg: If RPL_TABLE_LIST is t1->t2->t3, open_tables will modify
the list into t1->t2->t3->t1'->t2'. t1' and t2' will be skipped
because tables_to_lock_count logic in this for loop.
*/
if (table_list_ptr->parent_l)
continue;
/*
We can use a down cast here since we know that every table added
to the tables_to_lock is a RPL_TABLE_LIST (or child table which is
skipped above).
*/
RPL_TABLE_LIST *ptr= static_cast<RPL_TABLE_LIST*>(table_list_ptr);
DBUG_ASSERT(ptr->m_tabledef_valid); DBUG_ASSERT(ptr->m_tabledef_valid);
TABLE *conv_table; TABLE *conv_table;
if (!ptr->m_tabledef.compatible_with(thd, const_cast<Relay_log_info*>(rli), if (!ptr->m_tabledef.compatible_with(thd, const_cast<Relay_log_info*>(rli),
...@@ -7995,7 +8019,15 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) ...@@ -7995,7 +8019,15 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli)
*/ */
TABLE_LIST *ptr= rli->tables_to_lock; TABLE_LIST *ptr= rli->tables_to_lock;
for (uint i=0 ; ptr && (i < rli->tables_to_lock_count); ptr= ptr->next_global, i++) for (uint i=0 ; ptr && (i < rli->tables_to_lock_count); ptr= ptr->next_global, i++)
{
/*
Please see comment in above 'for' loop to know the reason
for this if condition
*/
if (ptr->parent_l)
continue;
const_cast<Relay_log_info*>(rli)->m_table_map.set_table(ptr->table_id, ptr->table); const_cast<Relay_log_info*>(rli)->m_table_map.set_table(ptr->table_id, ptr->table);
}
#ifdef HAVE_QUERY_CACHE #ifdef HAVE_QUERY_CACHE
query_cache.invalidate_locked_for_write(rli->tables_to_lock); query_cache.invalidate_locked_for_write(rli->tables_to_lock);
......
/* Copyright (c) 2007, 2014, Oracle and/or its affiliates. All rights reserved. /* Copyright (c) 2007, 2016, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by it under the terms of the GNU General Public License as published by
...@@ -119,16 +119,25 @@ Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, const Relay_log_info ...@@ -119,16 +119,25 @@ Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, const Relay_log_info
/* /*
When the open and locking succeeded, we check all tables to When the open and locking succeeded, we check all tables to
ensure that they still have the correct type. ensure that they still have the correct type.
We can use a down cast here since we know that every table added
to the tables_to_lock is a RPL_TABLE_LIST.
*/ */
{ {
RPL_TABLE_LIST *ptr= rli->tables_to_lock; TABLE_LIST *table_list_ptr= rli->tables_to_lock;
for (uint i= 0 ; ptr&& (i< rli->tables_to_lock_count); for (uint i=0 ; table_list_ptr&& (i< rli->tables_to_lock_count);
ptr= static_cast<RPL_TABLE_LIST*>(ptr->next_global), i++) table_list_ptr= table_list_ptr->next_global, i++)
{ {
/*
Please see comment in log_event.cc-Rows_log_event::do_apply_event()
function for the explanation of the below if condition
*/
if (table_list_ptr->parent_l)
continue;
/*
We can use a down cast here since we know that every table added
to the tables_to_lock is a RPL_TABLE_LIST(or child table which is
skipped above).
*/
RPL_TABLE_LIST *ptr=static_cast<RPL_TABLE_LIST*>(table_list_ptr);
DBUG_ASSERT(ptr->m_tabledef_valid); DBUG_ASSERT(ptr->m_tabledef_valid);
TABLE *conv_table; TABLE *conv_table;
if (!ptr->m_tabledef.compatible_with(thd, const_cast<Relay_log_info*>(rli), if (!ptr->m_tabledef.compatible_with(thd, const_cast<Relay_log_info*>(rli),
...@@ -162,7 +171,15 @@ Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, const Relay_log_info ...@@ -162,7 +171,15 @@ Old_rows_log_event::do_apply_event(Old_rows_log_event *ev, const Relay_log_info
*/ */
TABLE_LIST *ptr= rli->tables_to_lock; TABLE_LIST *ptr= rli->tables_to_lock;
for (uint i=0; ptr && (i < rli->tables_to_lock_count); ptr= ptr->next_global, i++) for (uint i=0; ptr && (i < rli->tables_to_lock_count); ptr= ptr->next_global, i++)
{
/*
Please see comment in log_event.cc-Rows_log_event::do_apply_event()
function for the explanation of the below if condition
*/
if (ptr->parent_l)
continue;
const_cast<Relay_log_info*>(rli)->m_table_map.set_table(ptr->table_id, ptr->table); const_cast<Relay_log_info*>(rli)->m_table_map.set_table(ptr->table_id, ptr->table);
}
#ifdef HAVE_QUERY_CACHE #ifdef HAVE_QUERY_CACHE
query_cache.invalidate_locked_for_write(rli->tables_to_lock); query_cache.invalidate_locked_for_write(rli->tables_to_lock);
#endif #endif
...@@ -1545,16 +1562,25 @@ int Old_rows_log_event::do_apply_event(Relay_log_info const *rli) ...@@ -1545,16 +1562,25 @@ int Old_rows_log_event::do_apply_event(Relay_log_info const *rli)
/* /*
When the open and locking succeeded, we check all tables to When the open and locking succeeded, we check all tables to
ensure that they still have the correct type. ensure that they still have the correct type.
We can use a down cast here since we know that every table added
to the tables_to_lock is a RPL_TABLE_LIST.
*/ */
{ {
RPL_TABLE_LIST *ptr= rli->tables_to_lock; TABLE_LIST *table_list_ptr= rli->tables_to_lock;
for (uint i= 0 ; ptr&& (i< rli->tables_to_lock_count); for (uint i=0; table_list_ptr&& (i< rli->tables_to_lock_count);
ptr= static_cast<RPL_TABLE_LIST*>(ptr->next_global), i++) table_list_ptr= static_cast<RPL_TABLE_LIST*>(table_list_ptr->next_global), i++)
{ {
/*
Please see comment in log_event.cc-Rows_log_event::do_apply_event()
function for the explanation of the below if condition
*/
if (table_list_ptr->parent_l)
continue;
/*
We can use a down cast here since we know that every table added
to the tables_to_lock is a RPL_TABLE_LIST (or child table which is
skipped above).
*/
RPL_TABLE_LIST *ptr=static_cast<RPL_TABLE_LIST*>(table_list_ptr);
TABLE *conv_table; TABLE *conv_table;
if (ptr->m_tabledef.compatible_with(thd, const_cast<Relay_log_info*>(rli), if (ptr->m_tabledef.compatible_with(thd, const_cast<Relay_log_info*>(rli),
ptr->table, &conv_table)) ptr->table, &conv_table))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment