1460305 - DNS cache: add telemetry for evicting non-expired cache entries

Status: RESOLVED FIXED

(Core :: Networking: DNS, enhancement, P1)

Description

[Daniel Stenberg [:bagder]]

Telemetry data for DNS_CLEANUP_AGE shows that DNS cache entries are evicted fairly early; in the order of 16% are nuked within the first minute.

This existing data does not take into account if the cached entries are still valid or not when evicted. This new telemetry probe, DNS_PREMATURE_EVICTION, is very similar to  DNS_CLEANUP_AGE but will only count entries that aren't expired. Entries that are prematurely evicted from the cache due to cache size.

This new probe is meant to be used to evaluate what gains that can be had by enlarging the default DNS cache size (number of entries).

Comment 1

[Daniel Stenberg [:bagder]]

Attachment: bug 1460305 - add DNS_PREMATURE_EVICTION telemetry probe

Counts age of non-expired DNS cache entries being evicted.

Review commit: https://reviewboard.mozilla.org/r/242776/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/242776/

Comment 2

[Patrick McManus [:mcmanus]]

Comment on attachment 8974435 [details]
bug 1460305 - add DNS_PREMATURE_EVICTION telemetry probe

https://reviewboard.mozilla.org/r/242776/#review248676

friendly reminder you also need a data collection review to add the new telem

::: netwerk/dns/nsHostResolver.cpp:1682
(Diff revision 1)
>                  // record the age of the entry upon eviction.
>                  TimeDuration age = TimeStamp::NowLoRes() - head->mValidStart;
>                  Telemetry::Accumulate(Telemetry::DNS_CLEANUP_AGE,
>                                        static_cast<uint32_t>(age.ToSeconds() / 60));
> +                if (head->CheckExpiration(TimeStamp::Now()) !=
> +                    nsHostRecord::EXP_EXPIRED) {

let's make this != VALID instead of == EXPIRED.. it could go either way, but != VALID is a little more conservative in the face of changing GRACE (which seems to also be a possibility now that we have reliable ttls)

::: toolkit/components/telemetry/Histograms.json:3240
(Diff revision 1)
> +    "record_in_processes": ["main"],
> +    "expires_in_version": "never",
> +    "kind": "exponential",
> +    "high": 1440,
> +    "releaseChannelCollection": "opt-out",
> +    "alert_emails": ["[e-mail]", "[e-mail]"],

pretty sure you mean bagder not mcmanus :)

Comment 3

[Daniel Stenberg [:bagder]]

Comment on attachment 8974435 [details]
bug 1460305 - add DNS_PREMATURE_EVICTION telemetry probe

https://reviewboard.mozilla.org/r/242776/#review248676

> let's make this != VALID instead of == EXPIRED.. it could go either way, but != VALID is a little more conservative in the face of changing GRACE (which seems to also be a possibility now that we have reliable ttls)

I figured it was the other way around. VALID and GRACE are both still non-expired so I did the check for != EXPIRED. Shouldn't evicted entries still in the grace period still count as "non-expired" ?

> pretty sure you mean bagder not mcmanus :)

I blame my copy and paste fingers!

Comment 4

[Patrick McManus [:mcmanus]]

Comment on attachment 8974435 [details]
bug 1460305 - add DNS_PREMATURE_EVICTION telemetry probe

https://reviewboard.mozilla.org/r/242776/#review248710

Comment 5

[Daniel Stenberg [:bagder]]

Attachment: data collection form for this new telemetry probe

Comment 6

[Daniel Stenberg [:bagder]]

Attachment: 0001-bug-1460305-add-DNS_PREMATURE_EVICTION-telemetry-pro.patch

Fixed the telemetry email address.

Kept the != EXPIRED logic to have the entries in the grace period included in this counter basically because they are counted as "still in the cache" by the code during that period as well.

Comment 7

[François Marier [:francois]]

Comment on attachment 8974618 [details]
data collection form for this new telemetry probe

1) Is there or will there be **documentation** that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes, in Histograms.json.

2) Is there a control mechanism that allows the user to turn the data collection on and off?

Yes, telemetry setting.

3) If the request is for permanent data collection, is there someone who will monitor the data over time?**

Yes, Daniel Stenberg.

4) Using the **[category system of data types](https://wiki.mozilla.org/Firefox/Data_Collection)** on the Mozilla wiki, what collection type of data do the requested measurements fall under?  **

Category 1.

5) Is the data collection request for default-on or default-off?

Default on, all channels.

6) Does the instrumentation include the addition of **any *new* identifiers** (whether anonymous or otherwise; e.g., username, random IDs, etc.  See the appendix for more details)?

No.

7) Is the data collection covered by the existing Firefox privacy notice?

Yes.

8) Does there need to be a check-in in the future to determine whether to renew the data?

No, permanent.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/07140868fd01
Add DNS_PREMATURE_EVICTION telemetry probe. r=mcmanus

Comment 9

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/07140868fd01