[ovs-dev] [PATCH 1/7] tests: Fix cluster torture test.

Mark Michelson mmichels at redhat.com
Fri Aug 3 12:34:53 UTC 2018


For the series:

Acked-by: Mark Michelson <mmichels at redhat.com>

I have a couple of small notes in-line below on this particular commit 
message.

On 07/26/2018 01:09 PM, Ben Pfaff wrote:
> A previous commit to improve timing also caused the cluster torture test to
> be skipped (unless it failed early).  This is related to the shell "while"
> loop's use of a variable $phase to indicate how far it got in the test
> procedure.  A very fast machine, or one on which the races went just the
> right way, might finish the test before all the torture properly starts, so
> the code is designed to just skip the test if that happens.  However, wh

The text cuts off here.

> 
> Prior to the timing commit, the loop looked something like this:
> 
>      phase=0
>      while :; do
>          ...things that eventually increment $phase to 2...
>      done
>      AT_SKIP_IF([test $phase != 2])
> 
> This works fine.
> 
> The timing commit changed the "while :" to "(...something...) | while
> read".  This looks innocuous but it actually causes everything inside the
> "while" loop to run in a subshell.  Thus, the increments to $phase are not
> visible after the loop ends, and the test always gets skipped.
> 
> This commit fixes the problem by storing the phase in a file instead of a
> shell variable.

I would suggest adding a small comment to the test explaining this. I 
could see someone trying to be "helpful" down the line by converting 
phase back into a shell variable.

> 
> Fixes: 0f03ae3754ec ("ovsdb: Improve timing in cluster torture test.")
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>   tests/ovsdb-cluster.at | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
> index 1c4149155dda..a5929cf04de2 100644
> --- a/tests/ovsdb-cluster.at
> +++ b/tests/ovsdb-cluster.at
> @@ -149,7 +149,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from ephemeral
>       sleep 2
>   
>       echo "waiting for ovn-sbctl processes to exit..."
> -    phase=0
> +    echo 0 > phase
>       i=0
>       (while :; do echo; sleep 1; done) | while read; do
>           printf "t=%2d s:" $i
> @@ -165,7 +165,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from ephemeral
>               break
>           fi
>   
> -        case $phase in # (
> +        case $(cat phase) in # (
>           0)
>               if test $done -ge $(expr $n1 / 4); then
>                   if test $variant = kill; then
> @@ -173,7 +173,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from ephemeral
>                   else
>                       remove_server $victim
>                   fi
> -                phase=1
> +                echo 1 > phase
>                   next=$(expr $i + 2)
>               fi
>               ;; # (
> @@ -185,7 +185,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from ephemeral
>                   else
>                       add_server $victim
>                   fi
> -                phase=2
> +                echo 2 > phase
>               fi
>               ;;
>           esac
> @@ -193,7 +193,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from ephemeral
>           i=$(expr $i + 1)
>       done
>       echo "...done"
> -    AT_CHECK([if test $phase != 2; then exit 77; fi])
> +    AT_CHECK([if test $(cat phase) != 2; then exit 77; fi])
>   
>       for i in $(seq 0 $(expr $n1 - 1) ); do
>           for j in `seq $n2`; do
> @@ -203,7 +203,7 @@ ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from ephemeral
>       AT_CHECK([ovn-sbctl --timeout=30 --log-file=finalize.log -vtimeval:off -vfile -vsyslog:off --bare get SB_Global . external-ids | sed 's/, /\n/g; s/[[{}""]]//g;' | sort], [0], [expout])
>   
>       for i in `seq $n`; do
> -        if test $i != $victim || test $phase != 1; then
> +        if test $i != $victim || test $(cat phase) != 1; then
>               stop_server $i
>           fi
>       done
> 



More information about the dev mailing list