diff mbox series

[ovs-dev,v2] hmap: Improve debug log message when reporting unusually large buckets.

Message ID 20190326165820.31931-1-blp@ovn.org
State Accepted
Commit 6a7bceaac390c3fcdf97a259d9d27ba66b44d827
Headers show
Series [ovs-dev,v2] hmap: Improve debug log message when reporting unusually large buckets. | expand

Commit Message

Ben Pfaff March 26, 2019, 4:58 p.m. UTC
I was seeing a lot of these messages, including a lot of them suppressed
by rate-limiting, and I wondered whether any really big messages were
being suppressed.  By reporting the largest bucket, instead of just every
large bucket, it becomes more likely that the truly too-large buckets get
reported.

(The problem I saw was a false alarm.)

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
v1->v2: Further improve the log message, thanks Han.

 lib/hmap.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

Comments

Han Zhou March 26, 2019, 5:55 p.m. UTC | #1
On Tue, Mar 26, 2019 at 9:58 AM Ben Pfaff <blp@ovn.org> wrote:
>
> I was seeing a lot of these messages, including a lot of them suppressed
> by rate-limiting, and I wondered whether any really big messages were
> being suppressed.  By reporting the largest bucket, instead of just every
> large bucket, it becomes more likely that the truly too-large buckets get
> reported.
>
> (The problem I saw was a false alarm.)
>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
> v1->v2: Further improve the log message, thanks Han.
>
>  lib/hmap.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/lib/hmap.c b/lib/hmap.c
> index 1ba4a5716a78..9ee05b6d499b 100644
> --- a/lib/hmap.c
> +++ b/lib/hmap.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2015 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2015, 2019 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -103,6 +103,9 @@ resize(struct hmap *hmap, size_t new_mask, const char *where)
>              tmp.buckets[i] = NULL;
>          }
>      }
> +    int n_big_buckets = 0;
> +    int biggest_count = 0;
> +    int n_biggest_buckets = 0;
>      for (i = 0; i <= hmap->mask; i++) {
>          struct hmap_node *node, *next;
>          int count = 0;
> @@ -112,14 +115,30 @@ resize(struct hmap *hmap, size_t new_mask, const char *where)
>              count++;
>          }
>          if (count > 5) {
> -            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
> -            COVERAGE_INC(hmap_pathological);
> -            VLOG_DBG_RL(&rl, "%s: %d nodes in bucket (%"PRIuSIZE" nodes, %"PRIuSIZE" buckets)",
> -                        where, count, hmap->n, hmap->mask + 1);
> +            n_big_buckets++;
> +            if (count > biggest_count) {
> +                biggest_count = count;
> +                n_biggest_buckets = 1;
> +            } else if (count == biggest_count) {
> +                n_biggest_buckets++;
> +            }
>          }
>      }
>      hmap_swap(hmap, &tmp);
>      hmap_destroy(&tmp);
> +
> +    if (n_big_buckets) {
> +        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
> +        COVERAGE_INC(hmap_pathological);
> +        VLOG_DBG_RL(&rl, "%s: %d bucket%s with 6+ nodes, "
> +                    "including %d bucket%s with %d nodes "
> +                    "(%"PRIuSIZE" nodes total across %"PRIuSIZE" buckets)",
> +                    where,
> +                    n_big_buckets, n_big_buckets > 1 ? "s" : "",
> +                    n_biggest_buckets, n_biggest_buckets > 1 ? "s" : "",
> +                    biggest_count,
> +                    hmap->n, hmap->mask + 1);
> +    }
>  }
>
>  static size_t
> --
> 2.20.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Thanks Ben!

Acked-by: Han Zhou <hzhou8@ebay.com>
Ben Pfaff March 26, 2019, 8:42 p.m. UTC | #2
On Tue, Mar 26, 2019 at 10:55:34AM -0700, Han Zhou wrote:
> On Tue, Mar 26, 2019 at 9:58 AM Ben Pfaff <blp@ovn.org> wrote:
> >
> > I was seeing a lot of these messages, including a lot of them suppressed
> > by rate-limiting, and I wondered whether any really big messages were
> > being suppressed.  By reporting the largest bucket, instead of just every
> > large bucket, it becomes more likely that the truly too-large buckets get
> > reported.
> >
> > (The problem I saw was a false alarm.)
> >
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> > ---
> > v1->v2: Further improve the log message, thanks Han.
>
> Thanks Ben!
> 
> Acked-by: Han Zhou <hzhou8@ebay.com>

Thanks, applied to master.
diff mbox series

Patch

diff --git a/lib/hmap.c b/lib/hmap.c
index 1ba4a5716a78..9ee05b6d499b 100644
--- a/lib/hmap.c
+++ b/lib/hmap.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2015, 2019 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -103,6 +103,9 @@  resize(struct hmap *hmap, size_t new_mask, const char *where)
             tmp.buckets[i] = NULL;
         }
     }
+    int n_big_buckets = 0;
+    int biggest_count = 0;
+    int n_biggest_buckets = 0;
     for (i = 0; i <= hmap->mask; i++) {
         struct hmap_node *node, *next;
         int count = 0;
@@ -112,14 +115,30 @@  resize(struct hmap *hmap, size_t new_mask, const char *where)
             count++;
         }
         if (count > 5) {
-            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
-            COVERAGE_INC(hmap_pathological);
-            VLOG_DBG_RL(&rl, "%s: %d nodes in bucket (%"PRIuSIZE" nodes, %"PRIuSIZE" buckets)",
-                        where, count, hmap->n, hmap->mask + 1);
+            n_big_buckets++;
+            if (count > biggest_count) {
+                biggest_count = count;
+                n_biggest_buckets = 1;
+            } else if (count == biggest_count) {
+                n_biggest_buckets++;
+            }
         }
     }
     hmap_swap(hmap, &tmp);
     hmap_destroy(&tmp);
+
+    if (n_big_buckets) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 10);
+        COVERAGE_INC(hmap_pathological);
+        VLOG_DBG_RL(&rl, "%s: %d bucket%s with 6+ nodes, "
+                    "including %d bucket%s with %d nodes "
+                    "(%"PRIuSIZE" nodes total across %"PRIuSIZE" buckets)",
+                    where,
+                    n_big_buckets, n_big_buckets > 1 ? "s" : "",
+                    n_biggest_buckets, n_biggest_buckets > 1 ? "s" : "",
+                    biggest_count,
+                    hmap->n, hmap->mask + 1);
+    }
 }
 
 static size_t