MySQL Bugs: #94455: mysqlpump incorrectly documents add-locks and single-transaction mutually exclus

Bug #94455	mysqlpump incorrectly documents add-locks and single-transaction mutually exclus
Submitted:	23 Feb 2019 17:26	Modified:	28 Feb 2019 21:38
Reporter:	Seth Willits	Email Updates:
Status:	Not a Bug	Impact on me:	None
Category:	MySQL Server: mysqlpump Command-line Client	Severity:	S3 (Non-critical)
Version:	8.0.15	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	documentation, mysqlpump

Description:
mysqlpump's documentation says --add-locks and --single-transaction are mutually exclusive, but looking at the code, this is clearly not the case. I also see no reason why it would be. --add-locks simply affects the written file, not the actual export session.

This can be seen by looking for instances of m_add_locks and m_single_transaction, as well as in Program::check_mutually_exclusive_options().

How to repeat:
n/a

Hi,

Thank you for your bug report.

First of all, there is a check in the code on whether these two incompatible options are both included or not. You should get a warning about it. If you use the latest version .....

These two options are totally incompatible, as --single-transaction works with consistent snapshot and the locks that are added are added in the InnoDB session, not for the external file.

In essence, your report is not a bug.

I'm looking at the code for 8.0.15, and there is no exclusivity check that I can see anywhere, and in fact the code runs just fine, placing the LOCK statements in the dump.

The only thing add-locks does is write "LOCK TABLES `table` WRITE" and "UNLOCK TABLES" into the dump file. It does no execution *during* the dump, placing locks on the server like you say it does. So I see no way how that's "incompatible" with a consistent read transaction.

So, simply trying it, it works fine, and looking at the code, there's no reason why it shouldn't, that I can see.

mysqlpump --default-parallelism=0 --single-transaction --add-locks

Hi,

The reason why you do not get the error is because you have excluded parallelism. Parallelism is on of this program's main performance boosts, which you kill with locks.

Also, you can add locks, but then you loose all the advantages of the lock-free consistent snapshot, as otherwise, other transactions would not have to wait on the tables !!!!!

Not a bug.

This is not a bug in mysqldump; This is a *documentation* problem.

The documentation says "single-transaction" and "add-locks" are mutually exclusive — they are not.

Hi,

Can you point me to the exact chapter, subchapter and paragraph in our 8.0.15 manual ????

My mistake, I should have done so already:
https://dev.mysql.com/doc/refman/8.0/en/mysqlpump.html#option_mysqlpump_add-locks
https://dev.mysql.com/doc/refman/8.0/en/mysqlpump.html#option_mysqlpump_single-transaction

"--add-locks and --single-transaction are mutually exclusive."

It also says so in the man page as well.

Hi,

First of all, you should not apologise. It is not your mistake.

And also, our manual is correct. --single-transaction is there to enable InnoDB to function without any locks. So, without any table locks, all other transactions will run concurrently with mysqlpump. However, when you add --add-locks, you loose literally ALL benefits that are brought by consistent snapshot (enabled by --single-transaction). Hence, they are exclusive. One option is there to enable concurrency, while the other is there to prevent concurrency.

Of course, in such a case, tables are locked and concurrency is lost. That is why those two are mutually exclusive.

Our manual does not say that mysqlpump will not start or function due to loosing all the benefits of not locking the tables. Our manual does not say that mysqlpump will exit with an error.

So, do you think that our manual should state that those two options are exclusive, but that the only consequence would be a loss of concurrency, hence benefits from consistent snapshot would be lost. Should our manual state that the program will still run ???

Because that is all that is missing.

Bear with me, there are three parts to this and they're all important.

1) You said: "Of course, in such a case, tables are locked and concurrency is lost. That is why those two are mutually exclusive."

That sentence once again makes me believe that you think the --add-locks options locks the tables on the server during the export. It does not. (Perhaps you are thinking of mysql*dump*'s --lock-all-tables option which does do that.)

You are correct that using --add-locks prevents any concurrency, but not because the tables are locked, it's because:

"This option does not work with parallelism because INSERT statements from different tables can be interleaved and UNLOCK TABLES following the end of the inserts for one table could release locks on tables for which inserts remain."

--add-locks only affects the exported file.

2) That misunderstanding I think is what leads you to:

