2
\$\begingroup\$

I have written a script, that reformats the output of qstat -f1 of the PBS queueing system. Unfortunately this project is by far too big to post here complete, also I am still working on it.
The purpose is to have a quick overview of what is running by whom and what resources are blocked. I am am also using chains, where certain jobs are waiting on the completion of another job. These will be put on hold by the queueing system. However, sometimes it is necessary to manually put a job on hold, because there was a more pressing one. Without looking through the whole output I wanted a way to determine which jobs are depending on each other, so that I don't accidentally release a job that does not have the right prerequisites and crashes on start-up.

I have therefore written the following subroutines to resolve such dependencies and list them. It is working quite well for me, but I am still wondering if I have mistakes in it. I cut the script together to a minimal working part, as well as a dummy routine to simulate the output (which is truncated).

#!/bin/bash

# Prepping for the output: truncate or expand variable for later formatting
#
truncExpVar ()
{
    local varcontent="$1"
    local -i varlength=$2
    local varname=$3
    local alignment=$4
    local -i startdigits
    local -i enddigits
    if (( $varlength < 3 )); then fatal "Fatality. Scipt needs debugging."; fi
    if (( ${#varcontent} > $varlength )); then
        if (( $varlength < 10 )); then startdigits=1; else startdigits=3; fi
        enddigits=$((varlength - startdigits -1))
        printf -v $varname "%s\~%s" "${varcontent:0:$startdigits}" "${varcontent:(-$enddigits)}"
    else
        if [ $alignment == "left" ]; then
            printf -v $varname "%-*s" $varlength "$varcontent"
        else
            printf -v $varname "%*s" $varlength "$varcontent"
        fi
    fi
}


#Parse input line for the Job ID
getJobID ()
{
    if [[ $1 =~ Job[[:space:]]Id:[[:space:]]+([0-9]+)\.([0-9a-z]+).* ]]; then
        fulljobid="${BASH_REMATCH[0]}"
        longjobid="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}"
        shortjobid=${BASH_REMATCH[1]}
    else
        return 2
    fi
    truncExpVar "$longjobid" 20 longjobid left
    truncExpVar "$shortjobid" 6 shortjobid right
}


truncDependencyList ()
{
    local truncContent="$1"
    local varname="$2"
    local varlist
    while [[ $truncContent =~ :([0-9]+)\.(.*) ]]; do
        varlist="$varlist ${BASH_REMATCH[1]}"
        truncContent="${BASH_REMATCH[2]}"
    done
    printf -v "$varname" "%s:%s" "${varname:0:1}" "$varlist"
}

getDependencies ()
{
    local afterok beforeok
    local pattern="^[[:space:]]*depend = (beforeok|afterok)(:[^,]+)[,]?(beforeok|afterok)?([:]?.*)$"
    if [[ $1 =~ $pattern ]]; then
        truncDependencyList "${BASH_REMATCH[2]}" ${BASH_REMATCH[1]}
        [[ ! -z ${BASH_REMATCH[3]} ]] && truncDependencyList "${BASH_REMATCH[4]}" ${BASH_REMATCH[3]}
        dependencies="$afterok $beforeok"
    else
        return 2
    fi
}

printline ()
{
    echo $fulljobid $longjobid $shortjobid
    echo $dependencies
    echo "###"
    unset fulljobid longjobid shortjobid dependencies
}

analyzeThis ()
{
    while read line; do
    getJobID "$line"; [[ $? == 0 ]] && continue
    getDependencies "$line"; [[ $? == 0 ]] && continue
#set +x
    if [[ -z "$line" ]]; then
        printline
    fi
    done
}

# For demonstration purpose. Output will come from
#   qstat -f1 $includebobids
# for the PBS queueing system
dummyInput ()
{
#Truncated output
cat << EOF
Job Id: 4765.foo123@bar.ac.jp
    Job_Name = h2o
    Job_Owner = martin@foo123@bar.ac.jp
    job_state = Q
    [...]
    depend = beforeok:4766.foo123@bar.ac.jp@foo123@bar.ac.jp:4768.foo123@bar.ac.jp@foo123@bar.ac.jp
    [...]
    submit_host = foo123@bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4766.foo123@bar.ac.jp
    Job_Name = h2o.freq
    Job_Owner = martin@foo123@bar.ac.jp
    job_state = H
    [...]
    depend = afterok:4765.foo123@bar.ac.jp@foo123@bar.ac.jp,beforeok:4767.foo123@bar.ac.jp@foo123@bar.ac.jp:4768.foo123@bar.ac.jp@foo123@bar.ac.jp
    [...]
    submit_host = foo123@bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4767.foo123@bar.ac.jp
    Job_Name = h2o.nbo
    Job_Owner = martin@foo123@bar.ac.jp
    job_state = H
    [...]
    depend = afterok:4766.foo123@bar.ac.jp@foo123@bar.ac.jp
    [...]
    submit_host = foo123@bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4768.foo123@bar.ac.jp
    Job_Name = h2o.nbo
    Job_Owner = martin@foo123@bar.ac.jp
    job_state = H
    [...]
    depend = afterok:4766.foo123@bar.ac.jp@foo123@bar.ac.jp:4765.foo123@bar.ac.jp@foo123@bar.ac.jp
    [...]
    submit_host = foo123@bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4769.foo123@bar.ac.jp
    Job_Name = h2o
    Job_Owner = martin@foo123@bar.ac.jp
    job_state = Q
    [...]
    submit_host = foo123@bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

EOF
}

analyzeThis < <(dummyInput)
\$\endgroup\$
1
\$\begingroup\$

Use exit code directly

This is a bit awkwardly written:

getJobID "$line"; [[ $? == 0 ]] && continue

The natural (and simpler) way to write this would be:

getJobID "$line" && continue

User-friendliness

This error message doesn't look very useful:

if (( $varlength < 3 )); then fatal "Fatality. Scipt needs debugging."; fi

It would be better to include in the error message a hint about the problem, in this case that $varlength should be >= 3. (There's also a typo in "Scipt".)

Readability

The truncExpVar function could use some blank lines to make it slightly easier to read. For example you could add a blank line after the local variable declarations, and one more after the $varlength validation.

In analyzeThis it would be better to indent the body of the while loop.

Actually I would rewrite that function like this:

analyzeThis ()
{
    while read -r line; do
        getJobID "$line" && continue
        getDependencies "$line" && continue
        [[ "$line" ]] || printline
    done
}

One-liner if

Some one-liner if statements could be written slightly simpler, for example this one:

if (( $varlength < 3 )); then fatal "Fatality. Scipt needs debugging."; fi

Could be written more compactly like this:

(( varlength < 3 )) && fatal "Fatality. Scipt needs debugging."

Math in Bash

You can simplify some of the math, by dropping $ inside ((...)), for example instead of:

if (( $varlength < 3 )); then fatal "Fatality. Scipt needs debugging."; fi
if (( ${#varcontent} > $varlength )); then
    if (( $varlength < 10 )); then startdigits=1; else startdigits=3; fi
    enddigits=$((varlength - startdigits -1))

You can write:

if (( varlength < 3 )); then fatal "Fatality. Scipt needs debugging."; fi
if (( ${#varcontent} > varlength )); then
    if (( varlength < 10 )); then startdigits=1; else startdigits=3; fi
    ((enddigits = varlength - startdigits - 1))

== operator in [ ... ] ?

help [ tells that [ ... ] is an alias to test, and help test only explains the = operator and says nothing about ==. So to be safe, I would replace == with = in this code:

    if [ $alignment == "left" ]; then

shellcheck

http://www.shellcheck.net/# is a great site where you can copy-paste your Bash scripts and it will point out code smells.

\$\endgroup\$
  • \$\begingroup\$ I have substituted [ $alignment == "left" ] with [[ $alignment == "left" ]] now. Is there any downside to that use, or would [ $alignment = "left" ] be better? I concluded from this that [[...]]it might be preferable. \$\endgroup\$ – Martin - マーチン Jun 7 '16 at 8:26
  • \$\begingroup\$ @Martin-マーチン yes, [[ ... ]] is preferable, as long as you don't need POSIX compatibility. And apparently you don't need, as you're already using ((...)) which is also not POSIX compatible. So yes, go for [[ ... ]] \$\endgroup\$ – Stop ongoing harm to Monica Jun 7 '16 at 10:17
0
\$\begingroup\$

After the helpful comments from janos I have adjusted the script in the following way:

  • used exit code directly in analyzeThis
  • added -r to read as well as IFS="" as per suggestion and shellcheck code SC2162
  • added more comments to improve understandability and readability
  • replaced one-line if commands with shorter syntax
  • simplified bash maths
  • Introduced intermediate array storeRematch to save the result of BASH_REMATCH as it was overwritten by the call of truncDependencyList and therefore did not include the full dependencies if the job has before and after dependencies. See also my question on SO concerning this behaviour.

Once I finish the script where this function is used, I'll publish it on github.


#!/bin/bash 

# Part of the myq bash script
# http://codereview.stackexchange.com/q/130034/92423
#
# Declare some functions
#
# Errors, Info and Warnings
#
# Print logging information and warnings nicely.
# If there is an unrecoverable error: display a message and exit.
#

message ()
{
    echo "INFO   : " "$@"
}

warning ()
{
    echo "WARNING: " "$@"
}

fatal ()
{
    echo "ERROR  : " "$@"
    exit 1
}


# Prepping for the output: truncate or expand variable for later formatting
# 

truncExpVar ()
{
    # Expand arguments into variables for processing
    local varcontent="$1"
    local -i varlength=$2
    local varname=$3
    local alignment=$4
    local -i startdigits
    local -i enddigits

    # Test if there is an error in scripting.
    # A variable cannot be truncated to less than three characters
    (( varlength < 3 )) && fatal "Cannot truncate a variable to less than three characters. The issue is likely to be in the source code."

    # Test if variable is longer than the field where it should be printed
    if (( ${#varcontent} > varlength )); then
        # For short variables only echo the first char, else use three chars
        if (( varlength < 10 )); then startdigits=1; else startdigits=3; fi
        # One character is replaced by a tilde to show the truncation
        ((enddigits = varlength - startdigits -1))
        printf -v $varname "%s\~%s" "${varcontent:0:$startdigits}" "${varcontent:(-$enddigits)}"
    # Fill with spaces if variable is shorter than the field
    else
        if [[ $alignment == "left" ]]; then
            printf -v $varname "%-*s" $varlength "$varcontent"
        else
            printf -v $varname "%*s" $varlength "$varcontent"
        fi
    fi
}


#Parse input line for the Job ID
getJobID ()
{
    # The job ID usually comes in the form of 123.servername00@domainname.ac.jp
    # If the server is just an IP address, then longjobid will not make any sense
    if [[ $1 =~ Job[[:space:]]Id:[[:space:]]+([0-9]+)\.([0-9a-z]+).* ]]; then
        fulljobid="${BASH_REMATCH[0]}" 
        longjobid="${BASH_REMATCH[1]}.${BASH_REMATCH[2]}"
        shortjobid=${BASH_REMATCH[1]}
    else
        return 2
    fi
    truncExpVar "$longjobid" 20 longjobid left
    truncExpVar "$shortjobid" 6 shortjobid right
}


truncDependencyList ()
{
    # This function will only get the job ID numbers of the dependency list
    # and discard the rest. The varname is beforeok or afterok, which will
    # be truncated to just one character to indicate that in the output.
    local rawContent="$1" 
    local varname="$2"
    local truncContent
    while [[ $rawContent =~ :([0-9]+)\.(.*) ]]; do
        truncContent="$truncContent ${BASH_REMATCH[1]}"
        rawContent="${BASH_REMATCH[2]}"
    done
    printf -v "$varname" "%s:%s " "${varname:0:1}" "$truncContent"
}

getDependencies ()
{
    # Dependecies may either resolve as being before or after a job is finished or both
    # There is no intrinsic order, hence the second part is optional.
    local afterok beforeok
    # It is necessary to store the rematches, as calling the truncation function will cause
    # BASH_REMATCH to be overwritten, and leak into this function, hence causing 
    # $BASH_REMATCH[3] and [4] to be empty and lost.
    # Ref.: https://stackoverflow.com/q/37674871/3180795
    local -a storeRematch
    # The line comes in the form of
    # depend = afterok:4765.foo123.bar.ac.jp@foo123.bar.ac.jp,beforeok:4767.foo123.bar.ac.jp@foo123.bar.ac.jp
    # strictly there should not be a 'beforeok' after an 'afterok', but better safe than sorry
    local pattern="^[[:space:]]*depend = (beforeok|afterok)(:[^,]+)[,]?(beforeok|afterok)?([:]?.*)$"
    if [[ $1 =~ $pattern ]]; then
        storeRematch=("${BASH_REMATCH[@]}")
        truncDependencyList "${storeRematch[2]}" ${storeRematch[1]} 
        [[ ! -z ${storeRematch[3]} ]] && truncDependencyList "${storeRematch[4]}" ${storeRematch[3]}
        dependencies="$afterok$beforeok"
    else
        return 2
    fi
}

printline ()
{
    #This is a dummy routine
    echo "$fulljobid, $longjobid, $shortjobid"
    echo "$dependencies"
    echo "###"
    unset fulljobid longjobid shortjobid dependencies
}

analyzeThis ()
{
    # Read the input line by line, do not strip leading and trailing spaces
    # This is a truncated version of the function in use in myq
    IFS="" 
    while read -r line; do
        getJobID "$line" && continue
        getDependencies "$line" && continue
        [[ "$line" ]] || printline
    done 
}

# For demonstration purpose. Output will come from 
#   qstat -f1 $includebobids
# for the PBS queueing system

dummyInput ()
{
#Truncated output
cat << EOF
Job Id: 4765.foo123.bar.ac.jp
    Job_Name = h2o
    Job_Owner = martin@foo123.bar.ac.jp
    job_state = Q
    [...]
    depend = beforeok:4766.foo123.bar.ac.jp@foo123.bar.ac.jp:4768.foo123.bar.ac.jp@foo123.bar.ac.jp
    [...]
    submit_host = foo123.bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4766.foo123.bar.ac.jp
    Job_Name = h2o.freq
    Job_Owner = martin@foo123.bar.ac.jp
    job_state = H
    [...]
    depend = afterok:4765.foo123.bar.ac.jp@foo123.bar.ac.jp,beforeok:4767.foo123.bar.ac.jp@foo123.bar.ac.jp:4768.foo123.bar.ac.jp@foo123.bar.ac.jp
    [...]
    submit_host = foo123.bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4767.foo123.bar.ac.jp
    Job_Name = h2o.nbo
    Job_Owner = martin@foo123.bar.ac.jp
    job_state = H
    [...]
    depend = afterok:4766.foo123.bar.ac.jp@foo123.bar.ac.jp
    [...]
    submit_host = foo123.bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4768.foo123.bar.ac.jp
    Job_Name = h2o.nbo
    Job_Owner = martin@foo123.bar.ac.jp
    job_state = H
    [...]
    depend = afterok:4766.foo123.bar.ac.jp@foo123.bar.ac.jp:4765.foo123.bar.ac.jp@foo123.bar.ac.jp
    [...]
    submit_host = foo123.bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

Job Id: 4769.foo123.bar.ac.jp
    Job_Name = h2o
    Job_Owner = martin@foo123.bar.ac.jp
    job_state = Q
    [...]
    submit_host = foo123.bar.ac.jp
    init_work_dir = /home/martin/tester/test-water

EOF
}

analyzeThis < <(dummyInput)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.