Thanks for the comments. I resolved all of them and applied this series to master and branch-2.10.
On Fri, Aug 03, 2018 at 08:34:53AM -0400, Mark Michelson wrote: > For the series: > > Acked-by: Mark Michelson <[email protected]> > > 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 <[email protected]> > >--- > > 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 > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
