Closed Bug 867401 Opened 7 years ago Closed 7 years ago
remove hash checksums from saved pings
After finally picking out the right message from the spew of messages in the console, I see why trying to save pings in profile-before-change2 was not working well. We save sha<mumble> hashes of ping payloads as checksums in the saved pings. Works great, except that the bits of security/manager/ssl/ responsible for implementing that hash shuts down at profile-before-change. So if we're saving pings in profile-before-change2, we ask for an instance of nsICryptoHash and an exception is thrown, which totally kills the process of saving pings. We're not really using the checksum for anything besides a quick check that the saved ping didn't get corrupted, so we could ditch it without loss of functionality.
FWIW, the READ_SAVED_PING_SUCCESS histogram indicates a 99.995% success rate for reading saved histograms across *all* channels. We don't have any numbers on whether that's due to bad checksums or something else, but that's good enough that I would feel comfortable removing any checksumming and just letting the JSON syntax checker catch problems.
Generating checksums means relying on nsICryptoHash, which is unavailable after profile-before-change. We'd like to save pings after profile-before-change. Hence, don't checksum. Over 99.99% of our pings are read successfully across all channels, so checksumming isn't really buying us anything.
Attachment #744112 - Flags: review?(vdjeric)
can we just compress them instead?
(In reply to Taras Glek (:taras) from comment #3) > can we just compress them instead? Compress what? The entire saved ping? How is that relevant to this bug?
Comment on attachment 744112 [details] [diff] [review] don't generate checksums for saved pings Looks good to me. Can you just make sure Metrics isn't using the checksum field for anything?
Attachment #744112 - Flags: review?(vdjeric) → review+
(In reply to Vladan Djeric (:vladan) from comment #5) > Comment on attachment 744112 [details] [diff] [review] > don't generate checksums for saved pings > > Looks good to me. Can you just make sure Metrics isn't using the checksum > field for anything? it isn't. (In reply to Nathan Froyd (:froydnj) from comment #4) > (In reply to Taras Glek (:taras) from comment #3) > > can we just compress them instead? > > Compress what? The entire saved ping? How is that relevant to this bug? 09:47 < vladan> froydnj: i think Taras is saying use "compression as a checksum that will be available late during shutdown" We should reduce disk footprint of pings too. 2birds, 1 stone sort of thing
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.