MDEV-14992 BACKUP SERVER

[dr-m]

The following SQL statements will be introduced:

BACKUP SERVER TO '/path/to/directory';
BACKUP SERVER TO '/path/to/directory' 1 CONCURRENT;
BACKUP SERVER WITH 'command';
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 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 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.

Note: The parameter innodb_log_recovery_start in backup.cnf is STRICTLY NECESSARY TO AVOID CORRUPTION! 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 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-39092 Less blocking backup of ENGINE=Aria

The implementation introduces a basic 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_file(): A partial or sparse file-copying service for all systems.

backup_stream_append(): Equivalent to copy_file(), but appending to a stream. On Linux, this uses sendfile(2), which assumes that the source data will not be changed before the data has been consumed from the pipe.

backup_stream_append_async(): A variant of backup_stream_append() where the source file region is guaranteed to be immutable after the call returns. We must not use Linux sendfile(2) 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.

InnoDB_backup::context: Backup context, attached to backup_sink so that context can continue to exist between the time a BACKUP SERVER releases all locks and another BACKUP SERVER starts executing, with innodb_backup pointing to the new backup, while the old backup is still being finished.

fil_space_t::write_or_backup: Keep track of in-flight page writes and pending backup operation. We must not allow them concurrently, because that could lead into torn pages in the backup.

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.

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.

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_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.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[dr-m]

I plan to rebase this once #5070 has been merged up to the 13.0 branch. @grooverdan pushed a merge to 10.11 today, and I pushed to 11.4 and 11.8. I hope that the conflicts for 12.3 and potentially 13.0 will have be resolved by Monday.

The ultimate merge target is main. For testing, it is better to be based on the oldest maintained branch that includes #4405, which forms the fundament for this, innodb_log_archive=ON.

While rebasing, I will write a description based on the commit message of 4769a43, but mentioning actual MDEVs for the outstanding work. Soon after the rebase, we can include #5140 so that this can be tested more conveniently.

[dr-m]

If this is a ROW_FORMAT=COMPRESSED table, then the file may be 1024, 2048, or 3172 bytes shorter than calculated, and the copying could fail. This API as well as the one in stream() must be refactored so that we will know how much was actually copied. The reason for this short file is that fil_space_extend_must_retry() will only extend files to integer multiples of 4096 bytes.

In stream() we must pad with field_ref_zero so that the file size will match what was written to the header. The last page will be recovered from the redo log.

Note: We don’t currently keep track of the file size or the allocated file size as of the checkpoint when the backup started. If we did that, we could copy even less. That could be an even more elegant fix of this. I think we would create sparse files that match the current file size.

[Thirunarayanan]

