Percona XtraBackup moved to https://jira.percona.com/projects/PXB

Merge lp://qastaging/~gl-az/percona-xtrabackup/2.1-valgrind into lp://qastaging/percona-xtrabackup/2.1

2.1-valgrind
Merge into 2.1

Proposed by George Ormond Lorch III on 2013-07-12

Status:	Work in progress
Proposed branch:	lp://qastaging/~gl-az/percona-xtrabackup/2.1-valgrind
Merge into:	lp://qastaging/percona-xtrabackup/2.1
Diff against target:	198 lines (+97/-67) 1 file modified utils/build.sh (+97/-67)
To merge this branch:	bzr merge lp://qastaging/~gl-az/percona-xtrabackup/2.1-valgrind
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Alexey Kopytov (community)		2013-07-12	Needs Fixing on 2013-07-27
Review via email: mp+174494@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2013-07-12.

Description of the change

Added new environment variables to control build:
  MAKE_ONLY - Set this to 1 to only re 'make' the server and xtrabackup. Nothing will be downloaded, unpacked or patched. This is not advised if any compiler or other flags are being changed from previous builds.
  RELWITHDEBINFO - Set this to 1 to eliminate optimizations by changing C/CXXFLAGS to -g -O0 and pass CMAKE_BUILD_TYPE=RelWithDebInfo to PS or MySQL 5.5+ cmake.
  WITH_VALGRIND - Set this to 1 to perform Valgrind compatible build. Eliminates optimizations just like RELWITHDEBINFO and also passes WITH_VALGRIND=ON to PS or MySQL 5.5+ cmake.
Added innobackupex option --use-valgrind which will prefix backup and apply-log calls to xtrabackup with "valgrind".
The valgrind binary must be within path and valgrind options may be set by either using the ~/.valgrindrc file or setting the environment variable VALGRIND_OPTS.

----

Rebased to trunk and fixed typo.

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.1-param/378

Revision history for this message

Stewart Smith (stewart) wrote on 2013-06-07: Posted in a previous version of this proposal

This will probably conflict with https://code.launchpad.net/~stewart/percona-xtrabackup/source-dist-refactor/+merge/167709 as I've moved things about a bit with regards to downloading/unpacking things and it may have the same end result as MAKE_ONLY

Revision history for this message

Alexey Kopytov (akopytov) wrote on 2013-07-08: Posted in a previous version of this proposal

This needs a bug/BP and rebasing on trunk (as source-dist-refactor has been merged to 2.1).

review: Needs Fixing

Revision history for this message

George Ormond Lorch III (gl-az) wrote on 2013-07-12: Posted in a previous version of this proposal

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.1-param/377/

Revision history for this message

Alexey Kopytov (akopytov) wrote on 2013-07-27:

Hm, the comments mention the new --use-valgrind option in innobackupex, but I don't see it in the code?

Which is good, because I would prefer this to be implemented in a different way. Instead of adding a new option to innobackupex we can modify the test suite to pass:

--ibbackup="valgrind xtrabackup*"

to innobackupex. And likewise, call "valgrind xtrabackup*", if the xtrabackup binary is called directly by tests.

This is a bit tricky to implement due to Bash word splitting. But the following proof-of-concept patch works for me:

=== modified file 'test/inc/common.sh'
--- test/inc/common.sh 2013-07-25 15:04:49 +0000
+++ test/inc/common.sh 2013-07-27 10:17:31 +0000
@@ -2,7 +2,7 @@ set -eu

function innobackupex()
{
- run_cmd $IB_BIN $IB_ARGS $*
+ run_cmd $IB_BIN "${IB_ARGS[@]}" $*
}

function xtrabackup()
@@ -239,7 +239,7 @@ function switch_server()
MYSQLD_ARGS="$MYSQLD_ARGS --user=root"
fi

- IB_ARGS="--defaults-file=$MYSQLD_VARDIR/my.cnf --ibbackup=$XB_BIN"
+ IB_ARGS=("--defaults-file=$MYSQLD_VARDIR/my.cnf" "--ibbackup=$XB_BIN")
XB_ARGS="--defaults-file=$MYSQLD_VARDIR/my.cnf"

# Some aliases for compatibility, as tests use the following names

=== modified file 'test/run.sh'
--- test/run.sh 2013-07-25 15:04:49 +0000
+++ test/run.sh 2013-07-27 10:17:40 +0000
@@ -435,7 +435,7 @@ function get_version_info()
  vlog "Cannot find '$XB_BIN' in PATH"
  return 1
     fi
- XB_BIN="$XB_PATH"
+ XB_BIN="valgrind $XB_PATH"

# Set the correct binary for innobackupex
IB_BIN="`which innobackupex`"

The downside is that IB_ARGS is now an array (this is required to prevent "--ibbackup=valgrind xtrabackup" as 2 different words), so all tests referencing IB_ARGS also have to be modified. But I like it more than another option to innobackupex.

review: Needs Fixing

Revision history for this message

Alexey Kopytov (akopytov) wrote on 2013-07-27:

Having a Blueprint for this work would also be nice.

Unmerged revisions

629. By George Ormond Lorch III on 2013-07-12: Added new environment variables to control build:
  MAKE_ONLY - Set this to 1 to only re 'make' the server and xtrabackup. Nothing will be downloaded, unpacked or patched. This is not advised if any compiler or other flags are being changed from previous builds.
  RELWITHDEBINFO - Set this to 1 to eliminate optimizations by changing C/CXXFLAGS to -g -O0 and pass CMAKE_BUILD_TYPE=RelWithDebInfo to PS or MySQL 5.5+ cmake.
  WITH_VALGRIND - Set this to 1 to perform Valgrind compatible build. Eliminates optimizations just like RELWITHDEBINFO and also passes WITH_VALGRIND=ON to PS or MySQL 5.5+ cmake.
Added innobackupex option --use-valgrind which will prefix backup and apply-log calls to xtrabackup with "valgrind".
The valgrind binary must be within path and valgrind options may be set by either using the ~/.valgrindrc file or setting the environment variable VALGRIND_OPTS.