Naman
Naman

Reputation: 31868

Tail recursion in Bash

I've tried to write a script to verify that all the stats of a metrics are positive before I make any further changes using the service. The part I'm stuck at is thinking over how to tail the recursion for the following use-case :

function load_cache() {
    cacheStat=( $(curl -s -X GET "http://localhost:${MET_PORT}/metrics" | sed 's/\\\\\//\//g' | sed 's/[{}]//g' | awk -v k="cacheSize" '{n=split($0,a,","); for (i=1; i<=n; i++) print a[i]}' | sed 's/\"\:\"/\|/g' | sed 's/[\,]/ /g' | sed 's/\"//g' | grep -w "cacheSize" | cut -d ':' -f 2) )

    # the above gives me the ouput(cacheStat) as -
    # 2.0
    # 311.0
    # 102.0

    count=0
    for index in ${!cacheStat[*]}
    do
        if [[ ${cacheStat[$index]} -le 0 ] && [ $count -lt 3 ]]; then
            sleep .5
            count=$[$count +1];
            load_cache
            #Wouldn't the above initialise `count` to 0 again.
        fi
    done
}

What I am trying to do is if any of the elements in the cacheStat is less than or equal to 0, then sleep for .5 secs and query the cacheStat again and perform the check on all its elements again. Though not do this more than 3 times for which I am trying to use `count.

Open to any suggestion to improve the script.


Update - On modifying the scripts as suggested by @Inian to

RETRY_COUNT=0
function load_cache() {
    cacheStat=( $(curl -s -X GET "http://localhost:${MET_PORT}/metrics" | sed 's/\\\\\//\//g' | sed 's/[{}]//g' | awk -v k="cacheSize" '{n=split($0,a,","); for (i=1; i<=n; i++) print a[i]}' | sed 's/\"\:\"/\|/g' | sed 's/[\,]/ /g' | sed 's/\"//g' | grep -w "cacheSize" | cut -d ':' -f 2) );
    for index in ${!cacheStat[*]}
    do
        echo "Stat - ${cacheStat[$index]}"
        if (( ${cacheStat[$index]} <= 0 )) && (( $RETRY_COUNT < 3 )); then
            echo "Attempt count - ${RETRY_COUNT}"
            sleep .5s
            RETRY_COUNT=$((RETRY_COUNT +1));
            load_cache
        fi
    done
}

The logs read -

>     > + cacheStat=($(curl -s -X GET "http://localhost:${MET_PORT}/metrics" | sed 's/\\\\\//\//g' | sed
> 's/[{}]//g' | awk -v k="cacheSize"
>     > '{n=split($0,a,","); for (i=1; i<=n; i++) print a[i]}' | sed
>     > 's/\"\:\"/\|/g' | sed 's/[\,]/ /g' | sed 's/\"//g' | grep -w
>     > "cacheSize" | cut -d ':' -f 2))
>     > ++ curl -s -X GET http://localhost:8181/metrics
>     > ++ sed 's/\\\\\//\//g'
>     > ++ sed 's/[{}]//g'
>     > ++ sed 's/[\,]/ /g'
>     > ++ awk -v k=cacheSize '{n=split($0,a,","); for (i=1; i<=n; i++) print a[i]}'
>     > ++ sed 's/\"\:\"/\|/g'
>     > ++ cut -d : -f 2
>     > ++ sed 's/\"//g'
>     > ++ grep -w cacheSize

It doesn't even iterate I guess.

Upvotes: 1

Views: 1026

Answers (2)

tripleee
tripleee

Reputation: 189317

You can avoid all that ad-hoc JSON parsing by using a JSON parser.

# Avoid using Bash-only "function" keyword
load_cache () {
    local try
    for try in 1 2 3; do
        # Suction: jq doesn't return non-zero exit code for no match
        # work around that by piping to grep .
        if curl -s -X GET "http://localhost:${MET_PORT}/metrics" |
            jq '.[] | select(cacheSize < 0)' |
            grep .
        then
            # Notice also redirection to stderr for diagnostic messages
            echo "$0: Attempt $try failed, sleeping before retrying" >&2
            sleep 0.5
        else
            # Return with success, we are done, exit function
            return 0
        fi
    done

    # Return failure
    return 1
}

I see no reason to prefer recursion over a straightforward for loop for controlling the number of retries.

If you never want to see the offending values, you can use grep -q in the conditional. I'm expecting you would do load_cache >/dev/null if you don't want the output.

If you want to see the non-offending values, the code will need some refactoring, but I'm focusing on getting the central job done elegantly and succinctly. Here's a sketch, mainly to show you the jq syntax for that.

load_cache () {
    local try
    local results
    for try in 1 2 3; do
        results=$(curl -s -X GET "http://localhost:${MET_PORT}/metrics" |
            jq '.[] | .cacheSize' | tr '\n' ' ')
        echo "$0: try $try: cacheSize $results" >&2
        # Funky: massage the expression we test againt into a normalized form
        # so that we know that the value will always be preceded by a space
        case " $results " in
          *" 0 "* | *" -"* )
             case $try in
              3) echo "$0: try $try failed; aborting" >&2 ;;
              *) echo "$0: try $try failed; sleeping before retrying" >&2
                 sleep 0.5 ;;
             esac;;
          *) return 0
        esac
    done
    return 1
}

The nested case to avoid sleeping on the final iteration isn't particularly elegant, but at least it should ensure that the reader is awake. /-8

Upvotes: 1

Inian
Inian

Reputation: 85550

Remove the infinite recursion by moving the count=0 outside the function body.

Also your script has couple of issues, a syntax violation and an outdated construct, lines 12-14 should have been,

if [[ ${cacheStat[$index]} -le 0 ]] && [[ $count -lt 3 ]]; then
    sleep .5s
    count=$((count +1));
    load_cache
fi

or) use a more readable arithmetic operator, (()) in the if-clause as

if (( ${cacheStat[$index]} <= 0 )) && (( $count < 3 )); then

bash does not inherently support floating point arithmetic (comparison in your case), use a third party tool like bc, awk for this,

if (( $(echo "${cacheStat[$index]} <= 0" | bc -l) )) && (( $count < 3 )); then

Upvotes: 4

Related Questions