Merge ~rodsmith/plainbox-provider-checkbox:fix-disk-cpu-load into plainbox-provider-checkbox:master

Proposed by Rod Smith
Status: Merged
Approved by: Jeff Lane 
Approved revision: 507507ebc8bdbfd1ec4c1a62b5e393cb3f08e072
Merged at revision: 15ec3e8bf85e0aac21729f41a5fa4c993c06270a
Proposed branch: ~rodsmith/plainbox-provider-checkbox:fix-disk-cpu-load
Merge into: plainbox-provider-checkbox:master
Diff against target: 68 lines (+19/-2)
1 file modified
bin/disk_cpu_load (+19/-2)
Reviewer Review Type Date Requested Status
Jeff Lane  Approve
Review via email: mp+314029@code.launchpad.net

Description of the change

This MR fixes bug #1641684, at least minimally. (More extensive verbose-mode messages might conceivably be desired in some cases, but I thought it best to start with the basics.) Specifically, there are two chances:

* The cpu_load variable is now calculated via the "bc" command rather
  than with bash's built-in arithmetic. The script failure was caused
  by the ultra-low CPU load of disk on the P8 server, which produced
  a fractional (0.01) CPU load; but bash's integer arithmetic caused
  this to produce an error, so the "set -e" in the script caused a
  failure. Using "bc" to do the calculation works around this problem.
* A new "--verbose" option, if used, produces messages to be displayed
  reporting when key actions are started and on the values of some
  intermediate variables.

To post a comment you must log in.
Revision history for this message
Jeff Lane  (bladernr) wrote :

Cool. Thanks for addressing that.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/bin/disk_cpu_load b/bin/disk_cpu_load
index 5e0d0eb..93b1569 100755
--- a/bin/disk_cpu_load
+++ b/bin/disk_cpu_load
@@ -24,13 +24,14 @@
24#24#
25# Usage:25# Usage:
26# disk_cpu_load [ --max-load <load> ] [ --xfer <mebibytes> ]26# disk_cpu_load [ --max-load <load> ] [ --xfer <mebibytes> ]
27# [ <device-filename> ]27# [ --verbose ] [ <device-filename> ]
28#28#
29# Parameters:29# Parameters:
30# --max-load <load> -- The maximum acceptable CPU load, as a percentage.30# --max-load <load> -- The maximum acceptable CPU load, as a percentage.
31# Defaults to 30.31# Defaults to 30.
32# --xfer <mebibytes> -- The amount of data to read from the disk, in32# --xfer <mebibytes> -- The amount of data to read from the disk, in
33# mebibytes. Defaults to 4096 (4 GiB).33# mebibytes. Defaults to 4096 (4 GiB).
34# --verbose -- If present, produce more verbose output
34# <device-filename> -- This is the WHOLE-DISK device filename (with or35# <device-filename> -- This is the WHOLE-DISK device filename (with or
35# without "/dev/"), e.g. "sda" or "/dev/sda". The36# without "/dev/"), e.g. "sda" or "/dev/sda". The
36# script finds a filesystem on that device, mounts37# script finds a filesystem on that device, mounts
@@ -44,6 +45,7 @@ set -e
44get_params() {45get_params() {
45 disk_device="/dev/sda"46 disk_device="/dev/sda"
46 short_device="sda"47 short_device="sda"
48 verbose=0
47 max_load=3049 max_load=30
48 xfer=409650 xfer=4096
49 while [ $# -gt 0 ] ; do51 while [ $# -gt 0 ] ; do
@@ -54,6 +56,8 @@ get_params() {
54 --xfer) xfer="$2"56 --xfer) xfer="$2"
55 shift57 shift
56 ;;58 ;;
59 --verbose) verbose=1
60 ;;
57 *) disk_device="/dev/$1"61 *) disk_device="/dev/$1"
58 disk_device=`echo $disk_device | sed "s/\/dev\/\/dev/\/dev/g"`62 disk_device=`echo $disk_device | sed "s/\/dev\/\/dev/\/dev/g"`
59 short_device=$(echo $disk_device | sed "s/\/dev//g")63 short_device=$(echo $disk_device | sed "s/\/dev//g")
@@ -109,8 +113,15 @@ compute_cpu_load() {
109 let diff_total=${end_total}-${start_total}113 let diff_total=${end_total}-${start_total}
110 let diff_used=$diff_total-$diff_idle114 let diff_used=$diff_total-$diff_idle
111115
116 if [ "$verbose" == "1" ] ; then
117 echo "Start CPU time = $start_total"
118 echo "End CPU time = $end_total"
119 echo "CPU time used = $diff_used"
120 echo "Total elapsed time = $diff_total"
121 fi
122
112 if [ "$diff_total" != "0" ] ; then123 if [ "$diff_total" != "0" ] ; then
113 let cpu_load=($diff_used*100)/$diff_total124 cpu_load=$(echo "($diff_used*100)/$diff_total" | bc)
114 else125 else
115 cpu_load=0126 cpu_load=0
116 fi127 fi
@@ -127,7 +138,13 @@ echo "Testing CPU load when reading $xfer MiB from $disk_device"
127echo "Maximum acceptable CPU load is $max_load"138echo "Maximum acceptable CPU load is $max_load"
128blockdev --flushbufs $disk_device139blockdev --flushbufs $disk_device
129start_load="$(grep "cpu " /proc/stat | tr -s " " | cut -d " " -f 2-)"140start_load="$(grep "cpu " /proc/stat | tr -s " " | cut -d " " -f 2-)"
141if [ "$verbose" == "1" ] ; then
142 echo "Beginning disk read...."
143fi
130dd if="$disk_device" of=/dev/null bs=1048576 count="$xfer" &> /dev/null144dd if="$disk_device" of=/dev/null bs=1048576 count="$xfer" &> /dev/null
145if [ "$verbose" == "1" ] ; then
146 echo "Disk read complete!"
147fi
131end_load="$(grep "cpu " /proc/stat | tr -s " " | cut -d " " -f 2-)"148end_load="$(grep "cpu " /proc/stat | tr -s " " | cut -d " " -f 2-)"
132compute_cpu_load "$start_load" "$end_load"149compute_cpu_load "$start_load" "$end_load"
133echo "Detected disk read CPU load is $cpu_load"150echo "Detected disk read CPU load is $cpu_load"

Subscribers

People subscribed via source and target branches