763342 - Cache hit rate telemetry is wrong

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

AFAICT, this is caused by part 6 of the patchset from bug 722034.

Basically, we are now only counting per-device cache hit/miss telemetry for the case where we don't have to do any revalidation. Thus, HTTP_DISK_CACHE_DISPOSITION and similar per-device telemetry only includes cache hits.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Fix cache hit rate telemetry

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Nick, I didn't write an automated test for this, but you can verify that it is working by browsing around a little bit and then observing that you have positive counts for values 1, 2, 3, and 4 for HTTP_CACHE_DISPOSITION_2 and the other DISPOSITION_2 probes.

I left the max at 5 even though there are only 4 distinct valid values now. I am not sure how much of a difference this makes, but AFAICT it shouldn't hurt.

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 632532 [details] [diff] [review]
Fix cache hit rate telemetry

Wrong bug.

Comment 4

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Attachment: Fix cache hit rate telemetry regression

You can verify that this patch fixes the problem by observing that, in a build before the patch, HTTP_CACHE_DISPOSITION_2 only has counts for the value 1 after surfing. After surfing for a while with the patch, you will see counts for values 1, 2, 3, 4.

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/121f87144909

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/121f87144909