BACKUP SERVER WITH ''` runs an arbitrary shell command as the mysqld OS user, gated only by global RELOAD + SELECT_ACL. Is a dedicated privilege planned? Should there be a server switch to disable the WITH variant, and is the command captured by the audit log?

[Thirunarayanan]

what if we cached backup_page_end before backup server set backup bit? We cache it only once. This
could lead to torn page read?

[dr-m]

InnoDB_backup invokes fil_space_t::backup_start(), which will assign last_page and subsequently invoke write_or_backup.fetch_add(BACKUP) and return the old value. Let us consider the two possible execution orders:

1. InnoDB_backup::backup_batch_start() atomically sets the BACKUP flag and reads the write count as 0
2. A writer invokes fil_space_t::writing_start(), that is, write_or_backup.fetch_add(1) and returns the BACKUP flag as set.
3. Because the BACKUP flag was set, the writer will sample the space->backup_page_end() (which the preceding step had written before setting the BACKUP flag) and skip any writes to this page range.
4. The writer will invoke space->writing_stop() to decrement write_or_backup.fetch_sub(1) and InnoDB_backup will clear the BACKUP flag after completing the page range flush, with no overlap.

Another possibility is that the two first steps are reversed:

1. A writer invokes fil_space_t::writing_start(), that is, write_or_backup.fetch_add(1) and returns the BACKUP flag as clear.
2. InnoDB_backup::backup_batch_start() atomically sets the BACKUP flag and reads the write count as nonzero. Hence, it will invoke innodb_backup_batch_wait() (a separate function as of bfed32b), to ensure that there is no write conflict.
3. After submitting the writes, the writer will invoke space->writing_stop() to release the write count.
4. The backup batch may copy the page range.
5. Any other writers will be blocked (as in the previous sequence of events) until the backup batch is done.
6. The backup batch clears the BACKUP flag.

I do see a potential problem at step 4, because the writer count was cleared before buf_page_t::write_complete() had been invoked on each page on write completion.

There is one more failure scenario where InnoDB_backup::backup_batch_start() would be invoked after step 3. In this case, the write count would read as 0 and any wait would be skipped.

It seems to me that InnoDB_backup must acquire and hold S latch on each write-fixed buf_page_t that exists in the page range. Furthermore, it seems to me that the "writer count" serves no purpose. We will only need an indication whether a backup batch is in progress. We don’t need a BACKUP flag for that; it suffices to have just fil_space_t::backup_end. When it is 0, we know that the file is not currently being backed up.

[dr-m]

fd8ad0d removes the useless fil_space_t::write_or_backup. Two writers will clearly read space->backup_page_end() while holding buf_pool.mutex.

So actually does buf_flush_list_space(). There is a bug in that function; we’re acquiring buf_pool.mutex another time even though we did not release it, if the page is being backed up. I’ll revise that in the next update.

On the backup side, the revised logic is as follows:

1. InnoDB_backup::backup_batch_start() stores a non-zero value to fil_space_t::backup_end.
2. innodb_backup_batch_wait() will acquire&release S-latch for any write-fixed pages.
3. The backup batch is executed (synchronously).
4. The fil_space_t::backup_end will be cleared to 0.

Each writer will read fil_space_t::backup_end and skip any pages that reside within the currently-backed-up range.

The following bad sequence of events is possible:

1. A writer reads fil_space_t::backup_end as 0.
2. InnoDB_backup::backup_batch_start() stores a non-zero fil_space_t::backup_end and finds no write-fixed pages (yet).
3. The writer write-fixes some blocks and initiates asynchronous writes.
4. Some asynchronous writes may overlap with copy_file() invoked from InnoDB_backup.

What if we acquired a shared latch on each page for the duration of copy_file()? This is not acceptable, because it is prone to deadlocks. For example, if the pages belong to index trees, the locking order is from root onwards, left to right. Besides, secondary index page latches must not be waited for while holding a clustered index page latch. We would get into a deadlock rather easily, for example by holding a latch on page 3 (clustered index root) and simultaneously waiting for a latch on a secondary index page (4 to 63).

Making InnoDB_backup::backup_batch_start() acquire buf_pool.mutex is not a sufficient fix. A part of a solution could be to mark the currently-backed-up blocks as write-fixed and make buf_page_t::flush() skip such blocks. I think that we do need fil_space_t::backup_end in order to block further write batches after the completion of InnoDB_backup::backup_batch_start().

[dr-m]

I believe that a8fcc66 should fix these race conditions. Backup sets a nonzero fil_space_t::backup_end while holding buf_pool.mutex, to synchronize with any page writers. Then, after releasing buf_pool.mutex, it will write-fix any pages in the range that reside in the buffer pool. Freed blocks as well as read-fixed blocks can be skipped. Finally, after the backup batch is completed, the write-fixes will be released. buf_page_t::flush() will yield to these write-fixes.

[Thirunarayanan]

1024 lines is sufficient for shell script commands?

[dr-m]

I think yes. The script should likely be in $PATH anyway. On Linux, extfs and its successors such as ext4 would limit the length of a path component (a file name) to 255 bytes.

[Thirunarayanan]

we're deleting the logs before backup archiving stops.

[dr-m]

We also had executed the following:

    this->ctx= nullptr; /* fini() will delete the object */

Therefore, nothing will be recorded in logs on checkpoint completion:

  /**
     Complete the first checkpoint in a new archive log file.
  */
  void checkpoint_complete() noexcept
  {
    ut_ad(log_sys.latch_have_wr());
    if (ctx)
      logs.emplace_back(log_sys.get_first_lsn() - log_sys.capacity());
  }

However, new log files would have been created. It seems that we should swap some statements to ensure that no garbage log files will be left behind:

      log_sys.latch.wr_unlock();
      fail= log_sys.backup_stop_archiving(thd);
      log_sys.latch.wr_lock();
      delete_logs();
      logs.clear();

[Thirunarayanan]

should the unlink() deferred till copy() succeeds?

[dr-m]

The log file that we are duplicating here is one in the BACKUP SERVER target directory. If the duplication failed, we did not remove the hard link to the active (last) log of the server, and the user started to use the output directory despite the error, they could end up corrupting the log of the server that is being backed up.

[Thirunarayanan]

Do we need to handle bad_alloc() here?

[Thirunarayanan]

Hotter latch acquired for each archive log and datafile. Can we tweak it? Need to check whether it impact the server performance

[dr-m]

We could use a separate latch for protecting queue. However, we must prioritise the copying of the log files that have been collected in logs, because otherwise we could run out of storage space while copying a huge server.

Originally I had a separate mutex in the InnoDB_backup singleton, but in the end, most things ended up being protected by log_sys.latch, most notably the InnoDB_backup::checkpoint_complete().

I checked the size of the critical section between log_sys.latch.wr_lock() and log_sys.latch.wr_unlock() in an optimized build where this code was inlined in innodb_backup_step():

   0x00000000010bb199 <+153>:	mov    0x153b238(%rip),%rcx        # 0x25f63d8 <_ZN12_GLOBAL__N_1L13innodb_backupE+24>
   0x00000000010bb1a0 <+160>:	mov    0x153b249(%rip),%rax        # 0x25f63f0 <_ZN12_GLOBAL__N_1L13innodb_backupE+48>
   0x00000000010bb1a7 <+167>:	mov    0x153b23a(%rip),%rdx        # 0x25f63e8 <_ZN12_GLOBAL__N_1L13innodb_backupE+40>
   0x00000000010bb1ae <+174>:	mov    %rcx,%r10
   0x00000000010bb1b1 <+177>:	sub    0x153b218(%rip),%r10        # 0x25f63d0 <_ZN12_GLOBAL__N_1L13innodb_backupE+16>
   0x00000000010bb1b8 <+184>:	sar    $0x3,%r10
   0x00000000010bb1bc <+188>:	cmp    %rdx,%rax
   0x00000000010bb1bf <+191>:	je     0x10bb290 <_Z18innodb_backup_stepP3THDPK13backup_target12backup_phasePK11backup_sink+400>
   0x00000000010bb1c5 <+197>:	mov    %r12,-0x20(%rbp)
   0x00000000010bb1c9 <+201>:	sub    $0x8,%rax
   0x00000000010bb1cd <+205>:	mov    (%rax),%r12
   0x00000000010bb1d0 <+208>:	mov    0xb8(%rbx),%rcx
   0x00000000010bb1d7 <+215>:	mov    %rax,0x153b212(%rip)        # 0x25f63f0 <_ZN12_GLOBAL__N_1L13innodb_backupE+48>
   0x00000000010bb1de <+222>:	sub    %rdx,%rax
   0x00000000010bb1e1 <+225>:	sar    $0x3,%rax
   0x00000000010bb1e5 <+229>:	test   %r10,%r10
   0x00000000010bb1e8 <+232>:	movl   $0x0,0x2c(%rbx)
   0x00000000010bb1ef <+239>:	cmove  %rax,%r10

The log_sys.wr_unlock() is also duplicated below the following (else if (size)):

   0x00000000010bb290 <+400>:	test   %r10,%r10
   0x00000000010bb293 <+403>:	je     0x10bb390 <_Z18innodb_backup_stepP3THDPK13backup_target12backup_phasePK11backup_sink+656>
   0x00000000010bb299 <+409>:	mov    %r12,-0x20(%rbp)
   0x00000000010bb29d <+413>:	sub    $0x8,%rcx
   0x00000000010bb2a1 <+417>:	mov    (%rcx),%r12
   0x00000000010bb2a4 <+420>:	mov    %r14,-0x10(%rbp)
   0x00000000010bb2a8 <+424>:	mov    %r15,-0x8(%rbp)
   0x00000000010bb2ac <+428>:	mov    %rcx,0x153b125(%rip)        # 0x25f63d8 <_ZN12_GLOBAL__N_1L13innodb_backupE+24>
   0x00000000010bb2b3 <+435>:	dec    %r10
   0x00000000010bb2b6 <+438>:	movl   $0x0,0x2c(%rbx)

The "nothing to do" branch at +656 is duplicating the assignment at +438. All in all, we have 0 or 1 taken conditional branches in the likely code path (there is some work to do). We could avoid that if we used a common data structure for logs and queue.

I think that this should only matter when there are huge amounts of tiny files being backed up.

[dr-m]

Both logs and queue are a std::vector on a 64-bit integer. We could remove the conditional branch inside the critical section of log_sys.latch in InnoDB_backup::step() if we added something that distinguishes the two. An efficient way could be to introduce a separate bit-vector that indicates the type of each 64-bit element. If we simply constructed a std::vector<std::pair<uint64_t,bool>>, I am afraid that the size of an element would be 128 bits. I’ll explore both options.