MySQL Bugs: #54621: Use of pipe in the backup dir name shuts down the server when performing MEB

Bug #54621	Use of pipe in the backup dir name shuts down the server when performing MEB
Submitted:	18 Jun 2010 19:25	Modified:	30 Sep 2010 22:32
Reporter:	Hema Sridharan	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Enterprise Backup	Severity:	S3 (Non-critical)
Version:	mysql-5.1-meb, MEB 3.1.0	OS:	Any
Assigned to:	Ingo Strüwing	CPU Architecture:	Any

Description:
* Create backup directory name that has special character pipe (say meb_bup_|dir)
* Now execute the innobackup
* Shutdown the server, apply logs for recovery and execute copy-back option of innobackup
* Now restart server and the server will shut down

How to repeat:
Please execute the test attached in the file to reproduce the issue.

Suggested fix:
Ideally, the server should not shutdown and issue an error message.

Please find the attached test file

Attachment: innobackup_shutdown.test (application/test, text), 2.15 KiB.

Thank you for the report.

Verified as described. I am leaving MEB category, because according to error messages it is MEB who did not create necessary files.

The attached innobackup_shutdown.test uses the shell meta character '|'
(pipe) in the backup directory name and does not quote it. The
resulting, simplified command line is this:

  innobackup ... meb_bup_|dir > backup.log

Or, to make the shell meta character more visible:

  innobackup ... meb_bup_ | dir > backup.log

Innobackup starts to execute, using meb_bup_ as the backup directory and
writes status messages to the pipe.

The command 'dir' starts to execute at the same time and writes the
current directory's file list to backup.log.

'dir' finishes before innobackup. It closes its end of the pipe. The
compound statement (the pipe innobackup|dir) finishes with the exit code
of the last command in the pipe, 'dir', which is zero (success).
mysqltest assumes, --exec innobackup ... succeeded and starts the next
statement. But innobackup did not finish yet. It continues to work in
the background.

The next status message from innobackup goes to the pipe, which has no
reader any more. This is called a "broken" pipe. Innobackup receives
SIGPIPE from the operating system. It catches this signal, writes an
error message about the broken pipe to stderr, and exits with an error
code. Since stderr is not redirected and not catched by MTR, it goes
directly to the controlling terminal. Since innobackup ran in the
background, no one cares about its error exit code. Note that innobackup
did not do a complete backup at this stage.

The same sequence repeats with the apply-log.

Then innobackup_shutdown.test removes the complete datadir tree.

The foolowing copy-back operation fails on the same problem as the other
calls of innobackup. The result is a vastly incomplete datadir. The
attempt to restart the server on it fails due to missing system tables.
The server cannot work around it and shuts down.

IMHO it is kind of horseplay to pipe innobackup's output into a pipe,
which breaks during innobackup's execution. The next possible bug report
might complain that innobackup doesn't work correctly if the backup
directory is specified as /dev/null or similar.

Also it is questionable if the pipe symbol works in the same way on all
platforms. We shouldn't use any characters unquoted, which could be
special to any command interpreter on the platforms, the tests shall
run on.

Fix 1:
Nonetheless there is a possible way around this problem. In the signal
handler for SIGPIPE, innobackup does already make a difference between a
broken pipe the the mysql client and other pipes. Since innobackup
doesn't establish other pipes itself, the broken one could only be
stdout or stderr. Since innobackup does not write to stderr explicitly
(but only through die()), it is most likely that stdout is broken.
Innobackup could redirect stdout to stderr and try to continue its work.
The user won't have a good log of innobackup's operation, but having the
log in stderr may be better than not having it at all. But the main
purpose of this change is to let innobackup continue its work, even if
stdout breaks. Note that I'm not sure if this change in behavior is
wanted. To stop, if stdout is broken during the operation may be a valid
approach.

Fix 2:
Quote the pipe symbol in innobackup_shutdown.test.

But even if innobackup_shutdown.test is fixed to properly quote the pipe
symbol, the test still does not work. Innobackup does not properly quote
the backup directory argument internally.

Fix 3:
Add quotes to all uses of $backup_dir and to all uses of all variables
that are composed from it.

With the tree fixes applied, the test passes. Whith only fix 1 and 3
applied, the test works too, but adds some file listings from some 'dir'
commands to the test result. Also messages from innobackup (that are now
redirected to stderr) flood the screen.

I will propose patches for the proposed fixes.

The patch for innobackup

Attachment: bug54621-innobackup-1.7.1-rc.patch (text/x-patch), 3.08 KiB.

The commit email for the test case

Attachment: bug54621-mysql-5.1-meb-commit.eml (message/rfc822, text), 15.20 KiB.

A Clearly Written Patch.
Patch Approved!

The innodb fix is pushed (svn commit) to innodb-innobackup/tags/1.7.1-rc and innodb-innobackup/trunk.
The test case is pushed to mysql-5.1-meb.

Bug does not appear in any released version. No changelog entry needed.