Skip to content

Commit

Permalink
MDEV-15270 Mariabackup should not try to use doublewrite buffer
Browse files Browse the repository at this point in the history
When Mariabackup gets a bad read of the first page of the system
tablespace file, it would inappropriately try to apply the doublewrite
buffer and write changes back to the data file (to the source file)!
This is very wrong and must be prevented.

The correct action would be to retry reading the system tablespace
as well as any other files whose first page was read incorrectly.
Fixing this was not attempted.

xb_load_tablespaces(): Shorten a bogus message to be more relevant.
The message can be displayed by --backup or --prepare.

xtrabackup_backup_func(), os_file_write_func(): Add a missing space
to a message.

Datafile::restore_from_doublewrite(): Do not even attempt the
operation in Mariabackup.

recv_init_crash_recovery_spaces(): Do not attempt to restore the
doublewrite buffer in Mariabackup (--prepare or --export), because
all pages should have been copied correctly in --backup already,
and because --backup should ignore the doublewrite buffer.

SysTablespace::read_lsn_and_check_flags(): Do not attempt to initialize
the doublewrite buffer in Mariabackup.

innodb_make_page_dirty(): Correct the bounds check.

Datafile::read_first_page(): Correct the name of the parameter.
  • Loading branch information
dr-m committed Feb 12, 2018
1 parent 33f70c4 commit 00f0c03
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 23 deletions.
16 changes: 2 additions & 14 deletions extra/mariabackup/xtrabackup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2908,19 +2908,7 @@ xb_load_tablespaces()
&flush_lsn);

if (err != DB_SUCCESS) {
msg("mariabackup: Could not open or create data files.\n"
"mariabackup: If you tried to add new data files, and it "
"failed here,\n"
"mariabackup: you should now edit innodb_data_file_path in "
"my.cnf back\n"
"mariabackup: to what it was, and remove the new ibdata "
"files InnoDB created\n"
"mariabackup: in this failed attempt. InnoDB only wrote "
"those files full of\n"
"mariabackup: zeros, but did not yet use them in any way. "
"But be careful: do not\n"
"mariabackup: remove old data files which contain your "
"precious data!\n");
msg("mariabackup: Could not open data files.\n");
return(err);
}

Expand Down Expand Up @@ -3859,7 +3847,7 @@ xtrabackup_backup_func()
err = xb_load_tablespaces();
if (err != DB_SUCCESS) {
msg("mariabackup: error: xb_load_tablespaces() failed with"
"error code %u\n", err);
" error code %u\n", err);
goto fail;
}

Expand Down
1 change: 0 additions & 1 deletion mysql-test/suite/mariabackup/huge_lsn.test
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ EOF
--remove_files_wildcard $MYSQLD_DATADIR ib_logfile*

--source include/start_mysqld.inc
let SEARCH_RANGE= -50000;
let SEARCH_FILE= $MYSQLTEST_VARDIR/log/mysqld.1.err;
--let SEARCH_PATTERN= InnoDB: New log files created, LSN=175964\d{8}
--source include/search_pattern_in_file.inc
Expand Down
6 changes: 5 additions & 1 deletion storage/innobase/fsp/fsp0file.cc
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*****************************************************************************
Copyright (c) 2013, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2017, MariaDB Corporation.
Copyright (c) 2017, 2018, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -770,6 +770,10 @@ the double write buffer.
bool
Datafile::restore_from_doublewrite()
{
if (srv_operation != SRV_OPERATION_NORMAL) {
return true;
}

/* Find if double write buffer contains page_no of given space id. */
const byte* page = recv_sys->dblwr.find_page(m_space_id, 0);
const page_id_t page_id(m_space_id, 0);
Expand Down
5 changes: 3 additions & 2 deletions storage/innobase/fsp/fsp0sysspace.cc
Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,9 @@ SysTablespace::read_lsn_and_check_flags(lsn_t* flushed_lsn)

ut_a(it->order() == 0);


buf_dblwr_init_or_load_pages(it->handle(), it->filepath());
if (srv_operation == SRV_OPERATION_NORMAL) {
buf_dblwr_init_or_load_pages(it->handle(), it->filepath());
}

/* Check the contents of the first page of the
first datafile. */
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/handler/ha_innodb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18571,7 +18571,7 @@ innodb_make_page_dirty(
return;
}

if (srv_saved_page_number_debug > space->size) {
if (srv_saved_page_number_debug >= space->size) {
fil_space_release(space);
return;
}
Expand Down
3 changes: 2 additions & 1 deletion storage/innobase/include/fsp0file.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*****************************************************************************
Copyright (c) 2013, 2016, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2018, MariaDB Corporation.
This program is free software; you can redistribute it and/or modify it under
the terms of the GNU General Public License as published by the Free Software
Expand Down Expand Up @@ -363,7 +364,7 @@ class Datafile {
@param[in] read_only_mode if true, then readonly mode checks
are enforced.
@return DB_SUCCESS or DB_IO_ERROR if page cannot be read */
dberr_t read_first_page(bool read_first_page)
dberr_t read_first_page(bool read_only_mode)
MY_ATTRIBUTE((warn_unused_result));

/** Free the first page from memory when it is no longer needed. */
Expand Down
4 changes: 3 additions & 1 deletion storage/innobase/log/log0recv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3147,7 +3147,9 @@ recv_init_crash_recovery_spaces()
<< "', but there were no modifications either.";
}

buf_dblwr_process();
if (srv_operation == SRV_OPERATION_NORMAL) {
buf_dblwr_process();
}

if (srv_force_recovery < SRV_FORCE_NO_LOG_REDO) {
/* Spawn the background thread to flush dirty pages
Expand Down
4 changes: 2 additions & 2 deletions storage/innobase/os/os0file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Copyright (c) 1995, 2017, Oracle and/or its affiliates. All Rights Reserved.
Copyright (c) 2009, Percona Inc.
Copyright (c) 2013, 2017, MariaDB Corporation.
Copyright (c) 2013, 2018, MariaDB Corporation.
Portions of this file contain modifications contributed and copyrighted
by Percona Inc.. Those modifications are
Expand Down Expand Up @@ -4994,7 +4994,7 @@ os_file_write_func(
if ((ulint) n_bytes != n && !os_has_said_disk_full) {

ib::error()
<< "Write to file " << name << "failed at offset "
<< "Write to file " << name << " failed at offset "
<< offset << ", " << n
<< " bytes should have been written,"
" only " << n_bytes << " were written."
Expand Down

0 comments on commit 00f0c03

Please sign in to comment.