Merge lp:~percona-toolkit-dev/percona-toolkit/fix-937793 into lp:percona-toolkit/2.0

[Brian Fraser]

One somewhat weird thing you might notie is using . instead of source in the test file.
Apparently source is a bashism? It didn't work at all in my NetBSD box.
Besides that, I think it has everything in there.

[Daniel Nichter (daniel-nichter)]

There are three problems. First and foremost, it doesn't fix the bug:

       Log File Size | 2 * 1,46G = 2,9G

Did you test it manually with innodb_log_file_size=1500M?

Second, please name tests, e.g. (last arg):

is \
   "$(shorten 10485760 1)" \
   "10.0M" \
   "10485760 = 10.0M"

The Bash test stuff currently allows you not to name tests, but I should really remove that "feature".

Fourth, echo "$num $prec $div" | awk ... is a useless use of echo; see awk -v var=val.

Finally, I haven't documented this yet, but use:

report_formatting.t@ -> ../../../util/test-bash-functions

I.e. just symlink <bash test>.t in t/lib/bash/ to ../../../util/test-bash-functions and this script will do the proper magic so perl <bash test>.t runs <bash test>.sh.

review: Needs Fixing

[lp:~percona-toolkit-dev/percona-toolkit/fix-937793]

199. By Brian Fraser <email address hidden> on 2012-03-07

Fixes as per Daniel's review.

The previous commit made the change to shorten() opt-in, but never
changed the code in pt-mysql-summary to actually enable the new
behavior.

[Daniel Nichter (daniel-nichter)]

This branch was never actually merged, and after further though, I'm fixing this problem while also fixing bug 993436.

review: Disapprove

Unmerged revisions

199. By Brian Fraser <email address hidden> on 2012-03-07

Fixes as per Daniel's review.

The previous commit made the change to shorten() opt-in, but never
changed the code in pt-mysql-summary to actually enable the new
behavior.

198. By Brian Fraser <email address hidden> on 2012-03-07

Backported fix for https://bugs.launchpad.net/percona-toolkit/+bug/937793

This commit adds a report_formatting bash lib. Currently it only has
one function, shorten(), but in future versions of the toolkit it
will contain shared functions between pt-summary and pt-mysql-summary.

=== modified file 'bin/pt-mysql-summary'
--- bin/pt-mysql-summary	2012-02-03 23:25:29 +0000
+++ bin/pt-mysql-summary	2012-03-07 19:46:18 +0000
@@ -22,18 +22,19 @@
 # See https://launchpad.net/percona-toolkit for more information.
 # ###########################################################################
 
+
 TMPDIR=""
 
 mk_tmpdir() {
-   local dir=${1:-""}
+   local dir="${1:-""}"
 
    if [ -n "$dir" ]; then
       if [ ! -d "$dir" ]; then
-         mkdir $dir || die "Cannot make tmpdir $dir"
+         mkdir "$dir" || die "Cannot make tmpdir $dir"
       fi
       TMPDIR="$dir"
    else
-      local tool=`basename $0`
+      local tool="${0##*/}"
       local pid="$$"
       TMPDIR=`mktemp -d /tmp/${tool}.${pid}.XXXXX` \
          || die "Cannot make secure tmpdir"
@@ -42,7 +43,7 @@
 
 rm_tmpdir() {
    if [ -n "$TMPDIR" ] && [ -d "$TMPDIR" ]; then
-      rm -rf $TMPDIR
+      rm -rf "$TMPDIR"
    fi
    TMPDIR=""
 }
@@ -51,6 +52,45 @@
 # End tmpdir package
 # ###########################################################################
 
+# ###########################################################################
+# report_formatting package
+# This package is a copy without comments from the original.  The original
+# with comments and its test file can be found in the Bazaar repository at,
+#   lib/bash/report_formatting.sh
+#   t/lib/bash/report_formatting.sh
+# See https://launchpad.net/percona-toolkit for more information.
+# ###########################################################################
+
+
+shorten() {
+   local num="$1"
+   local prec="${2:-2}"
+   local div="${3:-1024}"
+
+   echo "$num" | awk -v prec="$prec" -v div="$div" '
+   {
+      size = 4;
+      val  = $1;
+
+      unit = val >= 1099511627776 ? "T" : val >= 1073741824 ? "G" : val >= 1048576 ? "M" : val >= 1024 ? "k" : "";
+
+      while ( int(val) && !(val % 1024) ) {
+         val /= 1024;
+      }
+
+      while ( val > 1000 ) {
+         val /= div;
+      }
+
+      printf "%.*f%s", prec, val, unit;
+   }
+   '
+}
+
+# ###########################################################################
+# End report_formatting package
+# ###########################################################################
+
 # ########################################################################
 # Some global setup is necessary for cross-platform compatibility, even
 # when sourcing this script for testing purposes.
@@ -132,24 +172,6 @@
    printf "%20s | %s\n" "$1" "$2"
 }
 
