diff mbox series

[ovs-dev] tests: fix "recompute" test

Message ID 20220829161320.3126201-1-xsimonar@redhat.com
State Accepted
Headers show
Series [ovs-dev] tests: fix "recompute" test | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Xavier Simonart Aug. 29, 2022, 4:13 p.m. UTC
Recompute test might fail on some slow systems due to sb inactivity probe
causing recomputes.
Disable inactivity probe to avoid this issue.

Fixes: a7c7d451 ("controller: avoid recomputes triggered by SBDB Port_Binding updates")

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
---
 tests/ovn.at | 2 ++
 1 file changed, 2 insertions(+)

Comments

Han Zhou Aug. 29, 2022, 7:30 p.m. UTC | #1
On Mon, Aug 29, 2022 at 9:13 AM Xavier Simonart <xsimonar@redhat.com> wrote:
>
> Recompute test might fail on some slow systems due to sb inactivity probe
> causing recomputes.
> Disable inactivity probe to avoid this issue.
>
Hi Xavier,

Thanks for making the test case stable, but before that shall we check why
inactivity probe triggers recompute? Inactivity probe doesn't change any
input, so it is not expected to cause recompute. If that's not the case,
I'd fix the I-P engine instead of the test case.

Thanks,
Han

> Fixes: a7c7d451 ("controller: avoid recomputes triggered by SBDB
Port_Binding updates")
>
> Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
> ---
>  tests/ovn.at | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 5d73c3379..53519b92b 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -32611,6 +32611,8 @@ for i in $(seq 1 $n_hv); do
>      ovn_attach n1 br-phys 192.168.0.$i 24 geneve
>  done
>
> +check ovn-sbctl set connection . inactivity_probe=0
> +
>  add_switch_ports() {
>      start_port=$1
>      end_port=$2
> --
> 2.31.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Xavier Simonart Aug. 30, 2022, 7:21 a.m. UTC | #2
Hi Han

Thanks for your feedback.

Inactivity probe kicks in as the test is heavy (4 hv, each claiming
hundreds of ports) + the fact that github might run multiple of those tests
in parallel (monitor-all true / false, parallel northd).
I saw very long poll interval in the controller (> 5 seconds), due to
sb_port_binding handlers.
When inactivity probe kicked in, commits to sb from controller failed as
connection has dropped, causing recompute in the next run.Isn't this the
expected behavior?

Thanks
Xavier

On Mon, Aug 29, 2022 at 9:30 PM Han Zhou <zhouhan@gmail.com> wrote:

>
>
> On Mon, Aug 29, 2022 at 9:13 AM Xavier Simonart <xsimonar@redhat.com>
> wrote:
> >
> > Recompute test might fail on some slow systems due to sb inactivity probe
> > causing recomputes.
> > Disable inactivity probe to avoid this issue.
> >
> Hi Xavier,
>
> Thanks for making the test case stable, but before that shall we check why
> inactivity probe triggers recompute? Inactivity probe doesn't change any
> input, so it is not expected to cause recompute. If that's not the case,
> I'd fix the I-P engine instead of the test case.
>
> Thanks,
> Han
>
> > Fixes: a7c7d451 ("controller: avoid recomputes triggered by SBDB
> Port_Binding updates")
> >
> > Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
> > ---
> >  tests/ovn.at | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index 5d73c3379..53519b92b 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -32611,6 +32611,8 @@ for i in $(seq 1 $n_hv); do
> >      ovn_attach n1 br-phys 192.168.0.$i 24 geneve
> >  done
> >
> > +check ovn-sbctl set connection . inactivity_probe=0
> > +
> >  add_switch_ports() {
> >      start_port=$1
> >      end_port=$2
> > --
> > 2.31.1
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Han Zhou Aug. 31, 2022, 12:33 a.m. UTC | #3
On Tue, Aug 30, 2022 at 12:21 AM Xavier Simonart <xsimonar@redhat.com>
wrote:
>
> Hi Han
>
> Thanks for your feedback.
>
> Inactivity probe kicks in as the test is heavy (4 hv, each claiming
hundreds of ports) + the fact that github might run multiple of those tests
in parallel (monitor-all true / false, parallel northd).
> I saw very long poll interval in the controller (> 5 seconds), due to
sb_port_binding handlers.
> When inactivity probe kicked in, commits to sb from controller failed as
connection has dropped, causing recompute in the next run.Isn't this the
expected behavior?
>
Thanks for clarification! So it is probe failure causing recompute instead
of probe itself, which makes sense.
I applied it to main with the minor word changes in the commit message.

Han

> Thanks
> Xavier
>
> On Mon, Aug 29, 2022 at 9:30 PM Han Zhou <zhouhan@gmail.com> wrote:
>>
>>
>>
>> On Mon, Aug 29, 2022 at 9:13 AM Xavier Simonart <xsimonar@redhat.com>
wrote:
>> >
>> > Recompute test might fail on some slow systems due to sb inactivity
probe
>> > causing recomputes.
>> > Disable inactivity probe to avoid this issue.
>> >
>> Hi Xavier,
>>
>> Thanks for making the test case stable, but before that shall we check
why inactivity probe triggers recompute? Inactivity probe doesn't change
any input, so it is not expected to cause recompute. If that's not the
case, I'd fix the I-P engine instead of the test case.
>>
>> Thanks,
>> Han
>>
>> > Fixes: a7c7d451 ("controller: avoid recomputes triggered by SBDB
Port_Binding updates")
>> >
>> > Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
>> > ---
>> >  tests/ovn.at | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/tests/ovn.at b/tests/ovn.at
>> > index 5d73c3379..53519b92b 100644
>> > --- a/tests/ovn.at
>> > +++ b/tests/ovn.at
>> > @@ -32611,6 +32611,8 @@ for i in $(seq 1 $n_hv); do
>> >      ovn_attach n1 br-phys 192.168.0.$i 24 geneve
>> >  done
>> >
>> > +check ovn-sbctl set connection . inactivity_probe=0
>> > +
>> >  add_switch_ports() {
>> >      start_port=$1
>> >      end_port=$2
>> > --
>> > 2.31.1
>> >
>> > _______________________________________________
>> > dev mailing list
>> > dev@openvswitch.org
>> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/tests/ovn.at b/tests/ovn.at
index 5d73c3379..53519b92b 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -32611,6 +32611,8 @@  for i in $(seq 1 $n_hv); do
     ovn_attach n1 br-phys 192.168.0.$i 24 geneve
 done
 
+check ovn-sbctl set connection . inactivity_probe=0
+
 add_switch_ports() {
     start_port=$1
     end_port=$2