"you loose literally ALL benefits that are brought by consistent snapshot (enabled by --single-transaction)"

... which isn't the case. When using --single-transaction, --add-locks, and concurrency is disabled, the tables on the server remain unlocked, but a consistent export is created, AND the insert statements in the dump file are surround by LOCK/UNLOCK TABLES. There is no concurrency, but everything else is fine.

3) The language "mutually exclusive" be definition means the two *cannot exist* at the same time.

--add-locks and parallelism for example, are absolutely mutually exclusive. If one option is on, the other *cannot* be one. An error is produced if you try to turn both of them on.

--all-databases and --databases are mutually exclusive also. It makes no sense if both are present, and an error is produced.

--add-locks and --single-transaction does work. It is not an error, it simply requires that *concurrency* also be turned off. This is not the definition of "mutually exclusive". That language is simply incorrect.

Simply removing the lines: "--add-locks and --single-transaction are mutually exclusive" actually corrects the documentation. Technically there's nothing else that needs to be said because --single-transaction and --add-locks do work together, but as the documentation also says: "Usage of --add-locks is mutually exclusive with parallelism." Which means the user would also have to disable concurrency. A further explaination like my own at the bottom of #2 might be useful.

Hi,

You are completely wrong. --add-locks has nothing to do with external file, which lead you to come to totally wrong conclusions.

From the mysqlpump source:

  if (m_options->m_add_locks)
    this->append_output("LOCK TABLES ") + ......... );

Hence it is table locks that are added with that option. Since the only real advantage of --single-transaction is a lock-less full concurrency operation without LOCK TABLES statement, while --add-locks adds the locking of the table, those two are mutually exclusive.

Not a bug.

Take another look. 

Sql_formatter::append_output is calling Abstract_output_writer_wrapper::append_output which sends the string to each of its output writers, which you can see in Mysqldump_tool_chain_maker::create_chain the output writer being added to the Sql_formatter is the file. The add_locks code you quoted, is appending to the string which is written to the file. 

(You can also look at everything else in Sql_formatter appending to the output string, like all the DROPs, ALTERs, INSERTs... that's all the stuff being written into the file, not executed.)

Thank you for the info, but I am well aware of this, since I am one of the original authors of mysqldump (from which mysqlpump evolved).

Problem is in DDLs and restore of the dump file. 

As it is well described in our source, due to concurrent DDLs you might get inconsistent restore.

I am completely baffled by your response.

You said "You are completely wrong. --add-locks has nothing to do with external file" The code you quoted *proves* that I am right, I explained exactly how, and yet now you say are "well aware" of this?

"As it is well described in our source, due to concurrent DDLs you might get inconsistent restore."

This is completely irrelevant to this discussion.

You seem to have again misunderstood what I've said. This code:

  if (m_options->m_add_locks)
    this->append_output("LOCK TABLES ") + ......... );

... is *writing to the dump file*. I am not wrong. Read the code again.

I understood you 100 % correct. You do not seem to understand me, though.

--single-transaction has consequences during the run of mysqlpump, hence during the backup.

--add-locks has only consequences in the restore.

That is where the problem lies.

The first one permits concurrent DDLs and the second option does not. permit them. Which can make problems for consistent restore. That is why those two options are exclusive.

I completely agree with these statements:
   --single-transaction has consequences during the run of mysqlpump, hence during the backup.
   --add-locks has only consequences in the restore.

So we're good there.

As for the "problems for consistent restore" etc, I don't think there is. My earlier comment from [26 Feb 17:22] discusses this under #2. If concurrency is disabled, there's still a logical benefit of a lockless consistent read across all tables, which is where I disagree.

Well, it is good that we agree.

Regarding those those "exclusive" sentences in our manual, those are based on some real-world experiences with heavy-duty, very high loaded servers, running DDLs in concurrent sessions ......