Reputation: 31868
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
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
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 likebc
,awk
for this,
if (( $(echo "${cacheStat[$index]} <= 0" | bc -l) )) && (( $count < 3 )); then
Upvotes: 4