[ovs-dev] [PATCH ovn] controller: Add IDL memory reports.

[Numan Siddique]

On Tue, Nov 30, 2021 at 4:50 AM Dumitru Ceara <dceara at redhat.com> wrote:
>
> On 11/29/21 23:21, Numan Siddique wrote:
> > On Tue, Nov 23, 2021 at 10:02 AM Dumitru Ceara <dceara at redhat.com> wrote:
> >>
> >> OVS commit 066741d9c5ca ("ovsdb-idl: Add memory report function.")
> >> enhanced the IDL to track memory usage.  Use it in ovn-controller for
> >> the Southbound DB and local OVS DB IDLs.
> >>
> >> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> >> ---
> >>  controller/ovn-controller.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> >> index 29c1a05b2..fa1ff13bd 100644
> >> --- a/controller/ovn-controller.c
> >> +++ b/controller/ovn-controller.c
> >> @@ -3444,6 +3444,8 @@ main(int argc, char *argv[])
> >>              ofctrl_get_memory_usage(&usage);
> >>              if_status_mgr_get_memory_usage(if_mgr, &usage);
> >>              local_datapath_memory_usage(&usage);
> >> +            ovsdb_idl_get_memory_usage(ovnsb_idl_loop.idl, &usage);
> >> +            ovsdb_idl_get_memory_usage(ovs_idl_loop.idl, &usage);
> >
> > Thanks for the patch.  I've one question.
>
> Thanks for the review!
>
> >
> > The function ovsdb_idl_get_memory_usage() adds 2 items to the "usage"
> > simap - "idl-cells" and "idl-outstanding-txns" (if its value is
> > greater than zero).
> > After the ovsdb_idl_get_memory_usage() for OVS IDL is called, the
> > "idl-cells" value in usage will be the sum of SB IDL cells and OVS IDL
> > cells.
>
> Right.
>
> > That's intentional right ?  I think it makes sense.  Just wanted to
> > check with you.
>
> Yes, it's due to how ovsdb_idl_get_memory_usage() currently works.
>
> > Is there a value in reporting as - "sb-idl-cells=<n>", ovs-idl-cells=<p>.."  ?
>
> Good point, I think there is.  And I think we can do that by changing
> ovsdb_idl_get_memory_usage() to prefix the "usage" simap item keys with
> the DB name.
>
> I sent a patch for that in OVS (the OVN bits don't need to change):
> https://patchwork.ozlabs.org/project/openvswitch/patch/20211130094757.23210-1-dceara@redhat.com/

Thanks.   Since there would be no change in OVN.  I applied this
patch.  Probably we have to bump the ovs submodule
once your OVS patch is merged.

Numan

>
> Thanks,
> Dumitru
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>