Fix SRO + multi-table update statements in replication

[slyglif]

Bug fix for when a multi-table update statement is binlogged in statement form and the slave is in super read only.

This also pulls the code to check for RO into an inline function and replaces other occurrences to reduce the likelihood of this happening again.

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

[slyglif]

I also have a clean patch against webscale (since this won't apply cleanly to that because of some differences in FB's 5.6 branch and the webscale branch.

[slyglif]

Hello Santosh,

The error is related to running a slave in super-read-only with statement or mixed replication, then executing a multi-table update statement on the master. Specifically, if you have SRO set on the slave, then execute the following on the master, replication will break on the update statement:

create table t1(a int);
create table t2(a int, b int);
insert into t1 values(1), (2);
insert into t2 (a) values (1), (2), (3), (4);
update t2 natural join t1 set b=3;

The message when it breaks is:

Error 'The MySQL server is running with the --read-only (super) option so it cannot execute this statement' on query. Default database: 'test'. Query: 'update t2 natural join t1 set b=3'

The problem is in sql_parse.cc lines 3834-3836, in that it isn't considering the value of thd->slave_thread (which is considered on lock.cc:201, handler.cc:1479, and sql_trigger.cc:464). I included the change to transaction.cc because it used similar logic and was also missing the check for slave_thread, though I'm not sure if/what error that one would generate.

The reason for the refactoring was to prevent any future issues with this, since everything could just start checking the function instead.

[santoshbanda]

Hello Chris,

Thanks for the explanation. Could you update the description including the details in your previous comment. Someone going through the change should understand that you have by passed super_read_only check for replication thread at couple of places i,e; sql_parse.cc and sql_transaction.cc

[santoshbanda]

Hello Chirs,

We require you sign the Contributor License Agreement in here https://code.facebook.com/cla.

[slyglif]

Santosh, I can't seem to figure out how to update / add a description other than the title? Also, I accepted the CLA on Tuesday.

[steaphan-fb-com]

Santosh will be pulling your commit in directly from your repo, so the description he's talking about is actually the git commit message. But, if you could just post a concise description which can be used in the commit, as a comment here instead, that would be fine too.

We will put this patch through our own internal code review, and, assuming there are no problems, after it is approved through that, it will be included in this public branch.

Thanks for the contribution! Sorry for the confusion about the CLA - something was apparently wrong with one of our bots.

[slyglif]

This is a bug fix for super-read-only not being ignored for the replication thread on multi-table update statements in sql_parse.cc that would cause replication the stop with the error "Error 'The MySQL server is running with the --read-only (super) option so it cannot execute this statement' on query. Default database: 'test'. Query: 'update t2 natural join t1 set b=3'". The same issue was found in transaction.cc.

To help avoid this issue in the future the logic for testing if read-only should be enforced is refactored into a new inline function, and existing code that performed the logic in each location was changed to instead call this function.

[santoshbanda]

This is included in our public release now. Here is the link fd5b0ca

Thanks for the contribution !!!