Hi Sven,
On 06/17/2011 12:33 PM, Sven Sandberg wrote:
> Hi Alfranio,
>
> Thank you for this work! I think it is great that we clarify how
> replication tables work and that we make them work like other tables.
> You have analyzed and identified the correct things that need to be
> fixed. As we discussed on IRC, we need to refine the check that analyzes
> which statements should be allowed to execute concurrently with slave
> threads. Also, two optimizations may be possible. And I think we need a
> test case and I agree with all of Luis comments. So I will reject the patch.
>
> 1. The check for which commands are allowed needs to be refined:
> - A SELECT can update the table (through a SF), so we have to check
> what tables SELECT accesses, so please remove
> CF_ACCESS_RPL_INFO_COMMAND from SQLCOM_SELECT.
>
> - We have to distinguish reads and writes, because we want SELECT
> to be able to read from the table but not write to it. It is not
> known at parse which tables are read and which are written.
> So we have to move the check to later, probably around
> lock_tables or decide_logging_format.
I took another approach that does not require to identify what commands are allowed or
not.
Basically, at the lock.cc, I have implemented the following rule: Tt is always possible to
read the replication tables at any time and to write when replication is not running.
The exception is LOCK READ/FLUSH LOCK READ what requires to stop replication.
>
> 2. Two optimizations may be possible:
> - Please, check if it is possible to optimize the check by
> comparing the table_share pointers instead of the table names and
> database names (not sure if this is possible).
The lock.cc already identifies the type of a table. So I don't need to do anything else.
if (t->s->table_category == TABLE_CATEGORY_RPL_INFO)
do something;
>
> - Please, optimize the check for commands that could not access the
> tables such as CREATE USER etc, by setting
> CF_ACCESS_RPL_INFO_COMMAND for them. But please note that many
> commands that look like they would not touch any tables can
> actually call stored functions that could read or write tables:
> for instance, SHOW VARIABLES LIKE stored_function(),
> SET @foo= stored_function(), etc.
See previous comment.
>
> 3. Please write a test case that verifies that:
>
> - For each of the commands
> SELECT, DO, SHOW, SET, INSERT, INSERT...SELECT REPLACE,
> REPLACE...SELECT, UPDATE, DELETE, and CREATE...SELECT,
> if the command invokes a stored function:
> - An error is generated if the stored function updates a
> replication table when a slave thread is running.
> - No error is generated if the stored function reads a
> replication table when a slave thread is running.
> - No error is generated if the stored function writes a
> replication table when no slave thread is running.
ok.
>
> - For each of the commands
> CREATE, CREATE...SELECT, INSERT, INSERT...SELECT, REPLACE,
> REPLACE...SELECT, UPDATE, multi-table UPDATE, DELETE,
> ALTER TABLE, DROP TABLE, TRUNCATE, BINLOG, OPTIMIZE,
> LOAD DATA INFILE:
> An error is generated if the target table is a replication table
> and a slave thread is running.
>
> - SHOW CREATE TABLE does not generate an error when the target
> table is a replication table and a slave threads is running.
>
> - CREATE TEMPORARY TABLE mysql.rpl_info_table does not crash and
> does not allow users to access tables that they don't have
> permissions for.
ok.
>
> /Sven
Cheers.
