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

Proposed by George Ormond Lorch III on 2013-07-12
Status: Work in progress
Proposed branch: lp:~gl-az/percona-xtrabackup/2.1-valgrind
Merge into: lp: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:~gl-az/percona-xtrabackup/2.1-valgrind
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2013-07-12 Needs Fixing on 2013-07-27
Review via email: mp+174494@code.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

To post a comment you must log in.
Stewart Smith (stewart) wrote : 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

Alexey Kopytov (akopytov) wrote : 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
Alexey Kopytov (akopytov) wrote :

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
Alexey Kopytov (akopytov) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'utils/build.sh'
2--- utils/build.sh 2013-06-05 08:39:54 +0000
3+++ utils/build.sh 2013-07-12 20:00:37 +0000
4@@ -35,8 +35,20 @@
5 ;;
6 esac
7
8+if [ -n "$WITH_VALGRIND" ]
9+then
10+ export CFLAGS="$CFLAGS -g -O0"
11+ export CXXFLAGS="$CXXFLAGS -g -O0"
12+ extra_config_51=
13+ extra_config_55plus="-DCMAKE_BUILD_TYPE=RelWithDebInfo -DWITH_VALGRIND=ON"
14+elif [ -n "$RELWITHDEBINFO" ]
15+then
16+ export CFLAGS="$CFLAGS -g -O0"
17+ export CXXFLAGS="$CXXFLAGS -g -O0"
18+ extra_config_51=
19+ extra_config_55plus="-DCMAKE_BUILD_TYPE=RelWithDebInfo"
20 # Percona Server 5.5 does not build with -Werror, so ignore DEBUG for now
21-if [ -n "$DEBUG" -a "$type" != "xtradb55" -a "$type" != "xtradb51" ]
22+elif [ -n "$DEBUG" -a "$type" != "xtradb55" -a "$type" != "xtradb51" ]
23 then
24 # InnoDB extra debug flags
25 innodb_extra_debug="-DUNIV_DEBUG -DUNIV_MEM_DEBUG \
26@@ -219,14 +231,17 @@
27 server_tarball=mysql-$mysql_version.tar.gz
28 innodb_dir=$server_dir/storage/$innodb_name
29
30- echo "Downloading sources"
31- auto_download $server_tarball
32-
33- test -d $server_dir && rm -r $server_dir
34-
35- echo "Preparing sources"
36- unpack_and_patch $server_tarball $server_patch
37- mv $top_dir/mysql-$mysql_version $server_dir
38+ if [ -z "$MAKE_ONLY" ]
39+ then
40+ echo "Downloading sources"
41+ auto_download $server_tarball
42+
43+ test -d $server_dir && rm -r $server_dir
44+
45+ echo "Preparing sources"
46+ unpack_and_patch $server_tarball $server_patch
47+ mv $top_dir/mysql-$mysql_version $server_dir
48+ fi
49
50 build_server $type
51
52@@ -302,35 +317,43 @@
53 fi
54
55
56- echo "Downloading sources"
57+ if [ -z "$MAKE_ONLY" ]
58+ then
59+ echo "Downloading sources"
60
61- # Get Percona Server
62- if [ -d $branch_dir ]
63- then
64- rm -rf $branch_dir
65- fi
66-
67- if [ ! -f Percona-Server-XtraBackup-$PS_51_VERSION.tar.gz ]
68- then
69- make ps51source
70- fi
71-
72- rm -rf $branch_dir
73- tar xfz Percona-Server-XtraBackup-$PS_51_VERSION.tar.gz
74- cd $branch_dir
75- $MAKE_CMD main
76- cd $top_dir
77- rm -rf $server_dir
78- ln -s $branch_dir/Percona-Server $server_dir
79-
80- # Patch Percona Server
81- cd $server_dir
82- patch -p1 < $top_dir/patches/xtradb51.patch
83-
84- build_server $type
85-
86- build_xtrabackup
87-
88+ # Get Percona Server
89+ if [ -d $branch_dir ]
90+ then
91+ rm -rf $branch_dir
92+ fi
93+
94+ if [ ! -f Percona-Server-XtraBackup-$PS_51_VERSION.tar.gz ]
95+ then
96+ make ps51source
97+ fi
98+
99+ rm -rf $branch_dir
100+ tar xfz Percona-Server-XtraBackup-$PS_51_VERSION.tar.gz
101+ cd $branch_dir
102+ $MAKE_CMD main
103+ cd $top_dir
104+ rm -rf $server_dir
105+ ln -s $branch_dir/Percona-Server $server_dir
106+
107+ # Patch Percona Server
108+ cd $server_dir
109+ patch -p1 < $top_dir/patches/xtradb51.patch
110+
111+ build_server $type
112+
113+ build_xtrabackup
114+ else
115+ cd $server_dir
116+
117+ build_server $type
118+
119+ build_xtrabackup
120+ fi
121 ;;
122 "xtradb55" )
123 server_dir=$top_dir/Percona-Server-5.5
124@@ -350,37 +373,44 @@
125 configure_cmd="LIBS=-lrt $configure_cmd"
126 fi
127
128-
129- echo "Downloading sources"
130+ if [ -z "$MAKE_ONLY" ]
131+ then
132+ echo "Downloading sources"
133
134- # Get Percona Server
135- if [ -d $branch_dir ]
136- then
137- rm -rf $branch_dir
138- fi
139-
140- if [ ! -f Percona-Server-XtraBackup-$PS_55_VERSION.tar.gz ]
141- then
142- make ps55source
143- fi
144-
145- rm -rf $branch_dir
146- tar xfz Percona-Server-XtraBackup-$PS_55_VERSION.tar.gz
147- cd $branch_dir
148-
149- $MAKE_CMD PERCONA_SERVER=Percona-Server-5.5 main
150- cd $top_dir
151- rm -rf $server_dir
152- ln -s $branch_dir/Percona-Server $server_dir
153-
154- # Patch Percona Server
155- cd $server_dir
156- patch -p1 < $top_dir/patches/xtradb55.patch
157-
158- build_server $type
159-
160- build_xtrabackup
161-
162+ # Get Percona Server
163+ if [ -d $branch_dir ]
164+ then
165+ rm -rf $branch_dir
166+ fi
167+
168+ if [ ! -f Percona-Server-XtraBackup-$PS_55_VERSION.tar.gz ]
169+ then
170+ make ps55source
171+ fi
172+
173+ rm -rf $branch_dir
174+ tar xfz Percona-Server-XtraBackup-$PS_55_VERSION.tar.gz
175+ cd $branch_dir
176+
177+ $MAKE_CMD PERCONA_SERVER=Percona-Server-5.5 main
178+ cd $top_dir
179+ rm -rf $server_dir
180+ ln -s $branch_dir/Percona-Server $server_dir
181+
182+ # Patch Percona Server
183+ cd $server_dir
184+ patch -p1 < $top_dir/patches/xtradb55.patch
185+
186+ build_server $type
187+
188+ build_xtrabackup
189+ else
190+ cd $server_dir
191+
192+ build_server $type
193+
194+ build_xtrabackup
195+ fi
196 ;;
197 *)
198 usage

Subscribers

People subscribed via source and target branches