Percona Monitoring Plugins

pmp-check-mysql-status trap fails to remove temp files

Bug #977514 reported by Michael Kania
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona Monitoring Plugins
Fix Released
Medium
Baron Schwartz
Percona Monitoring Plugins 1.0.1

Bug Description

pmp-check-mysql-status creates assigns paths for two temporary files as local variables TEMP1 and TEMP2 in main(). The script then sets up a trap to delete the two temporary files on EXIT. The trouble that I am noticing is that TEMP1 and TEMP2 are local to main() only and the trap is called outside of main() which means TEMP1 and TEMP2 are outside of the scope of the trap and the files never get removed. Below is part of a debug trace highlighting what I am seeing.

+++ mktemp /var/tmp/pmp-check-mysql-status.XXXX
++ local TEMP1=/var/tmp/pmp-check-mysql-status.jrvG
+++ mktemp /var/tmp/pmp-check-mysql-status.XXXX
++ local TEMP2=/var/tmp/pmp-check-mysql-status.K9Ex
++ trap 'rm -rf "${TEMP1}" "${TEMP2}"' EXIT
.
.
.
.
++ rm -rf '' ''
+ OUTPUT='OK Threads_running = 2'
+ EXITSTATUS=3
+ case "${OUTPUT}" in
+ EXITSTATUS=0
+ echo 'OK Threads_running = 2'
OK Threads_running = 2
+ exit 0

Tags: i24357
Revision history for this message
Baron Schwartz (baron-xaprb) wrote :

I have observed that the plugin works correctly on all of the platforms I use. Can you provide more details about the OS, version, and shell version you are using?

Revision history for this message
Michael Kania (dynamike) wrote :

the default shell in debian is dash. I tried both dash and bash with the same results.

Debian Squeeze(Debian 2.6.32-41squeeze2)
GNU bash, version 4.1.5(1)-release (x86_64-pc-linux-gnu)
dash 0.5.5.1-7.4

Revision history for this message
Moritz Schuepp (msc-0) wrote :

Bug applies to all Plugins using Traps:

pmp-check-mysql-innodb: trap 'rm -rf "${TEMP}" >/dev/null 2>&1' EXIT
pmp-check-mysql-processlist: trap 'rm -rf "${TEMP}" >/dev/null 2>&1' EXIT
pmp-check-mysql-status: trap 'rm -rf "${TEMP1}" "${TEMP2}" >/dev/null 2>&1' EXIT
pmp-check-pt-table-checksum: trap 'rm -rf "${TEMP}" >/dev/null 2>&1' EXIT

Please remove the single quotes as those prevent variables from expanding.

I would also recommend to use "rm -f" instead of "rm -rf" as you are removing tempfiles and not directories. Just in case... :)

So my recommandation for the traps would be:

pmp-check-mysql-innodb: trap "rm -f ${TEMP} >/dev/null 2>&1" EXIT
pmp-check-mysql-processlist: trap "rm -f ${TEMP} >/dev/null 2>&1" EXIT
pmp-check-mysql-status: trap "rm -f ${TEMP1} ${TEMP2} >/dev/null 2>&1" EXIT
pmp-check-pt-table-checksum: trap "rm -f ${TEMP} >/dev/null 2>&1" EXIT

Thanks!

Revision history for this message
Michael Kania (dynamike) wrote :

Any updates on this guy?

Revision history for this message
Baron Schwartz (baron-xaprb) wrote :

The proper solution appears to be different than that proposed in comment #3. Quoting is necessary to prevent problems with a temporary directory that has a space in the path. Here is the solution I intend to implement:

   trap "rm -f '${TEMP1}' '${TEMP2}' >/dev/null 2>&1" EXIT

This will be released in the next version of the plugins.

Baron Schwartz (baron-xaprb)
Changed in percona-monitoring-plugins:
status: New → Confirmed
importance: Undecided → Medium
assignee: nobody → Baron Schwartz (baron-xaprb)
milestone: none → 1.0.1
Baron Schwartz (baron-xaprb)
Changed in percona-monitoring-plugins:
status: Confirmed → Fix Committed
Marcos Albe (marcos-albe)
tags: added: i24357
Baron Schwartz (baron-xaprb)
Changed in percona-monitoring-plugins:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.