[ovs-dev] [PATCH v2] [python] Add monitor_cond_since support

Terry Wilson twilson at redhat.com
Thu Nov 4 13:07:35 UTC 2021


On Wed, Nov 3, 2021, 11:22 AM Ilya Maximets <i.maximets at ovn.org> wrote:

> On 11/3/21 01:36, Terry Wilson wrote:
> > Add support for monitor_cond_since / update3 to python-ovs to
> > allow more efficient reconnections when connecting to clustered
> > OVSDB servers.
> >
> > Signed-off-by: Terry Wilson <twilson at redhat.com>
> > ---
> >  python/ovs/db/idl.py | 55 +++++++++++++++++++++++++++++++++++---------
> >  1 file changed, 44 insertions(+), 11 deletions(-)
> >
>
> Hi, Terry.  Thanks for working on this!
>

Thank you so much for the review!

This patch is missing some unit tests.  At least, you need to enable
> tests that we have for C implementation, so they will run with the
> python implementation too.  E.g:
>
> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
> index 0f229b2f9..1de7f4e67 100644
> --- a/tests/ovsdb-idl.at
> +++ b/tests/ovsdb-idl.at
> @@ -2293,7 +2293,7 @@ m4_define([OVSDB_CHECK_CLUSTER_IDL],
>
>  # Checks that monitor_cond_since works fine when disconnects happen
>  # with cond_change requests in flight (i.e., IDL is properly updated).
> -OVSDB_CHECK_CLUSTER_IDL_C([simple idl, monitor_cond_since, cluster
> disconnect],
> +OVSDB_CHECK_CLUSTER_IDL([simple idl, monitor_cond_since, cluster
> disconnect],
>    3,
>    [['["idltest",
>         {"op": "insert",
> ---
>
> I didn't check, but I guess that this test will actually fail for
> python idl.  The reason is that I don't see any extra handling of
> monitor conditions in the patch.  The sequence of events that test
> above is testing is following:
>

That test does fail. I'll work on adding the support for handling
interrupted cond_change to the patch. It's a bit more of a pain without the
clearly defined state machine of ovsdb-cs, but I'm close to getting it
finished.

It's increasing my resolve to do a refactor of idl.py to be a bit more
ovsdb-cs-ish and a bit more Pythonic in places. Is this something the
community would like to see and would have time to review?

Terry

1. create a monitor
> 2. set conditions
> 3. change conditions and disconnect before receiving reply from
>    a server.
> 4. re-connect.
>
> In order to have correctly updated information with monitor_cond_since,
> in this scenario, on re-connect, idl should send 'old' conditions first
> in order to receive all the current updates and then send 'new' conditions
> in order to receive data for condition update.
>
> If the 'new' conditions will be sent right away, idl will never receive
> data that matches 'new' conditions, but doesn't match 'old' ones for
> transactions that happened before the 'last_id'.
>
> See the corresponding C code here:
>   https://github.com/openvswitch/ovs/blob/master/lib/ovsdb-cs.c#L1058
>
> Best regards, Ilya Maximets.
>
>


More information about the dev mailing list