Changed FTWRL to produce error

[morgo]

What problem does this PR solve?

Many programs use FLUSH TABLES WITH READ LOCK (ftwrl) to create a synchronization point between multi-threaded programs. For example, mydumper works like this:

Main thread: FLUSH TABLES WITH READ LOCK;
Worker threads: START TRANSACTION WITH CONSISTENT SNAPSHOT;
Main thread: UNLOCK TABLES;

Because FTWRL is parsed but ignored, these programs are not getting a synchronization point, leading to inconsistent backups. The alternatives for these programs are:

• Use single threaded execution (start transaction is sufficient to provide repeatable-read consistency)
• Use a tidb_snapshot read. Our fork of mydumper can do this (although it doesn't by default). I have a PR for mydumper upstream.

What is changed and how it works?

This changes FTWRL to be an error, similar to KILL x (not supported, but judged not safe to parse but ignore). An alternative implimentation could be to raise a warning, but error is my first preference.

Check List

Tests

• Unit test

Code changes

• Has exported function/method change

Side effects

• Breaking backward compatibility

Related changes

• Need to update the documentation
• Need to be included in the release note

Important! This change is not backwards compatible. It will cause an error in existing mydumpers (both upstream and our fork). However, mydumper also checks for warnings: so lessening the severity will not help.

Assuming the mydumper PR is pulled, we should document that TiDB requires a version of mydumper of X.Y.Z or greater.

[morgo]

/run-all-tests

[shenli]

@morgo This change may lead to a compatible issue. We need to consider this carefully.

[morgo]

/run-all-tests tidb-test=pr/626

[coocood]

@morgo
Do we need to return the error if WITH READ LOCK is not specified?

[morgo]

@coocood No. You are correct, it is safe for FLUSH TABLES (no additions) to be a no-op.

[shenli]

@morgo Should we merge this PR after the PR for mydumper upstream merged?

[shenli]

LGTM

[morgo]

@shenli Yes, that would be my preference. The release notes can say minimum mydumper version required.

[zhexuany]

It seems that this PR is blocked by mydumper/mydumper#155
but LGTM

[zhexuany]

LGTM

[winoros]

We need to wait for that pr merged?

[morgo]

@winoros I have a backup plan, which is for these two PRs to merge:
pingcap/mydumper#4
pingcap/docs#644

[morgo]

This PR is now unblocked and Ready for Merge.

[winkyao]

LGTM