diff mbox series

[ovs-dev,v2] Add memory reports to all OVN processes.

Message ID 20201217181427.639377-1-i.maximets@ovn.org
State Accepted
Headers show
Series [ovs-dev,v2] Add memory reports to all OVN processes. | expand

Commit Message

Ilya Maximets Dec. 17, 2020, 6:14 p.m. UTC 
It is important to have memory reports in logs to track the state
of OVN daemons.  With this change we will see the dynamic of the
memory consumption.

We have no any specific information to report yet, but we have to
call memory_report() to reply to all the incoming 'memory/show'
requests.

Not documenting 'memory/show' command as there is nothing to
report there yet.  This change will only add reports about
memory usage growth to logs, e.g.:

  |memory|INFO|peak resident set size grew 50% in last 8 seconds,
               from 27548 kB to 41336 kB

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---

Version 2:
  - Added forgotten memory_wait() calls.  Oops.
    Wihtout them accidential call to 'memory/show' might hang.

 controller-vtep/ovn-controller-vtep.c | 11 +++++++++++
 controller/ovn-controller.c           | 12 ++++++++++++
 ic/ovn-ic.c                           | 12 ++++++++++++
 northd/ovn-northd.c                   | 12 ++++++++++++
 utilities/ovn-nbctl.c                 | 12 ++++++++++++
 5 files changed, 59 insertions(+)

Comments

Dumitru Ceara Dec. 17, 2020, 7:45 p.m. UTC | #1 
On 12/17/20 7:14 PM, Ilya Maximets wrote:
> It is important to have memory reports in logs to track the state
> of OVN daemons.  With this change we will see the dynamic of the
> memory consumption.
> 
> We have no any specific information to report yet, but we have to
> call memory_report() to reply to all the incoming 'memory/show'
> requests.
> 
> Not documenting 'memory/show' command as there is nothing to
> report there yet.  This change will only add reports about
> memory usage growth to logs, e.g.:
> 
>   |memory|INFO|peak resident set size grew 50% in last 8 seconds,
>                from 27548 kB to 41336 kB
> 
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

A tiny question: Do you already have ideas about what specific memory
usage reports we should add?  If so, should we add an item to TODO.rst
to address this in the near future?

In any case, it's already useful to get some memory usage information:

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks,
Dumitru
Numan Siddique Jan. 25, 2021, 10:42 a.m. UTC | #2 
On Fri, Dec 18, 2020 at 1:16 AM Dumitru Ceara <dceara@redhat.com> wrote:

> On 12/17/20 7:14 PM, Ilya Maximets wrote:
> > It is important to have memory reports in logs to track the state
> > of OVN daemons.  With this change we will see the dynamic of the
> > memory consumption.
> >
> > We have no any specific information to report yet, but we have to
> > call memory_report() to reply to all the incoming 'memory/show'
> > requests.
> >
> > Not documenting 'memory/show' command as there is nothing to
> > report there yet.  This change will only add reports about
> > memory usage growth to logs, e.g.:
> >
> >   |memory|INFO|peak resident set size grew 50% in last 8 seconds,
> >                from 27548 kB to 41336 kB
> >
> > Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> > ---
>
> A tiny question: Do you already have ideas about what specific memory
> usage reports we should add?  If so, should we add an item to TODO.rst
> to address this in the near future?
>
> In any case, it's already useful to get some memory usage information:
>
> Acked-by: Dumitru Ceara <dceara@redhat.com>
>

Thanks Ilya for adding this and Dumitru for the reviews.
I applied this patch to master.

Numan


>
> Thanks,
> Dumitru
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
diff mbox series

Patch

diff --git a/controller-vtep/ovn-controller-vtep.c b/controller-vtep/ovn-controller-vtep.c
index c13280bc0..1d35c7f04 100644
--- a/controller-vtep/ovn-controller-vtep.c
+++ b/controller-vtep/ovn-controller-vtep.c
@@ -25,9 +25,11 @@ 
 #include "compiler.h"
 #include "daemon.h"
 #include "dirs.h"
+#include "memory.h"
 #include "openvswitch/dynamic-string.h"
 #include "fatal-signal.h"
 #include "openvswitch/poll-loop.h"
+#include "simap.h"
 #include "stream.h"
 #include "stream-ssl.h"
 #include "unixctl.h"
@@ -99,12 +101,21 @@  main(int argc, char *argv[])
             .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
         };
 
+        memory_run();
+        if (memory_should_report()) {
+            struct simap usage = SIMAP_INITIALIZER(&usage);
+
+            /* Nothing special to report yet. */
+            memory_report(&usage);
+            simap_destroy(&usage);
+        }
         gateway_run(&ctx);
         binding_run(&ctx);
         vtep_run(&ctx);
         unixctl_server_run(unixctl);
 
         unixctl_server_wait(unixctl);
+        memory_wait();
         if (exiting) {
             poll_immediate_wake();
         }
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 366fc9c06..2d370e452 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -38,6 +38,7 @@ 
 #include "lflow.h"
 #include "lib/vswitch-idl.h"
 #include "lport.h"
