Process monitoring script

Question

I wrote a script which logs memory usage of a process (pid in file), and reacts in case a limit is reached. I run the script nightly. In my case, the process concerned propably has a memory leak and locks up the whole system after a couple of days. Hence, the script should reboot the system if excess memory is used. Logged memory usage values are formated to have 1 decimal place.

The code works, but I'd like to know if and how I could optimize it, and if there are no mistakes.

#!/bin/sh

# process_monitor.sh
#
# monitor specific process
# customized for memory usage monitor (can easily be changed)
# customized to perform system reboot if limit is exceeded (can easily be changed)
# by geohei <[email protected]>
# created : 14.10.2016
# revised : n/a
#
# use cron to trigger (e.g. nightly at 04:15 '15 4 * * * ~/process_monitor.sh'
# /bin/bc is used (possibly not installed by default)
#
# $pidfile must include path
# $logpath is used to save 'process_monitor.log' file (omit trailing '/')
# $pidlimit can be floating point (e.g. 14.1)

pidfile='/tmp/process.pid'
logpath='~'
pidlimit=20.0

pidvalue=$(ps -p $(eval "cat $echo $pidfile") -o %mem --no-headers)

if [ "$(echo "${pidvalue} > ${pidlimit}" | bc)" -eq 1 ]; then
    # log $pidvalue, $pidlimit and reboot system
    echo "$(date '+%Y-%m-%d %H:%M:%S')   pidvalue: $pidvalue, pidlimit: $(echo $pidlimit | awk '{printf "%.1f\n", int($1)}') - reboot" >> $logpath/process_monitor.log
    shutdown -r -t 10
    exit 1
else
    # log $pidvalue, $pidlimit
    echo "$(date '+%Y-%m-%d %H:%M:%S')   pidvalue: $pidvalue, pidlimit: $pidlimit" >> $logpath/process_monitor.log
fi

exit 0

That's how the log looks like:

...
2016-10-14 04:15:04   pidvalue: 19.2, pidlimit: 20.0
2016-10-15 04:15:03   pidvalue: 20.4, pidlimit: 20.0 - reboot
...

Any remarks, comments or suggestions?

Rebooting the machine because one process allocated too much memory sounds like a drastic measure. Why wouldn't you just stop and restart the offending process? — 200_success
– 200_success, Commented Oct 15, 2016 at 13:51
Yes, you are absolutely right. The script was designed to have the basic structure for all kind of things which might be triggered. Of course I will adapt the script on the real productive server to first restart the monitored process at a first stage. Thanks for the hint. — geohei
– geohei, Commented Oct 15, 2016 at 14:06

200_success · Accepted Answer · 2016-10-15 14:36:52Z

3

Don't write custom scripts for common sysadmin tasks. Using the proper tools will get the job done better, and the solution will be more maintainable — especially by any colleagues you might have.

Using monit, for example, this would just be a matter of writing a simple configuration file.

set logfile /root/process_monitor.log
check process myproc with pidfile /tmp/pidfile.pid
    restart program = "shutdown -r -t 10"  # draconian!
    every "15 4 * * *"
    if memory > 20 MB then restart myproc

answered Oct 15, 2016 at 14:36

200_success

146k22 gold badges191 silver badges481 bronze badges

\$\begingroup\$ Wow! I had no clue such a tool existed. Going to dig myself through the man pages. Thanks a lot! \$\endgroup\$

geohei
– geohei

2016-10-15 18:49:05 +00:00
Commented Oct 15, 2016 at 18:49

Add a comment |

janos · Accepted Answer · 2016-10-15 21:14:15Z

Although @200_success already gave a great answer, there are some glaring issues with the posted code that should be pointed so you can avoid them in the future.

eval is evil. Not really, but it's prone to abuses, and it's very hard to justify its use. This is a very twisted example:

pidvalue=$(ps -p $(eval "cat $echo $pidfile") -o %mem --no-headers)

That should be simply:

pidvalue=$(ps -p $(cat "$pidfile") -o %mem --no-headers)

Using bc for a boolean evaluation is extremely awkward:

if [ "$(echo "${pidvalue} > ${pidlimit}" | bc)" -eq 1 ]; then

Write like this:

if [ "${pidvalue}" -gt "${pidlimit}" ]; then

I used -gt because you are using /bin/sh. If you don't mind changing that to Bash, then it would become even more simple and natural:

if (( pidvalue > pidlimit )); then

These lines are very similar:

echo "$(date '+%Y-%m-%d %H:%M:%S')   pidvalue: $pidvalue, pidlimit: $(echo $pidlimit | awk '{printf "%.1f\n", int($1)}') - reboot" >> $logpath/process_monitor.log
# ...
echo "$(date '+%Y-%m-%d %H:%M:%S')   pidvalue: $pidvalue, pidlimit: $pidlimit" >> $logpath/process_monitor.log

Probably you copy-pasted and customized. That's a very bad habit. Try to generalize instead. Extract common values to variables, such as the common prefix $(date '+%Y-%m-%d %H:%M:%S') pidvalue: $pidvalue, pidlimit:. Extract common operations to helper functions, such as logging to $logpath/process_monitor.log.

The last command of the program is exit 0. That's strange, for several reasons:

The script would exit anyway, you don't need to call exit explicitly.
By default, the exit code is the status code of the last command. An exit code of 0 means success. By running exit 0 as the last statement, you may cover up the failure of the last command. That's a suspicious thing to do.

Stack Exchange Network

Process monitoring script

2 Answers 2

You must log in to answer this question.

Hot Network Questions

Process monitoring script

2 Answers 2

You must log in to answer this question.

Related

Hot Network Questions