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 |
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>
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 --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
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(-)