On Wed, Nov 3, 2021, 11:22 AM Ilya Maximets <i.maxim...@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 <twil...@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.
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