+#include "memory.h"
 #include "ofctrl.h"
 #include "openvswitch/vconn.h"
 #include "openvswitch/vlog.h"
@@ -54,6 +55,7 @@ 
 #include "openvswitch/poll-loop.h"
 #include "lib/bitmap.h"
 #include "lib/hash.h"
+#include "simap.h"
 #include "smap.h"
 #include "sset.h"
 #include "stream-ssl.h"
@@ -2679,6 +2681,15 @@  main(int argc, char *argv[])
     restart = false;
     bool sb_monitor_all = false;
     while (!exiting) {
+        memory_run();
+        if (memory_should_report()) {
+            struct simap usage = SIMAP_INITIALIZER(&usage);
+
+            /* Nothing special to report yet. */
+            memory_report(&usage);
+            simap_destroy(&usage);
+        }
+
         /* If we're paused just run the unixctl server and skip most of the
          * processing loop.
          */
@@ -2960,6 +2971,7 @@  main(int argc, char *argv[])
         ovsdb_idl_track_clear(ovs_idl_loop.idl);
 
 loop_done:
+        memory_wait();
         poll_block();
         if (should_service_stop()) {
             exiting = true;
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index db9ef88da..18e37a31f 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -33,7 +33,9 @@ 
 #include "lib/ovn-nb-idl.h"
 #include "lib/ovn-sb-idl.h"
 #include "lib/ovn-util.h"
+#include "memory.h"
 #include "openvswitch/poll-loop.h"
+#include "simap.h"
 #include "smap.h"
 #include "sset.h"
 #include "stream.h"
@@ -1653,6 +1655,15 @@  main(int argc, char *argv[])
     state.had_lock = false;
     state.paused = false;
     while (!exiting) {
+        memory_run();
+        if (memory_should_report()) {
+            struct simap usage = SIMAP_INITIALIZER(&usage);
+
+            /* Nothing special to report yet. */
+            memory_report(&usage);
+            simap_destroy(&usage);
+        }
+
         if (!state.paused) {
             if (!ovsdb_idl_has_lock(ovnsb_idl_loop.idl) &&
                 !ovsdb_idl_is_lock_contended(ovnsb_idl_loop.idl))
@@ -1727,6 +1738,7 @@  main(int argc, char *argv[])
 
         unixctl_server_run(unixctl);
         unixctl_server_wait(unixctl);
+        memory_wait();
         if (exiting) {
             poll_immediate_wake();
         }
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index b377dffa1..317ea76f1 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -37,10 +37,12 @@ 
 #include "lib/ovn-sb-idl.h"
 #include "lib/ovn-util.h"
 #include "lib/lb.h"
+#include "memory.h"
 #include "ovn/actions.h"
 #include "ovn/logical-fields.h"
 #include "packets.h"
 #include "openvswitch/poll-loop.h"
+#include "simap.h"
 #include "smap.h"
 #include "sset.h"
 #include "svec.h"
@@ -13486,6 +13488,15 @@  main(int argc, char *argv[])
     state.had_lock = false;
     state.paused = false;
     while (!exiting) {
+        memory_run();
+        if (memory_should_report()) {
+            struct simap usage = SIMAP_INITIALIZER(&usage);
+
+            /* Nothing special to report yet. */
+            memory_report(&usage);
+            simap_destroy(&usage);
+        }
+
         if (!state.paused) {
             if (!ovsdb_idl_has_lock(ovnsb_idl_loop.idl) &&
                 !ovsdb_idl_is_lock_contended(ovnsb_idl_loop.idl))
@@ -13557,6 +13568,7 @@  main(int argc, char *argv[])
 
         unixctl_server_run(unixctl);
         unixctl_server_wait(unixctl);
+        memory_wait();
         if (exiting) {
             poll_immediate_wake();
         }
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 94e7eedeb..8efc0d3be 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -29,9 +29,11 @@ 
 #include "lib/acl-log.h"
 #include "lib/ovn-nb-idl.h"
 #include "lib/ovn-util.h"
+#include "memory.h"
 #include "packets.h"
 #include "openvswitch/poll-loop.h"
 #include "process.h"
+#include "simap.h"
 #include "smap.h"
 #include "sset.h"
 #include "stream.h"
@@ -6825,6 +6827,15 @@  server_loop(struct ovsdb_idl *idl, int argc, char *argv[])
     server_cmd_init(idl, &exiting);
 
     for (;;) {
+        memory_run();
+        if (memory_should_report()) {
+            struct simap usage = SIMAP_INITIALIZER(&usage);
+
+            /* Nothing special to report yet. */
+            memory_report(&usage);
+            simap_destroy(&usage);
+        }
+
         ovsdb_idl_run(idl);
         if (!ovsdb_idl_is_alive(idl)) {
             int retval = ovsdb_idl_get_last_error(idl);
@@ -6840,6 +6851,7 @@  server_loop(struct ovsdb_idl *idl, int argc, char *argv[])
             break;
         }
 
+        memory_wait();
         ovsdb_idl_wait(idl);
         unixctl_server_wait(server);
         poll_block();