-# Converts a value to units of power of 2.  Optional precision is $2.
-shorten() {
-   unit=k
-   size=1024
-   if [ $1 -ge 1099511627776 ] ; then
-      size=1099511627776
-      unit=T
-   elif [ $1 -ge 1073741824 ] ; then
-      size=1073741824
-      unit=G
-   elif [ $1 -ge 1048576 ] ; then
-      size=1048576
-      unit=M
-   fi
-   result=$(echo "$1 $size ${2:-0}" | $AP_AWK '{printf "%." $3 "f", $1 / $2}')
-   echo "${result}${unit}"
-}
-
 # Collapse a file into an aggregated list; file must be created with 'sort |
 # uniq -c'.  This function is copy-pasted from 'summary' so see there for full
 # docs and tests.
@@ -576,7 +598,7 @@
 # Summarizes various things about InnoDB status that are not easy to see by eye.
 format_innodb_status () {
    local file=$1
-   name_val "Checkpoint Age"       $(shorten $(find_checkpoint_age "${file}"))
+   name_val "Checkpoint Age"       $(shorten $(find_checkpoint_age "${file}") 0)
    name_val "InnoDB Queue"        "$(awk '/queries inside/{print}' "${file}")"
    name_val "Oldest Transaction"  "$(find_max_trx_time "${file}") Seconds";
    name_val "History List Len"     $(awk '/History list length/{print $4}' "${file}")
@@ -1202,7 +1224,7 @@
       lg_size="$(get_var innodb_log_file_size)"
       lg_fils="$(get_var innodb_log_files_in_group)"
       lg_totl="$((${lg_size} * ${lg_fils}))"
-      name_val "Log File Size" "${lg_fils} * $(shorten ${lg_size}) = $(shorten ${lg_totl} 1)"
+      name_val "Log File Size" "${lg_fils} * $(shorten ${lg_size} 1 1000) = $(shorten ${lg_totl} 1 1000)"
       name_val "Log Buffer Size"     $(shorten $(get_var innodb_log_buffer_size))
       name_val "Flush Method"        $(get_var innodb_flush_method)
       name_val "Flush Log At Commit" $(get_var innodb_flush_log_at_trx_commit)

=== added file 'lib/bash/report_formatting.sh'
--- lib/bash/report_formatting.sh	1970-01-01 00:00:00 +0000
+++ lib/bash/report_formatting.sh	2012-03-07 19:46:18 +0000
@@ -0,0 +1,54 @@
+# This program is copyright 2011 Percona Inc.
+# Feedback and improvements are welcome.
+#
+# THIS PROGRAM IS PROVIDED "AS IS" AND WITHOUT ANY EXPRESS OR IMPLIED
+# WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF
+# MERCHANTIBILITY AND FITNESS FOR A PARTICULAR PURPOSE.
+#
+# 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
+# Foundation, version 2; OR the Perl Artistic License.  On UNIX and similar
+# systems, you can issue `man perlgpl' or `man perlartistic' to read these
+# licenses.
+#
+# You should have received a copy of the GNU General Public License along with
+# this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+# Place, Suite 330, Boston, MA  02111-1307  USA.
+# ###########################################################################
+# report_formatting package
+# ###########################################################################
+
+# Package: report_formatting
+# (partially backported from 2.1)
+
+# Sub: shorten
+#   Shorten a value in bytes to another representation.
+# 
+shorten() {
+   local num="$1"
+   local prec="${2:-2}"
+   local div="${3:-1024}"
+
+   echo "$num" | awk -v prec="$prec" -v div="$div" '
+   {
+      size = 4;
+      val  = $1;
+
+      unit = val >= 1099511627776 ? "T" : val >= 1073741824 ? "G" : val >= 1048576 ? "M" : val >= 1024 ? "k" : "";
+
+      while ( int(val) && !(val % 1024) ) {
+         val /= 1024;
+      }
+
+      while ( val > 1000 ) {
+         val /= div;
+      }
+
+      printf "%.*f%s", prec, val, unit;
+   }
+   '
+}
+
+# ###########################################################################
+# End report_formatting package
+# ###########################################################################

=== added file 't/lib/bash/report_formatting.sh'
--- t/lib/bash/report_formatting.sh	1970-01-01 00:00:00 +0000
+++ t/lib/bash/report_formatting.sh	2012-03-07 19:46:18 +0000
@@ -0,0 +1,64 @@
+#!/usr/bin/env bash
+
+plan 11
+
+. "$LIB_DIR/report_formatting.sh"
+
+is                         \
+   "$(shorten 10485760 1)" \
+   "10.0M"                 \
+   "10485760, 1 precision, default divisor => 10.0M"
+
+is                           \
+   "$(shorten 3145728000 1)" \
+   "2.9G"                 \
+   "10485760, 1 precision, default divisor => 2.9G"
+
+is                                \
+   "$(shorten 3145728000 1 1000)" \
+   "3.0G"                         \
+   "Opt-in to the 2.1 behavior works"
+
+is                  \
+   "$(shorten 0 0)" \
+   "0"                 \
+   "0, 0 precision, default divisor => 0"
+
+is                           \
+   "$(shorten 1572864000 1)" \
+   "1.5G"                 \
+   "1572864000, 1 precision, default divisor => 1.5G"
+
+is                           \
+   "$(shorten 1572864000 1 1000)" \
+   "1.5G"                 \
+   "1572864000, 1 precision, divisor 1000 => 1.5G"
+
+is                           \
+   "$(shorten 364 0)" \
+   "364"                 \
+   "364, 0 precision, default divisor => 364"
+
+is                           \
+   "$(shorten 364 1)" \
+   "364.0"                 \
+   "364, 1 precision, default divisor => 364"
+
+is                           \
+   "$(shorten 649216 1)" \
+   "634.0k"                 \
+   "649216, 1 precision, default divisor => 634.0k"
+
+is                           \
+   "$(shorten 6492100000006 1)" \
+   "5.9T"                 \
+   "6492100000006, 1 precision, default divisor => 5.9T"
+
+is                           \
+   "$(shorten 6492100000006 1 1000)" \
+   "6.5T"                 \
+   "6492100000006, 1 precision, divisor 1000 => 6.5T"
+
+# ###########################################################################
+# Done
+# ###########################################################################

=== added symlink 't/lib/bash/report_formatting.t'
=== target is u'../../../util/test-bash-functions'
=== modified file 't/pt-mysql-summary/format_innodb_status.sh'
--- t/pt-mysql-summary/format_innodb_status.sh	2012-02-02 17:03:48 +0000
+++ t/pt-mysql-summary/format_innodb_status.sh	2012-03-07 19:46:18 +0000
@@ -98,7 +98,7 @@
 TEST_NAME="innodb-status.003.txt"
 # ############################################################################
 cat <<'EOF' > $TMPDIR/expected
-      Checkpoint Age | 0k
+      Checkpoint Age | 0
         InnoDB Queue | 0 queries inside InnoDB, 0 queries in queue
   Oldest Transaction | 35 Seconds
     History List Len | 11