Code review comment for lp:~sergei.glushchenko/percona-xtrabackup/BT-26901-2.0

Revision history for this message
Alexey Kopytov (akopytov) wrote :

Sergei,

A few of comments:

- what if innodb_doublewrite_file is not specified in my.cnf? As I understand, --print-param will either crash or print "innodb_doublewrite_file = (null)" (on Linux), and then innobackupex will assume we have a doublewrite full with "(null)" as its name.

- why don't we write stripped innodb_doublewrite_file (i.e. with path removed) to backup-my.cnf? Less code in innobackupex to strip it on --copy-back

- why don't we make 2.0 and 2.1 consistent and just modify trx_sys_sys_space(id) to include the #ifdef and avoid having a separate macro? Less code changes in 2.0, less conflicts when merging 2.0->2.1 in the future.

- please use one of MYSQLD_VARDIR/MYSQLD_DATADIR/MYSQLD_TMPDIR instead of hardcoded paths as in "${TEST_BASEDIR}/var1..."

- s/.idb/.ibd/

review: Needs Fixing

« Back to merge proposal