[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