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

Ilya Maximets i.maximets at ovn.org
Wed Nov 3 16:22:28 UTC 2021


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!

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:

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