MDEV-39061 mariadb-backup compatible wrappers for BACKUP SERVER - #5140
MDEV-39061 mariadb-backup compatible wrappers for BACKUP SERVER#5140Thirunarayanan wants to merge 5 commits into
Conversation
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces server-side backup support (BACKUP SERVER) for MariaDB, implementing backup and log archiving mechanisms for both the InnoDB and Aria storage engines. It also includes a compatibility wrapper script (mariabackup.sh) to map legacy mariabackup CLI commands to the new server-side SQL interface. The code review identified several critical issues, including a potential server crash in backup_innodb.cc due to invalid format arguments in error reporting, null pointer dereferences and assertion failures in both engines when backup steps are executed out of order or fail to initialize, and security vulnerabilities (command injection) and argument parsing bugs in the shell wrapper script.
| if [[ -n "$FINAL_INCLUDE" ]]; then | ||
| echo "Setting backup_include='$FINAL_INCLUDE'" >&2 | ||
| mariadb $MARIADB_OPTS -e "SET GLOBAL backup_include='$FINAL_INCLUDE'" | ||
| fi | ||
|
|
||
| if [[ -n "$FINAL_EXCLUDE" ]]; then | ||
| echo "Setting backup_exclude='$FINAL_EXCLUDE'" >&2 | ||
| mariadb $MARIADB_OPTS -e "SET GLOBAL backup_exclude='$FINAL_EXCLUDE'" | ||
| fi | ||
|
|
||
| # Execute BACKUP SERVER | ||
| SQL="BACKUP SERVER TO '$TARGET_DIR'" | ||
| echo "Executing: $SQL" >&2 | ||
| mariadb $MARIADB_OPTS -e "$SQL" |
There was a problem hiding this comment.
The parameters FINAL_INCLUDE, FINAL_EXCLUDE, and TARGET_DIR are interpolated into shell commands. To effectively prevent command injection, we should validate these inputs using a strict whitelist of allowed characters (such as alphanumeric characters and a limited set of safe symbols like '-', '_', '.') instead of relying on a blacklist or simple escaping.
| if [[ -n "$FINAL_INCLUDE" ]]; then | |
| echo "Setting backup_include='$FINAL_INCLUDE'" >&2 | |
| mariadb $MARIADB_OPTS -e "SET GLOBAL backup_include='$FINAL_INCLUDE'" | |
| fi | |
| if [[ -n "$FINAL_EXCLUDE" ]]; then | |
| echo "Setting backup_exclude='$FINAL_EXCLUDE'" >&2 | |
| mariadb $MARIADB_OPTS -e "SET GLOBAL backup_exclude='$FINAL_EXCLUDE'" | |
| fi | |
| # Execute BACKUP SERVER | |
| SQL="BACKUP SERVER TO '$TARGET_DIR'" | |
| echo "Executing: $SQL" >&2 | |
| mariadb $MARIADB_OPTS -e "$SQL" | |
| if [[ -n "$FINAL_INCLUDE" && ! "$FINAL_INCLUDE" =~ ^[a-zA-Z0-9_.-]+$ ]]; then | |
| echo "Error: Invalid characters in FINAL_INCLUDE" >&2 | |
| exit 1 | |
| fi |
References
- When validating input that will be interpolated into shell commands, use a strict whitelist of allowed characters (e.g., alphanumeric and a limited set of safe symbols like '-', '_', '.') instead of a blacklist to effectively prevent command injection.
| TABLES_PATTERN="" | ||
| TABLES_EXCLUDE_PATTERN="" | ||
| TABLES_FILE="" | ||
| MARIADB_OPTS="" |
There was a problem hiding this comment.
Handling MARIADB_OPTS as a flat string will break if any option contains spaces or special characters (e.g., --password="my secret pass"). We should use a bash array instead.
| MARIADB_OPTS="" | |
| MARIADB_OPTS=() |
| --user=*|--password=*|--host=*|--port=*|--socket=*) | ||
| # Connection options (long form) - pass through to mariadb client | ||
| MARIADB_OPTS="$MARIADB_OPTS $1" | ||
| shift | ||
| ;; | ||
| -u|-p|-h|-P|-S) | ||
| # Short form. If the next argv looks like a value (not another | ||
| # flag), consume it as well — supports both `-u USER` and bare | ||
| # `-p` (which mariadb client treats as a password prompt). | ||
| if [[ -n "${2-}" && "$2" != -* ]]; then | ||
| MARIADB_OPTS="$MARIADB_OPTS $1 $2" | ||
| shift 2 | ||
| else | ||
| MARIADB_OPTS="$MARIADB_OPTS $1" | ||
| shift | ||
| fi | ||
| ;; | ||
| -u*|-p*|-h*|-P*|-S*) | ||
| # Bundled short form: -uUSER, -pSECRET, -hHOST, ... | ||
| MARIADB_OPTS="$MARIADB_OPTS $1" | ||
| shift | ||
| ;; | ||
| --defaults-file=*|--defaults-extra-file=*) | ||
| # Config file options - pass through to mariadb client | ||
| MARIADB_OPTS="$MARIADB_OPTS $1" | ||
| shift | ||
| ;; |
There was a problem hiding this comment.
We should append connection options to the MARIADB_OPTS array instead of concatenating them into a flat string to correctly handle arguments with spaces or special characters.
--user=*|--password=*|--host=*|--port=*|--socket=*)
# Connection options (long form) - pass through to mariadb client
MARIADB_OPTS+=("$1")
shift
;;
-u|-p|-h|-P|-S)
# Short form. If the next argv looks like a value (not another
# flag), consume it as well — supports both `-u USER` and bare
# `-p` (which mariadb client treats as a password prompt).
if [[ -n "${2-}" && "$2" != -* ]]; then
MARIADB_OPTS+=("$1" "$2")
shift 2
else
MARIADB_OPTS+=("$1")
shift
fi
;;
-u*|-p*|-h*|-P*|-S*)
# Bundled short form: -uUSER, -pSECRET, -hHOST, ...
MARIADB_OPTS+=("$1")
shift
;;
--defaults-file=*|--defaults-extra-file=*)
# Config file options - pass through to mariadb client
MARIADB_OPTS+=("$1")
shift
;;| } | ||
|
|
||
| #ifndef _WIN32 | ||
| int dir= open(target.str, O_DIRECTORY); |
There was a problem hiding this comment.
To be fully standard and portable, open with O_DIRECTORY should explicitly include the access mode O_RDONLY.
| int dir= open(target.str, O_DIRECTORY); | |
| int dir= open(target.str, O_RDONLY | O_DIRECTORY); |
| explicit Aria_backup(THD *thd, Target target) noexcept | ||
| : target(target) | ||
| #ifndef _WIN32 | ||
| , datadir_fd(open(maria_data_root, O_DIRECTORY)) |
There was a problem hiding this comment.
To be fully standard and portable, open with O_DIRECTORY should explicitly include the access mode O_RDONLY.
, datadir_fd(open(maria_data_root, O_RDONLY | O_DIRECTORY))
|
Hey @Thirunarayanan , have you thought of how this is going to end up in packaging? It is replacing the original mariadb-backup? It needs some INSTALL/INSTALL_SCRIPT cmake directives around this to give it a install location and a cmake component. debian installation/packaging would need the relevant debian/{package}.install to include te script. |
8d0dc79 to
051f681
Compare
dr-m
left a comment
dr-m
left a comment
There was a problem hiding this comment.
Please rebase this on the current branch.
| @@ -0,0 +1,82 @@ | |||
| --source include/have_mariabackup_wrapper.inc | |||
There was a problem hiding this comment.
Can this be implemented in a way that is compatible with the .combinations logic?
I would like most of --suite=mariabackup to be run both with the genuine mariadb-backup and with the BACKUP SERVER based wrapper scripts. Currently, this is the only test that exercises the wrappers, and it fails to test the original executables to demonstrate compatibility between them and the wrappers.
| --echo # | ||
| --echo # --stream=mbstream emits a valid tar archive to stdout | ||
| --echo # | ||
| --let $targetdir=$MYSQLTEST_VARDIR/tmp/bk_stream | ||
| --let $streamfile=$MYSQLTEST_VARDIR/tmp/bk_stream.tar | ||
| --exec $XTRABACKUP $defaults --backup --target-dir=$targetdir --stream=mbstream > $streamfile 2>$logfile | ||
| --exec tar -tf $streamfile > /dev/null | ||
| --let SEARCH_PATTERN=Creating tar stream | ||
| --source include/search_pattern_in_file.inc | ||
| --rmdir $targetdir | ||
| --remove_file $streamfile |
There was a problem hiding this comment.
I’m not sure if this should be in the current scope. We don’t have any MDEV-38362 streaming backup yet. What should be more in the scope is demonstrating that the mbstream wrapper script is syntactically compatible with its namesake utility.
| @@ -0,0 +1,19 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Can we write this for the POSIX shell? Bourne Again Shell (bash) is not available in all environments, for good reasons: reduced resource usage as well as attack surface; remember https://en.wikipedia.org/wiki/Shellshock_(software_bug)
| -p|--parallel) | ||
| SKIP_NEXT=1 | ||
| ;; | ||
| -p*) | ||
| ;; | ||
| --parallel=*) | ||
| ;; |
There was a problem hiding this comment.
These are necessary but not sufficient rules. mbstream -xp4 would be a valid invocation of the original utility (equivalent to -x -p4), but not of GNU tar.
I did test that mbstream indeed treats anything after -p as a single argument. For example, -p4t and -p4x would report the following, respectively:
Warning: option 'parallel': signed value 4398046511104 adjusted to 2147483647
Unknown suffix 'x' used for variable 'parallel' (value '4x'). Legal suffix characters are: K, M, G, T, P, E
mbstream: Error while setting value '4x' to 'parallel'
Please add a test file that covers both the real mbstream and this wrapper. Both with some invalid and valid invocation. For example, mbstream -t is not allowed, while tar -t is.
051f681 to
6e37d3a
Compare
| --parallel=*|--throttle=*|--no-lock|--safe-slave-backup) | ||
| # Handled server-side by BACKUP SERVER. | ||
| shift ;; |
There was a problem hiding this comment.
The --parallel option will have to be passed through to non-streaming backup.
8077134 in #4817 implements multi-threaded backup to a mounted file system. By default, only 1 thread will be used.
For streaming backup (which is not implemented yet except as stubs), I think that we must limit the backup to a single thread, because unlike the old xbstream or mbstream format, BACKUP SERVER will generate one stream per thread. So, the only way to end up with one backup stream is to use a single thread.
6e37d3a to
3427e04
Compare
9ce9751 to
1fcb44e
Compare
dr-m
left a comment
dr-m
left a comment
There was a problem hiding this comment.
Now that #4817 includes support for streaming (see the test backup.backup_stream, which passes on Linux, FreeBSD, Microsoft Windows and Apple macOS), it would be good to add support for streaming backup.
When I tested the merge of this to the current #4817, I got one test failure:
CURRENT_TEST: mariabackup.wrapper_basic
NOT FOUND /Executing: BACKUP SERVER TO/ in wrapper.log
mysqltest: In included file "./include/search_pattern_in_file.inc":
included from /mariadb/main/mysql-test/suite/mariabackup/wrapper_basic.test at line 19:
At line 65: command "perl" failed with error: 255 my_errno: 0 errno: 0
I couldn’t figure this out. At the very end of scripts/mariabackup/mariabackup.sh we are writing a configuration file, and that is what happens. I even added set -eux or set -eu to the start of the script, and it did not complain about any uninitialized variables. I think that set -eu would be a good safety measure.
Note: I am going to squash #4817 shortly.
| if ($errno) | ||
| { | ||
| --skip mariabackup.sh wrapper unavailable (script or sh missing) | ||
| } |
There was a problem hiding this comment.
/bin/sh can’t possibly be missing on a POSIX system; it is what system(3), popen(3) and friends will invoke.
| # | ||
| # $XTRABACKUP — now points at mariabackup.sh | ||
|
|
||
| --source include/linux.inc |
There was a problem hiding this comment.
I think we only want to exclude non-POSIX environments, more specifically, Microsoft Windows. For that, I would suggest the following:
--source include/not_windows.incor if it cannot be used for some reason:
if ($MARIADB_UPGRADE_EXE) {
--skip Need POSIX
}
87036f8 to
4769a43
Compare
86c9bc3 to
b99a766
Compare
b99a766 to
cee7cfc
Compare
dr-m
left a comment
dr-m
left a comment
There was a problem hiding this comment.
Here are some initial comments. I did not check the mariabackup.sh in detail, but I think that this is a very good start.
| ######################################################################## | ||
| # mariadb-backup-server: BACKUP SERVER-compatible shell wrapper | ||
| ######################################################################## | ||
| # A drop-in mariadb-backup-compatible POSIX-sh wrapper that translates the | ||
| # CLI into server-side BACKUP SERVER SQL. Experimental; OFF by default | ||
| # Installed as a mariadb-backup-server; clients opt in via symlink/alias | ||
| # (see scripts/mariabackup/README.md). | ||
| OPTION(WITH_MARIABACKUP_WRAPPER | ||
| "Install the BACKUP SERVER shell wrapper (mariadb-backup-server)" OFF) |
There was a problem hiding this comment.
scripts/mariabackup/CMakeLists.txt would be a less surprising place for this. Would it work? If not, then I would suggest moving scripts/mariabackup to extra/mariabackup/scripts.
| -v|--verbose) ;; | ||
|
|
||
| --) shift; break ;; | ||
| -*) ;; |
There was a problem hiding this comment.
Shouldn’t we throw an error for any unknown option? For example, -b 1 works in GNU tar, but certainly not in mbstream.
| # The wrapper streams several tar archives | ||
| # back to back (one per stream, plus backup-prepare.cnf), | ||
| # so --ignore-zeros is required to extract them all rather | ||
| # than stopping at the first archive's end-of-file marker. | ||
| x) exec tar --ignore-zeros -x -f - -C "$dir" ;; |
There was a problem hiding this comment.
We are not currently adding any NUL block at the end of each stream, so the GNU tar specific option --ignore-zeros should not be needed. If you install libarchive-tools, you should get a bsdtar that is compatible with the tar.exe that is shipped with Microsoft Windows. That executable does not recognize an --ignore-zeros option.
I think that we should try to make this script compatible also with BSD tar.
| if [ -n "$_enc" ]; then | ||
| echo "plugin-load-add=$_enc" |
There was a problem hiding this comment.
Two tests would fail unless we extend this with a patch that you shared with me:
_plugin_dir=$(ask "SELECT @@global.plugin_dir")
[ -n "$_plugin_dir" ] && echo "plugin-dir=$_plugin_dir"| esac | ||
| # Where "rr record" stores traces, so we can point at it on a crash. | ||
| rr_trace_dir=${_RR_TRACE_DIR:-${XDG_DATA_HOME:-$HOME/.local/share}/rr} | ||
|
|
There was a problem hiding this comment.
Do we really want it this complicated, or could we do the following:
: ${MARIADBD=mariadbd}Then, to have the invoked server run under rr record, one would execute (for example)
_RR_TRACE_DIR=/dev/shm/rr MARIADBD='rr record -h` mariabackup.shAlso, should we rather call this script mariadb-backup-server? The name mariabackup was deprecated and replaced with mariadb-backup some years ago.
fee9458 to
ac4c513
Compare
| # Write more than 2MiB of FILE_MODIFY mini-transactions to exercise the parser. | ||
| my $extra = pack("CCxa*", 0xb9, 127, "a/b.ibd"); | ||
| $extra .= pack("CN", 0, mycrc32($extra, 0, $polynomial)); |
There was a problem hiding this comment.
As you can see here, the mariabackup.huge_lsn,strict_full_crc32 combinations are writing some normally impossible log records. The SERVER variant (having the wrapper script invoke BACKUP SERVER) would hang on many targets because log_t::set_archive() is waiting for a checkpoint, but apparently the buf_flush_page_cleaner() thread has nothing to do because the buf_pool.flush_list is apparently empty; the log buffer is filled solely with FILE_ records and no page-level records, like it normally would be the case.
I am unable to reproduce this hang locally, but it does occur on many platforms. I will apply the following patch to #4817, which I think should fix this hang:
diff --git a/storage/innobase/log/log0log.cc b/storage/innobase/log/log0log.cc
index aa2063719d8..c313b3d9b51 100644
--- a/storage/innobase/log/log0log.cc
+++ b/storage/innobase/log/log0log.cc
@@ -810,7 +810,7 @@ bool log_t::set_archive(my_bool archive, THD *thd, bool backup) noexcept
if (wait_lsn)
{
mysql_mutex_lock(&buf_pool.flush_list_mutex);
- buf_flush_wait(wait_lsn, false);
+ buf_flush_wait(wait_lsn, !UT_LIST_GET_LEN(buf_pool.flush_list));
mysql_mutex_unlock(&buf_pool.flush_list_mutex);
}
latch.wr_unlock();There was a problem hiding this comment.
This was applied to 9e27d73, which you should rebase on.
There was a problem hiding this comment.
The test is still hanging, with log_t::set_archive() apparently stuck in an infinite loop. I think that you must find an environment where it can be reproduced and check the content of log_sys and buf_pool.flush_list during the hang.
9e27d73 to
5235e7a
Compare
dr-m
left a comment
dr-m
left a comment
There was a problem hiding this comment.
I think that this is good enough for a first step. I only have some minor commments. More may be found by testing.
| # To run the prepare bootstrap under rr, include it in MARIADBD | ||
| # (rr's own _RR_TRACE_DIR controls where the trace is written). | ||
| # e.g. _RR_TRACE_DIR=/dev/shm/rr MARIADBD='rr record mariadbd' ... | ||
| : "${MARIADB:=mariadb}" | ||
| : "${TAR:=tar}" |
There was a problem hiding this comment.
Do we need to mention a tool here that is not likely to work for end users, at least until the instrumentation of the copy_file_range system call has been fixed?
Why is the mariadbd executable not wrapped in the same way?
| # The wrapped commands can be overridden for testing, e.g. | ||
| # MARIADB='mariadb --protocol=tcp', TAR=bsdtar. |
There was a problem hiding this comment.
I’d suggest to omit the , after MARIADB= and the . after TAR= because literally copying the line to the start of a shell command should result in --protocol=tcp, which I assume is not going to work. Maybe, in a preceding line, say that the environment variables can be overridden at the start of a shell command, or give more complete examples:
# MARIADB='mariadb --protocol=tcp' TAR=bsdtar mariadb-backup-server.sh --backup ...
# TAR=bsdtar mariadb-backup-server.sh --prepare ...| # In stream mode --target-dir is optional: | ||
| # it is only a scratch directory for the per-stream tar parts | ||
| # (a mktemp dir is used when it is omitted). | ||
| [ -n "$STREAM" ] || [ -n "$TARGET_DIR" ] || die "--target-dir required" |
There was a problem hiding this comment.
Here and elsewhere, we are invoking the external command [ a.k.a. test multiple times. That command does support logical -a and -o. We should use them. man 1 test to learn the syntax.
| @@ -1,3 +1,4 @@ | |||
| --source include/have_mariabackup_combination.inc | |||
There was a problem hiding this comment.
We have the SERVER variant of this test failing on FreeBSD:
mariabackup.full_backup '32k,SERVER' w6 [ fail ]
Test ended at 2026-06-30 01:54:32
CURRENT_TEST: mariabackup.full_backup
mysqltest: At line 39: exec of '/data/buildbot/workers/prod/amd64-freebsd-14/build/mysql-test/../extra/mariabackup/scripts/mariadb-backup-server.sh --prepare --target-dir=/data/buildbot/workers/prod/amd64-freebsd-14/build/mysql-test/var/6/tmp/backup 2>&1' failed, error: 256, status: 1, errno: 22
Output from before failure:
/data/buildbot/workers/prod/amd64-freebsd-14/build/mysql-test/../extra/mariabackup/scripts/mariadb-backup-server.sh: mariadbd: not found
ac4c513 to
45b94b3
Compare
file_name_t::page0_lsn: Keep track of the last applied recv_sys_t::parse_page0() so that a multi-batch recovery will not reset the file to a smaller size. Reviewed by: Thirunarayanan Balathandayuthapani (cherry picked from commit 8f00e6c)
e5a9d2d to
b625be2
Compare
| sql="BACKUP SERVER WITH" | ||
| case $PARALLEL in # --parallel=N -> "N CONCURRENT" (1 is default) | ||
| ''|*[!0-9]*) ;; | ||
| *) [ "$PARALLEL" -gt 1 ] && sql="$sql $PARALLEL CONCURRENT" ;; | ||
| esac |
There was a problem hiding this comment.
This is off by one. In mariadb-backup --backup, there is a dedicated log_copying_thread() that runs concurrently with any other copying threads. The number of CONCURRENT must be 1 more than the number specified by --parallel, always at least 2.
For supporting the --stream variant (which I suppose could be supported via a named pipe), I believe that we would have to stick to 1 thread so that a single stream will be produced.
There was a problem hiding this comment.
I can see two ways of supporting a genuine streamed operation. One is via named pipes (created with mkfifo, possibly the "server" or "sink" could be cat started in the background.) Another is via sockaddr_un(3type), served by Netcat (nc) which some Galera scripts already depend on: https://stackoverflow.com/questions/25256320/best-way-to-use-unix-domain-socket-in-bash-script
In both cases, I assume that the file name may have to be created with mktemp(1).
| # The server appends the stream index as $1 and writes that stream's tar | ||
| # to our stdin; we drop each one into the scratch dir under <index>.tar. | ||
| helper=$scratch/.mariabackup-stream.sh | ||
| cat > "$helper" <<EOF | ||
| #!/bin/sh | ||
| exec cat > "$scratch/\$1.tar" | ||
| EOF |
There was a problem hiding this comment.
This does not look like streaming operation to me. We appear to write the entire uncompressed stream to a local file, and maybe subsequently copy that tile to an output stream. Proper streaming operation could be possible by writing the output to a named pipe, which would be relayed to the standard output.
log_t::set_archive(archive=false): Ensure that both the latest checkpoint and the latest log record (which has possibly not been written out yet) will carry the log_sys.get_sequence_bit(lsn)==1, to guarantee a successful recovery after the switch to the innodb_log_archive=OFF format. Tested by: Matthias Leich Reviewed by: Thirunarayanan Balathandayuthapani (cherry picked from commit 1637316)
log_t::set_archive(): Prevent a crash in log_t::header_rewrite() by refusing the operation if the log is read-only.
The following SQL statements will be introduced: BACKUP SERVER TO '/path/to/directory' [ 1 CONCURRENT ]; BACKUP SERVER WITH [ 1 CONCURRENT ] 'command'; In place of the 1, any positive number of threads may be specified. For the first variant, '/path/to' must exist and '/path/to/directory' must be compatible with secure_file_priv and not exist; that is where the backup will be written to. For the second variant, 'command' must be the name of a script or command that will be executed in a child process. The standard input of that command will be in a format that is compatible with GNU tar --format=oldgnu (and also BSD tar variants that are also part of Microsoft Windows and Apple macOS). The command is expected to optionally compress and encrypt the stream and redirect it to a file on a local or a remote server. The BACKUP SERVER WITH will append an additional argument, a positive base-ten number in ASCII, starting with 1, to identify the current thread. In this way, each concurrent stream can write a separate file. The InnoDB write-ahead log will be streamed near the end, because the stream format requires the file sizes to be declared upfront. TODO: implement the following: In BACKUP SERVER TO ... 2 CONCURRENT or more, one thread will be responsible for copying the InnoDB write-ahead log from the beginning of the backup. Other files will be copied by other threads. Note: In single-threaded BACKUP SERVER TO, the parameter innodb_log_recovery_start that is written out to backup.cnf is STRICTLY NECESSARY TO AVOID CORRUPTION during recovery! By default, InnoDB crash recovery starts from the latest available log checkpoint. However, for restoring a backup, recovery must start from the checkpoint that was the latest when the backup was started. Starting recovery from a possible later checkpoint will result in a corrupted database! The backup or the first stream will contain a file backup.cnf, which includes parameters needed for restoring the backup. Currently, these are innodb_log_recovery_start and innodb_log_recovery_target. If innodb_log_recovery_target>0, InnoDB will be in read-only mode, not allowing any writes to persistent files other than via the log application. To restore a streaming backup made with BACKUP SERVER WITH, an empty directory needs to be created and all streams be extracted there using the standard tar utility of the operating system, optionally after undoing any encryption or compression that had been added by the backup command. Then, the backup is prepared or MariaDB server started up on the extracted directory, similar to as if the BACKUP SERVER TO statement had been used. The following will be implemented separately: MDEV-39061 mariadb-backup compatible wrapper script for BACKUP SERVER MDEV-40163 Partial backup and restore MDEV-39091 Back up ENGINE=RocksDB MDEV-40333 Concurrent DDL for Aria tables in BACKUP SERVER The implementation introduces a basic multi-threaded driver Sql_cmd_backup, storage engine interfaces, and basic copying of the storage engines InnoDB, Aria, MyISAM, MERGE (MyISAM), Archive, CSV. backup_target: A structured data type to represent a target directory. On Microsoft Windows, we must use directory paths because there is no variant of CopyFileEx() that would work on file handles. backup_sink: Wraps a per-thread output stream as well as storage engine specific context. handlerton::backup_start(), handlerton::backup_end(): Invoked at the start or end of a backup phase, in the thread that executes a BACKUP SERVER statement. handlerton::backup_step(): A backup step that can be invoked from multiple threads concurrently, between the execution of the corresponding handlerton::backup_start() and handlerton::backup_end() of the same phase. copy_entire_file(): A file copying service for POSIX systems. copy_mmap(): A zero-copy alternative to backup::copy(), to copy from a memory-mapped buffer. copy_file_range_try(): A wrapper for Linux copy_file_range(2), which may fail with EOPNOTSUPP or EXDEV and thus require a fallback to copy_mmap() or backup::copy(). backup::copy(): A partial or sparse file-copying service. On other platforms than FreeBSD or Microsoft Windows, there are shortcut alternatives to this. Note: On Linux we never invoke sendfile(2) for copying between files, because can be much slower than the alternatives. backup_stream_append_plain(): A wrapper of backup::append(), which is the streaming equivalent of backup::copy(). backup_stream_zeropad(): Zero-pad the last tar block if needed. backup_stream_append_async(): A variant of backup_stream_append_plain() where the source file region is guaranteed to be immutable after the call returns. Zero-copy mmap(2) or Linux sendfile(2) are inherently risky for copying data files that may be modified in place, because it could introduce a race condition between a page write that runs concurrently with a child process that is reading the data from the pipe. backup::append(): On systems where we can determine the size of the pipe buffer, invoke backup_stream_append_async() for the initial write, and pread_write() for the last part, to guarantee that the data written by the zero-copy shortcut will have been consumed before the call returns and the caller is able to resume writes to the source region. pread_write(): On 64-bit systems, allocate a buffer of up to 1 MiB. This is the "slow path" of copying or streaming files. struct Aria_backup: Context for multi-threaded backup, comprising directory handles, a mutex and enum Aria_backup_status. aria_backup_start(): Prepare the context for aria_backup_step(). Most files are copied in BACKUP_PHASE_NO_DDL after flush_tables(thd, FLUSH_NON_TRANS_TABLES) has been invoked. All ENGINE=Aria files (including TRANSACTIONAL=0) are copied in BACKUP_PHASE_NO_COMMIT. Thanks to Andrzej Jarząbek for writing test cases and suggesting this logic. aria_backup_step(): Copy one non-ACID file. Acquires Aria_backup::mutex, traverses directories to construct one file name, releases the mutex, and copies the file if one was found. aria_backup_data(): Copy one data file. On Microsoft Windows, this assumes that the current directory is the datadir. This assumption would not hold in the Embedded Server library, which is not supported on Microsoft Windows. aria_backup_log(): Copy one ENGINE=Aria log file. aria_backup_end(): Finish a copying phase and clean up the context. InnoDB_backup::init(): Wait for a possible previous BACKUP SERVER operation to reach the very end of InnoDB_backup::context::cleanup() so that the context can be safely reused. InnoDB_backup::queue: Collection of tablespace IDs and payload sizes at the start of the backup, and the log_sys.first_lsn of log files that have to be included in the backup. If any data file is created or extended while the backup is executing, we must have the corresponding write-ahead-log entries that we are copying since the latest checkpoint that was completed when the backup started. If any tablespaces are deleted during the backup, we may or may not copy them, and the application of a FILE_DELETE record will remove them. Similarly, applying FILE_RENAME or FILE_CREATE records will rename or create files during recovery as needed. log_sys.backup: Whether BACKUP SERVER is in progress. The purpose of this is to make BACKUP SERVER prevent the concurrent execution of SET GLOBAL innodb_log_archive=OFF or SET GLOBAL innodb_log_file_size when innodb_log_archive=OFF. log_sys.archived_checkpoint: Keep track of the earliest available checkpoint, corresponding to log_sys.archived_lsn. This reflects SET GLOBAL innodb_log_recovery_start (which is settable now), for incremental backup. fil_system.have_all_spaces: Whether all tablespace metadata is guaranteed to be known. To speed up startup, InnoDB does not normally open all tablespace files. fil_space_t::create_lsn: Change to Atomic_relaxed and use this to indicate tablespace creation LSN, in addition to indicate undo tablespace rebuild LSN. fil_space_t::backup_end: The first page number that is not being backed up (by default 0, to indicate that no backup is in progress). fil_space_t::BACKUP_BATCH_SIZE: The number of preceding pages that will be covered by fil_space_t::backup_end. This is the unit of "page range locking" during InnoDB backup. buf_page_t::write_fix_try(), buf_page_t::write_unfix_try(): Try to set or unset a fake "write fix" on a page, to prevent concurrent flush() during a backup batch. The atomic operations may run concurrently with set_reinit() and set_freed(). The fake "write fix" does not prevent any concurrent read or write of the page data in the buffer pool; it only blocks writes to the underlying data file. buf_page_t::flush(): Atomically test and set write fix, and skip the operation if the fake "write fix" was set. buf_page_t::set_freed(), buf_page_t::set_reinit(): Employ a compare-and-exchange loop to accommodate for the "write fix". innodb_backup_batch_wait(): Look up any pages that we are about to back up. For any dirty pages, invoke buf_page_t::write_fix_try() to try to set a fake "write fix" lock-free. If the page is currently write-fixed between buf_page_t::flush() and buf_page_t::write_complete(), acquire and release a page U-latch to wait for the conflicting write to complete. InnoDB_backup::backup_batch_start(), InnoDB_backup::backup_batch_stop(): Adjust fil_space_t::backup_end and fake "write fix" of dirty pages to protect the copying of a range of pages from the underlying file. InnoDB_backup::commit(): Enqueue the remaining log to be copied. InnoDB_backup::checkpoint_complete(): If backup is running and commit() has not been called, add each completed innodb_archive_log=ON file to the queue. Else, skip or delete, as appropriate. log_t::backup_start(): If we were running with innodb_log_archive=ON, ensure that the latest file is a valid recovery starting point. That is, wait for the latest log checkpoint to be within the file. buf_flush_list_space(): Check for concurrent backup before writing each page. This is inefficient, but this function may be invoked from multiple threads concurrently, and it cannot be changed easily, especially for fil_crypt_thread(). fil_ibd_create(): Set fil_space_t::create_lsn after the file has been created. dict_load_tablespaces(): Determine the size of each file if upgrade==true. Backup depends on that. buf_dblwr_t::begin(), buf_dblwr_t::end(), buf_dblwr_t::size(): Accessors to allow BACKUP SERVER to skip the contents of the doublewrite buffer in the system tablespace. It is only useful for crash recovery in case a data page had been incompletely written by the time the server was killed. If the server is killed during a backup, the backup will be incomplete and unusable anyway. Furthermore, the page range locking makes page writes and backup mutually exclusive.
This adds a shell script that lets users keep using their existing mariadb-backup commands while the real work is done by the new server-side BACKUP SERVER command. The goal is "drop-in": users should not have to change their backup scripts. extra/mariabackup/scripts/mariadb-backup-server.sh (plain POSIX sh) understands the usual mariadb-backup modes and translates each one. A companion helper, extra/mariabackup/scripts/mbstream-server.sh, lets streamed backups be unpacked by pipelines that expect the mbstream CLI. Both are documented in extra/mariabackup/scripts/README.md. --backup ======== Connects with the mariadb client and runs "BACKUP SERVER TO '<dir>'". Connection options (--user, --host, --port, --socket, --defaults-file, ssl, ...) are passed through to the client; --parallel=N becomes the "<N + 1> CONCURRENT": mariadb-backup runs a dedicated log_copying_thread() besides its N-data copy threads. Minimum value is 2 After the backup it writes backup-prepare.cnf into the backup directory, recording what --prepare needs later: where mariadbd lives, the InnoDB parameters (page size, data file path, undo tablespaces, checksum algorithm, log file size), and if the server is encrypted then how to reload the encryption key plugin (the file_key_management variables), so an encrypted backup can be prepared without extra input. --backup --stream ================= Runs "BACKUP SERVER WITH '<command>'", where <command> writes the tar into a fifo that the wrapper drains to its own stdout, so the backup reaches the consumer as it is produced and never lands on local disk. backup-prepare.cnf is appended as a final tar afterwards. The server's tar carries no end-of-archive marker; only that trailing archive adds one, so the whole stream extracts with a plain "tar -x". --parallel is ignored here, with a warning. CONCURRENT selects the number of output sinks as well as workers, and N self-contained tars cannot be interleaved on one stdout, so a stream needs one writer. For parallel streaming use BACKUP SERVER WITH N CONCURRENT directly, giving each stream its own destination via the appended index. Three properties follow from how BACKUP SERVER streams, all differing from mariadb-backup: - local: the stream command runs inside the server, so the wrapper must share its filesystem; ---target-dir is optional in stream mode (scratch for the per-stream. - tar only: any --stream=<format> (including xbstream) yields tar; - single-threaded: one worker, so no parallel read either. --target-dir is optional in stream mode; nothing is spooled there, it only holds the helper script, the fifo and backup-prepare.cnf (a mktemp dir is used otherwise). mbstream-server.sh maps the mbstream CLI onto a plain "tar -x"/"tar -c", so existing "mbstream -x"/"-c" pipelines keep working on the wrapper's stream. mbstream-only flags (-p/--parallel, ...) are accepted and ignored; any other unknown option is rejected. Environment overrides (mainly for testing): MARIADB (client), MARIADBD (the --prepare bootstrap server) and TAR (the tar implementation, e.g. TAR=bsdtar) can each be overridden. To run the bootstrap under rr, put it in MARIADBD and let rr's own _RR_TRACE_DIR choose the trace location, e.g. _RR_TRACE_DIR=/dev/shm/rr MARIADBD='rr record mariadbd' --prepare ========= Starts "mariadbd --bootstrap" on the backup directory using backup-prepare.cnf as its defaults file, replays the archived redo log between the start and target LSN read from backup.cnf, then builds a fresh ib_logfile0 so a normal server can start on the directory. mariadbd is taken from the path recorded in backup-prepare.cnf if that binary exists, otherwise by searching /libexec, /sbin, /bin and the configured install directories. PATH is not searched; set MARIADBD to point elsewhere. User --defaults-file/-extra-file and encryption options are layered onto the bootstrap. --copy-back / --move-back ========================= Copy or move a prepared backup into the datadir. The datadir is created if missing, a non-empty datadir is refused unless --force-non-empty-directories is given, and a chown reminder is printed. If --aria-log-dir-path is given, the Aria logs (aria_log_control, aria_log.*) are relocated into that directory. Packaging ========= The wrapper is not installed by default and never replaces the real mariadb-backup / mbstream binaries. 1. cmake -DWITH_MARIABACKUP_WRAPPER=ON (default OFF) controls it. 2. When ON, the scripts install as /usr/bin/mariadb-backup-server and /usr/bin/mbstream-server, tagged COMPONENT Backup so they ship in the mariadb-backup package. 3. RPM: nothing extra to do. the component handles it. 4. DEB: not wired. debian/rules uses --fail-missing and does not enable the option, so the -server binaries are not listed. To ship via DEB, make a paired change: add -DWITH_MARIABACKUP_WRAPPER=ON in debian/rules and list both usr/bin/mariadb-backup-server and usr/bin/mbstream-server in debian/mariadb-backup.install together. 5. The real mariadb-backup/mbstream binaries and the mariabackup symlink are left untouched; opt in via an alias or a symlink early in PATH. Limitations (not supported yet) =============================== 1) Incremental backup & prepare (--incremental-basedir, --incremental-dir, --apply-log-only) 2) --rollback-xa 3) Partial backup (--databases, --tables, --tables-file) 4) Output compression and encryption (--compress, --encrypt) 5) --export is accepted but only warns and runs a plain recovery 6) --extra-lsndir is ignored 7) --parallel is ignored with --stream 8) Windows: POSIX sh only, not installed on Windows Behaviour differences from native mariadb-backup ================================================ - The wrapper needs the mariadb client on PATH for --backup, and mariadbd on PATH (or recorded in backup-prepare.cnf) --backup; --prepare needs mariadbd recorded in backup-prepare.cnf, in a standard install directory, or named by MARIADBD - BACKUP SERVER refuses an already-existing target directory - BACKUP SERVER does copy the data file as raw pages without checksum validation, so a corrupted table is not detected at backup time - --prepare only works on a wrapper-made backup. It needs backup-prepare.cnf) - --stream is tar, not xbstream, local-only and single-threaded Tests ===== include/have_mariabackup_wrapper.inc redirects $XTRABACKUP to mariadb-backup-server.sh and $XBSTREAM to mbstream-server.sh, skipping when a wrapper or the mariadb client is unavailable. include/have_mariabackup_combination.inc runs a test under both the [CLIENT] mariadb-backup binary and the [SERVER] wrapper.
1370d5a to
0a23ebb
Compare
4 participants
This adds a shell script that lets users keep using their existing
mariadb-backup commands while the real work is done by the new
server-side BACKUP SERVER command. The goal is "drop-in": users should
not have to change their backup scripts.
extra/mariabackup/scripts/mariadb-backup-server.sh (plain POSIX sh)
understands the usual mariadb-backup modes and translates each one.
A companion helper, extra/mariabackup/scripts/mbstream-server.sh,
lets streamed backups be unpacked by pipelines that expect the
mbstream CLI. Both are documented in extra/mariabackup/scripts/README.md.
--backup --stream
=================
Runs "BACKUP SERVER WITH ''", where writes the
tar into a fifo that the wrapper drains to its own stdout, so the
backup reaches the consumer as it is produced and never lands on
local disk.
bootstrap under rr, put it in MARIADBD and let rr's own _RR_TRACE_DIR
choose the trace location, e.g.
_RR_TRACE_DIR=/dev/shm/rr MARIADBD='rr record mariadbd'
enable the option, so the -server binaries are not listed.
To ship via DEB, make a paired change: add
-DWITH_MARIABACKUP_WRAPPER=ON in debian/rules and list both
usr/bin/mariadb-backup-server and
usr/bin/mbstream-server in debian/mariadb-backup.install together.
5. The real mariadb-backup/mbstream binaries and the
mariabackup symlink are left untouched; opt in via an alias or a
symlink early in PATH.
Tests