Bad Network.Shill.*.ExpiredLeaseLengthSeconds Construction Parameters |
||||||
Issue descriptionChrome Version: HEAD Chrome OS Version: HEAD Chrome OS Platform: ALL Network info: N/A See also: crbug.com/736416 https://chromium.googlesource.com/chromiumos/platform/shill/+/master/metrics.cc#260 This has set the number of buckets to 604800 (one per second), which is just a wee bit large. It's clamped by Chrome to 16384 but ideally would be only 100 or so. Even at 16384 it's 64KiB of memory to store it. Or remove it if not being used.
,
Jun 30 2017
Might want to get someone who's actually on the Chrome OS team involved. I'm not on Chrome OS. Perhaps try cernkee@? I don't have permissions to edit owner, etc.
,
Jun 30 2017
,
Jun 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/a3d859782eb0834017f27eb147ba110a28ed2a66 commit a3d859782eb0834017f27eb147ba110a28ed2a66 Author: Ben Chan <benchan@chromium.org> Date: Fri Jun 30 23:02:49 2017 shill: fix count histogram minimums The Chrome metrics library requires the minimum of a count histogram metric to be >= 1, and always creates an underflow bucket for values < 1. This CL fixes a few shill metrics to define their count histogram minimum as 1 instead of 0. The maximum and the number of bucket of these metrics remain unchanged. BUG= chromium:738122 TEST=Run unit tests. Change-Id: Ied40fb2fa8eb7fc36a633043161dcd80cf52c709 Reviewed-on: https://chromium-review.googlesource.com/557296 Commit-Ready: Ben Chan <benchan@chromium.org> Tested-by: Ben Chan <benchan@chromium.org> Reviewed-by: Brian White <bcwhite@chromium.org> Reviewed-by: Kevin Cernekee <cernekee@chromium.org> [modify] https://crrev.com/a3d859782eb0834017f27eb147ba110a28ed2a66/metrics.cc
,
Jun 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/a3d859782eb0834017f27eb147ba110a28ed2a66 commit a3d859782eb0834017f27eb147ba110a28ed2a66 Author: Ben Chan <benchan@chromium.org> Date: Fri Jun 30 23:02:49 2017 shill: fix count histogram minimums The Chrome metrics library requires the minimum of a count histogram metric to be >= 1, and always creates an underflow bucket for values < 1. This CL fixes a few shill metrics to define their count histogram minimum as 1 instead of 0. The maximum and the number of bucket of these metrics remain unchanged. BUG= chromium:738122 TEST=Run unit tests. Change-Id: Ied40fb2fa8eb7fc36a633043161dcd80cf52c709 Reviewed-on: https://chromium-review.googlesource.com/557296 Commit-Ready: Ben Chan <benchan@chromium.org> Tested-by: Ben Chan <benchan@chromium.org> Reviewed-by: Brian White <bcwhite@chromium.org> Reviewed-by: Kevin Cernekee <cernekee@chromium.org> [modify] https://crrev.com/a3d859782eb0834017f27eb147ba110a28ed2a66/metrics.cc
,
Oct 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/b9f775603905f4cc3d87aae395748dd6df003888 commit b9f775603905f4cc3d87aae395748dd6df003888 Author: Ben Chan <benchan@chromium.org> Date: Thu Oct 05 10:04:16 2017 shill: reduce number of bucket for *.ExpiredLeaseLengthSeconds metrics shill records *.ExpiredLeaseLengthSeconds metrics for various network types (e.g. Ethernet, WiFi). Each of these metrics is recorded as a count histogram and tracks the DHCP lease length for a network of a particular type when the lease expires without the DHCP client being able to renew it. The maximum of the count histogram for *.ExpiredLeaseLengthSeconds is set to 604800 (the total number of seconds in 7 days) while the number of bucket is set to 604801, which far exceeds the maximum bucket count (16384) specified by the Chrome metrics library. [1] states: "By default count histograms bucket sizes scale exponentially so you can get fine granularity when the numbers are small yet still reasonable resolution for larger numbers. The macros default to 50 buckets (or 100 buckets for histograms with wide ranges) which is appropriate for most purposes." This CL reduces the bucket count for *.ExpiredLeaseLengthSeconds to 100 as recommended by [1]. [1] https://chromium.googlesource.com/chromium/src.git/+/HEAD/tools/metrics/histograms/README.md BUG= chromium:738122 TEST=Run unit tests. Change-Id: I0bf49e5d1e316efebe6de6ea5b078588c192b57b Reviewed-on: https://chromium-review.googlesource.com/557297 Commit-Ready: Ben Chan <benchan@chromium.org> Tested-by: Ben Chan <benchan@chromium.org> Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org> [modify] https://crrev.com/b9f775603905f4cc3d87aae395748dd6df003888/metrics.cc
,
Oct 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/aosp/platform/system/connectivity/shill/+/749dbc8d7d1d646ab560c21e8ab3a1709d2e3021 commit 749dbc8d7d1d646ab560c21e8ab3a1709d2e3021 Author: Ben Chan <benchan@chromium.org> Date: Sat Oct 07 04:31:11 2017 shill: rename metric 'ExpiredLeaseLengthSeconds' to 'ExpiredLeaseLengthSeconds2' CL:557297 changed the number of buckets for the 'ExpiredLeaseLengthSeconds' metric. That would lead to confusing display of samples collected before and after the change. To avoid that, this CL renames 'ExpiredLeaseLengthSeconds' to 'ExpiredLeaseLengthSeconds2'. BUG= chromium:738122 TEST=Run unit tests. Change-Id: I26bc15fca9602f7c6a98a60c96852d81a07f3e42 Reviewed-on: https://chromium-review.googlesource.com/703679 Commit-Ready: Ben Chan <benchan@chromium.org> Tested-by: Ben Chan <benchan@chromium.org> Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org> [modify] https://crrev.com/749dbc8d7d1d646ab560c21e8ab3a1709d2e3021/metrics.cc [modify] https://crrev.com/749dbc8d7d1d646ab560c21e8ab3a1709d2e3021/device_unittest.cc
,
Oct 7 2017
,
Oct 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f6731b289011598147edaa2c66ceb345c619eef9 commit f6731b289011598147edaa2c66ceb345c619eef9 Author: Ben Chan <benchan@chromium.org> Date: Wed Oct 11 00:26:07 2017 Add Network.Shill.*.ExpiredLeaseLengthSeconds2. Network.Shill.*.ExpiredLeaseLengthSeconds have deprecated and superceded by 'Network.Shill.*.ExpiredLeaseLengthSeconds2 due to change in the number of buckets since Chrome OS build 10010.0.0. Bug: 738122 Change-Id: Ib84de68d48a0cacef0f8e6a1061f4add724453c6 Reviewed-on: https://chromium-review.googlesource.com/710576 Commit-Queue: Ben Chan <benchan@chromium.org> Commit-Queue: Ilya Sherman <isherman@chromium.org> Reviewed-by: Kevin Cernekee <cernekee@chromium.org> Reviewed-by: Ilya Sherman <isherman@chromium.org> Cr-Commit-Position: refs/heads/master@{#507838} [modify] https://crrev.com/f6731b289011598147edaa2c66ceb345c619eef9/tools/metrics/histograms/histograms.xml
,
Jan 22 2018
,
Jan 23 2018
,
Sep 13
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bcwh...@chromium.org
, Jun 29 2017Also, some other parameters in that same file are incorrect: const int Metrics::kMetricLinkMonitorErrorCountMin = 0; const int Metrics::kMetricLinkMonitorErrorCountMax = LinkMonitor::kFailureThreshold; const int Metrics::kMetricLinkMonitorErrorCountNumBuckets = LinkMonitor::kFailureThreshold + 1; "Min" should be 1. "NumBuckets" should be +2. A "min" of "0" is automatically changed to "1" at run-time but the number of buckets isn't incremented.