Skip to content

Commit

Permalink
MDEV-28994 Backup of memory-mapped log is corrupted
Browse files Browse the repository at this point in the history
An interface to use memory-mapped I/O on the InnoDB redo log that
is stored in persistent memory was introduced
in commit 685d958 (MDEV-14425).

log_t::attach(): In mariadb-backup --backup, never attempt to
use memory-mapped I/O for reading the log file of the server.

xtrabackup_copy_logfile(): Assert !log_sys.is_pmem() and remove
the code to deal with a memory-mapped log.

This fixes a race condition scenario of the following type:
1. Backup parsed a mini-transaction from the memory-mapped buffer.
This took some time.
2. Meanwhile, the server might have overwritten this portion
of the circular log_sys.buf.
3. Backup copied the data to the output file while or after
the server had overwritten this portion of the file.
4. Backup failed to notice that a log overrun occurred.

The symptom of this was that a mariadb-backup --prepare of the
log failed. In the analyzed case, the error message was:
[ERROR] InnoDB: Missing FILE_CHECKPOINT(...)

This will also make it possible to run mariadb-backup --backup
under "rr replay".
  • Loading branch information
dr-m committed Jul 1, 2022
1 parent 3c2a5ad commit 155019b
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 96 deletions.
97 changes: 2 additions & 95 deletions extra/mariabackup/xtrabackup.cc
Expand Up @@ -2992,14 +2992,8 @@ static bool xtrabackup_copy_logfile()
const size_t sequence_offset{log_sys.is_encrypted() ? 8U + 5U : 5U};
const size_t block_size_1{log_sys.get_block_size() - 1};

#ifdef HAVE_PMEM
if (log_sys.is_pmem())
{
recv_sys.offset= size_t(log_sys.calc_lsn_offset(recv_sys.lsn));
recv_sys.len= size_t(log_sys.file_size);
}
else
#endif
ut_ad(!log_sys.is_pmem());

{
recv_sys.offset= size_t(recv_sys.lsn - log_sys.get_first_lsn()) &
block_size_1;
Expand All @@ -3011,87 +3005,6 @@ static bool xtrabackup_copy_logfile()
recv_sys_t::parse_mtr_result r;
size_t start_offset{recv_sys.offset};

#ifdef HAVE_PMEM
if (log_sys.is_pmem())
{
if ((ut_d(r=) recv_sys.parse_pmem(STORE_NO)) != recv_sys_t::OK)
{
ut_ad(r == recv_sys_t::GOT_EOF);
goto retry;
}

retry_count= 0;

do
{
const byte seq{log_sys.get_sequence_bit(recv_sys.lsn -
sequence_offset)};
ut_ad(recv_sys.offset >= log_sys.START_OFFSET);
ut_ad(recv_sys.offset < recv_sys.len);
ut_ad(log_sys.buf[recv_sys.offset
>= log_sys.START_OFFSET + sequence_offset
? recv_sys.offset - sequence_offset
: recv_sys.len - sequence_offset +
recv_sys.offset - log_sys.START_OFFSET] ==
seq);
static const byte seq_1{1};
if (UNIV_UNLIKELY(start_offset > recv_sys.offset))
{
const ssize_t so(recv_sys.offset - (log_sys.START_OFFSET +
sequence_offset));
if (so <= 0)
{
if (ds_write(dst_log_file, log_sys.buf + start_offset,
recv_sys.len - start_offset + so) ||
ds_write(dst_log_file, &seq_1, 1))
goto write_error;
if (so < -1 &&
ds_write(dst_log_file, log_sys.buf + recv_sys.len + (1 + so),
-(1 + so)))
goto write_error;
if (ds_write(dst_log_file, log_sys.buf + log_sys.START_OFFSET,
recv_sys.offset - log_sys.START_OFFSET))
goto write_error;
}
else
{
if (ds_write(dst_log_file, log_sys.buf + start_offset,
recv_sys.len - start_offset))
goto write_error;
if (ds_write(dst_log_file, log_sys.buf + log_sys.START_OFFSET, so))
goto write_error;
if (ds_write(dst_log_file, &seq_1, 1))
goto write_error;
if (so > 1 &&
ds_write(dst_log_file, log_sys.buf + recv_sys.offset -
(so - 1), so - 1))
goto write_error;
}
}
else if (seq == 1)
{
if (ds_write(dst_log_file, log_sys.buf + start_offset,
recv_sys.offset - start_offset))
goto write_error;
}
else if (ds_write(dst_log_file, log_sys.buf + start_offset,
recv_sys.offset - start_offset - sequence_offset) ||
ds_write(dst_log_file, &seq_1, 1) ||
ds_write(dst_log_file, log_sys.buf +
recv_sys.offset - sequence_offset + 1,
sequence_offset - 1))
goto write_error;

start_offset= recv_sys.offset;
}
while ((ut_d(r=)recv_sys.parse_pmem(STORE_NO)) == recv_sys_t::OK);

ut_ad(r == recv_sys_t::GOT_EOF);
pthread_cond_broadcast(&scanned_lsn_cond);
break;
}
else
#endif
{
{
auto source_offset=
Expand Down Expand Up @@ -3135,9 +3048,6 @@ static bool xtrabackup_copy_logfile()
if (ds_write(dst_log_file, log_sys.buf + start_offset,
recv_sys.offset - start_offset))
{
#ifdef HAVE_PMEM
write_error:
#endif
msg("Error: write to ib_logfile0 failed");
return true;
}
Expand Down Expand Up @@ -3167,9 +3077,6 @@ static bool xtrabackup_copy_logfile()
else
{
recv_sys.len= recv_sys.offset & ~block_size_1;
#ifdef HAVE_PMEM
retry:
#endif
if (retry_count == 100)
break;

Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/log/log0log.cc
Expand Up @@ -176,7 +176,7 @@ void log_t::attach(log_file_t file, os_offset_t size)
#ifdef HAVE_PMEM
ut_ad(!buf);
ut_ad(!flush_buf);
if (size && !(size_t(size) & 4095))
if (size && !(size_t(size) & 4095) && srv_operation != SRV_OPERATION_BACKUP)
{
void *ptr=
my_mmap(0, size_t(size),
Expand Down

0 comments on commit 155019b

Please sign in to comment.