diff mbox series

[ovs-dev,v2,2/2] ovsdb-cs: Add coverage for updates.

Message ID 2e943333189533bd6844a9ea5a506e8cce324c10.1773736007.git.felix.huettner@digits.schwarz
State Changes Requested
Delegated to: Ilya Maximets
Headers show
Series [ovs-dev,v2,1/2] ovsdb: Support custom transaction history size. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/cirrus-robot success cirrus build: passed
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Felix Huettner March 17, 2026, 8:28 a.m. UTC
This allows us to figure out how often we receive updates from upstream
dbs. In addition we can see e.g. during a reconnection if we did clear
the database (e.g. because monitor_cond_since did not work).

Signed-off-by: Felix Huettner <felix.huettner@digits.schwarz>
---
 lib/ovsdb-cs.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Ilya Maximets April 7, 2026, 8:45 p.m. UTC | #1
On 3/17/26 9:28 AM, Felix Huettner via dev wrote:
> This allows us to figure out how often we receive updates from upstream
> dbs. In addition we can see e.g. during a reconnection if we did clear
> the database (e.g. because monitor_cond_since did not work).
> 
> Signed-off-by: Felix Huettner <felix.huettner@digits.schwarz>
> ---
>  lib/ovsdb-cs.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
> index df33a835d..0f0f88ef7 100644
> --- a/lib/ovsdb-cs.c
> +++ b/lib/ovsdb-cs.c
> @@ -20,6 +20,7 @@
>  
>  #include <errno.h>
>  
> +#include "coverage.h"
>  #include "hash.h"
>  #include "jsonrpc.h"
>  #include "openvswitch/dynamic-string.h"
> @@ -38,6 +39,9 @@
>  #include "uuid.h"
>  
>  VLOG_DEFINE_THIS_MODULE(ovsdb_cs);

nit: An empty line here.

> +COVERAGE_DEFINE(ovsdb_cs_update);
> +COVERAGE_DEFINE(ovsdb_cs_update_with_clear);
> +COVERAGE_DEFINE(ovsdb_cs_update_from_monitor);

Maybe drop the 'ovsdb_' prefix from these to make them a little sorter?

Also, have you considered putting these into idl code that parses events
and count all types of events at the same location?  Not sure if it's
necessary, but might be a reasonable thing to do.

Best regards, Ilya Maximets.
Felix Huettner April 13, 2026, 2:18 p.m. UTC | #2
Am Tue, Apr 07, 2026 at 10:45:03PM +0200 schrieb Ilya Maximets:
> On 3/17/26 9:28 AM, Felix Huettner via dev wrote:
> > This allows us to figure out how often we receive updates from upstream
> > dbs. In addition we can see e.g. during a reconnection if we did clear
> > the database (e.g. because monitor_cond_since did not work).
> > 
> > Signed-off-by: Felix Huettner <felix.huettner@digits.schwarz>
> > ---
> >  lib/ovsdb-cs.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
> > index df33a835d..0f0f88ef7 100644
> > --- a/lib/ovsdb-cs.c
> > +++ b/lib/ovsdb-cs.c
> > @@ -20,6 +20,7 @@
> >  
> >  #include <errno.h>
> >  
> > +#include "coverage.h"
> >  #include "hash.h"
> >  #include "jsonrpc.h"
> >  #include "openvswitch/dynamic-string.h"
> > @@ -38,6 +39,9 @@
> >  #include "uuid.h"
> >  
> >  VLOG_DEFINE_THIS_MODULE(ovsdb_cs);

Hi Ilya,

thanks for the review.

> 
> nit: An empty line here.

done.

> 
> > +COVERAGE_DEFINE(ovsdb_cs_update);
> > +COVERAGE_DEFINE(ovsdb_cs_update_with_clear);
> > +COVERAGE_DEFINE(ovsdb_cs_update_from_monitor);
> 
> Maybe drop the 'ovsdb_' prefix from these to make them a little sorter?

done.

> 
> Also, have you considered putting these into idl code that parses events
> and count all types of events at the same location?  Not sure if it's
> necessary, but might be a reasonable thing to do.

The benefit i see in the cs code is that is then also directly usable
for relays.
But if you prefer i can also split it and create two sets of counters. One for
the IDL and one for the relays.

Thanks,
Felix

> 
> Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/ovsdb-cs.c b/lib/ovsdb-cs.c
index df33a835d..0f0f88ef7 100644
--- a/lib/ovsdb-cs.c
+++ b/lib/ovsdb-cs.c
@@ -20,6 +20,7 @@ 
 
 #include <errno.h>
 
+#include "coverage.h"
 #include "hash.h"
 #include "jsonrpc.h"
 #include "openvswitch/dynamic-string.h"
@@ -38,6 +39,9 @@ 
 #include "uuid.h"
 
 VLOG_DEFINE_THIS_MODULE(ovsdb_cs);
+COVERAGE_DEFINE(ovsdb_cs_update);
+COVERAGE_DEFINE(ovsdb_cs_update_with_clear);
+COVERAGE_DEFINE(ovsdb_cs_update_from_monitor);
 
 /* Connection state machine.
  *
@@ -1568,6 +1572,14 @@  ovsdb_cs_db_add_update(struct ovsdb_cs_db *db,
                        const struct json *table_updates, int version,
                        bool clear, bool monitor_reply)
 {
+    COVERAGE_INC(ovsdb_cs_update);
+    if (clear) {
+        COVERAGE_INC(ovsdb_cs_update_with_clear);
+    }
+    if (monitor_reply) {
+        COVERAGE_INC(ovsdb_cs_update_from_monitor);
+    }
+
     struct ovsdb_cs_event *event = ovsdb_cs_db_add_event(
         db, OVSDB_CS_EVENT_TYPE_UPDATE);
     event->update = (struct ovsdb_cs_update_event) {