Closed Bug 1502111 Opened 6 years ago Closed 6 years ago

[Shield] Study: IndexedDB Storage Initialization Errors, beta 64, nightly 65

Categories

(WebExtensions :: Storage, enhancement, P1)

enhancement

Tracking

(firefox64+ fixed, firefox65+ fixed, firefox66+ fixed)

RESOLVED FIXED
Tracking Status
firefox64 + fixed
firefox65 + fixed
firefox66 + fixed

People

(Reporter: rpl, Assigned: rpl)

References

Details

User Story

PHD doc: https://docs.google.com/document/d/1oAJsjFDGov1FiovzkVA1OPM96KgmqL5bh2GkBM99VUQ

Code repo: https://github.com/rpl/storage-init-errors-shield-study

Attachments

(8 files)

In Bug 1474562 we have enabled on Nightly the storage.local API backend to migrate from JSON to indexedDB. During the time the migration has been enabled on Nightly, we have been looking into the telemetry related to the "backend migration results" that we collect and we noticed that for some users the migration is not currently able to complete successfully because of errors raised from IndexedDB when we are opening a database (but before any data is actually stored in the newly opened db). There errors seems likely related to some known QuotaManager issues (e.g. https://bugzilla.mozilla.org/buglist.cgi?bug_id=944918%2C1423917). In order to verify the incidence of these storage initialization issues on non-Nightly channels (where we can't turn on the actual storage.local data migration yet), we need to write a shield study extension which tries to open an IndexedDB database under the same conditions of the storage.local IDB data migrations, and then collect and reports the results.
Assignee: nobody → lgreco
Status: NEW → ASSIGNED
User Story: (updated)
Priority: -- → P1
Attached file GitHub Pull Request (deleted) —
Hi Robert, the attached link is related to a github pull request which contains the current version of the sources of this shield study addon, so that you may be able to more easily add inline comment if/when needed as part of the review (and I'm going to be able to add any additional change that may arise from the review by pushing in that branch). I'm also going to attach separately the extension xpi or zip archive.
Attachment #9020092 - Flags: review?(rhelmer)
The built extension in zip archive format (created from the github repo using web-ext build).
Attachment #9020095 - Flags: review?(rhelmer)
Attachment #9020095 - Flags: review?(rhelmer) → review+
Attachment #9020092 - Flags: review?(rhelmer) → review+
User Story: (updated)
Hi :chutten, The attached markdown contains a data reviewed request related to the "shield-study-addon" telemetry ping recorded by the attached shield study webextension. The telemetry collected by this study extension are actually a subset of the ones described in the Bug 1470213 data review request (but I felt that an explicit and separate data review request was still needed even if the data is the same, as we are recording them into a different telemetry ping). The following are the parts of the study extension that are collecting these data and then recording them in the "shield-study-addon" ping (in case having a look to actual implementation may be helpful to the data review): - https://github.com/rpl/storage-init-errors-shield-study/blob/eda15697310bbd1c59088ca10abf6243ae180e2e/src/privileged/testIDBOpen/api.js#L54-L72 - https://github.com/rpl/storage-init-errors-shield-study/blob/eda15697310bbd1c59088ca10abf6243ae180e2e/src/background.js#L114-L128 In the study extension's github repository I also included a TELEMETRY.md file with some details about the telemetry recorded by the extension: - https://github.com/rpl/storage-init-errors-shield-study/blob/master/docs/TELEMETRY.md
Attachment #9023272 - Flags: review?(chutten)
Comment on attachment 9023272 [details] storage-init-error-shield-study-DataReviewRequest.md Preliminary notes: Thank you for requesting review. Changing the mechanism of collection is indeed criteria for review, so you were right in doing so. (Though, in this case the shield-study-addon pings don't look to contain anything of sufficiently-different character to be treated differently than measurements on the main or event ping, but eh :) Thank you also for the links to prior reviews and code. Interestingly, your questions do not require that you know the name of the failure, just its incidence rates. I presume you wish to see if the character of the failure rates on beta match that of Nightly and that's why you want directly-comparable data. DATA COLLECTION REVIEW RESPONSE: Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate? Yes. See TELEMETRY.md: https://github.com/rpl/storage-init-errors-shield-study/blob/master/docs/TELEMETRY.md#shield-study-addon-pings-specific-to-this-study Is there a control mechanism that allows the user to turn the data collection on and off? Yes. Standard study and Telemetry mechanisms apply. If the request is for permanent data collection, is there someone who will monitor the data over time? N/A, limited to study boundaries. Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under? ** Category 1, Technical. Is the data collection request for default-on or default-off? Default on for specific channels and versions. 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)? Nothing new, but it's worth noting once more that this collection contains the names of DOMExceptions, truncated to 80 characters. Is the data collection covered by the existing Firefox privacy notice? Yes. Does there need to be a check-in in the future to determine whether to renew the data? No, this study will run its course. --- Result: datareview+
Attachment #9023272 - Flags: review?(chutten) → review+
Comment on attachment 9020095 [details] storage-init-errors-shield-study-1.0.0.zip Hi Micheal, Could you sign storage-init-errors-shield-study-1.0.0.zip so that it can be verified on a Beta builds from QA? Thanks in advance for your help.
Flags: needinfo?(mcooper)
I've signed the add-on for QA testing. This add-on is signed with the key for development, and as such can only be used on unbranded builds with 'xpinstall.signatures.dev-root' set to true. QA has been informed of this process change. Once the add-on has QA signoff and Firefox peer review, it can receive final signing for deployment.
Flags: needinfo?(mcooper)
Hi Krupa, follows the QA request related to the Shield Study extension xpi (attachment 9023748 [details], which is signed for QA testing with the key for development, as mentioned in comment 7). Let me know if there are any additional details that are needed and missing in this QA request. ------- QA Request We are the Addons team, and we would would like to request testing help with the "storage-init-errors Shield Study" (storage-init-errors-shield-study@shield.mozilla.org) Project name: storage-init-errors-shield-study Description: A tiny Shield Study which checks that QuotaManagerService/IndexedDB are initializing successfully. Study owner: Luca Greco (lgreco@mozilla.com) Priority: normal Tracking Bug: 1502111 https://bugzilla.mozilla.org/1502111 PHD document: https://docs.google.com/document/d/1oAJsjFDGov1FiovzkVA1OPM96KgmqL5bh2GkBM99VUQ Channels: Beta and Nightly Platform: Firefox Desktop (OSX/Windows/Linux) Locales: any Complexity: Simple one-time passive data collection effort (single branch). Telemetry sent using shield-study-addon Telemetry ping. Testing instructions: https://github.com/rpl/storage-init-errors-shield-study/blob/master/docs/TESTPLAN.md
Needinfo for Matt related to the PHD https://docs.google.com/document/d/1oAJsjFDGov1FiovzkVA1OPM96KgmqL5bh2GkBM99VUQ, to double-check if there is any information missing or not yet complete.
Flags: needinfo?(mgrimes)
Needinfo Krupa on "QA request" added in comment 8.
Flags: needinfo?(kraj)
I'm clearing the needinfo assigned to Krupa, as the QA verification has been started.
Flags: needinfo?(kraj)
Hi Ben and Matt, during the study QA verification, QA has pointed out that this shield study extension is currently auto-uninstalling itself as soon as it has been able to fully run once. This is currently a behavior I implemented on purpose, but I wanted to double-check with you if it may make sense (from a data analysis point of view, as well as from a "shield studies conventions" point of view) to keep the shield study installed and enabled until it is expired, so that it may have the chance to try to run the IndexedDB test (and report back the result) more than once (e.g. one reason that currently comes to my mind could be "to be able to detect if some of the QuotaManagerService/IndexedDb errors detected were transient or consistent across Firefox restarts").
Flags: needinfo?(bmiroglio)
Luca, if it's not too much trouble, it is preferred to keep the extension enabled for the duration of the experiment.
Flags: needinfo?(bmiroglio)
Hi Robert, Do you mind to review this github pull request which includes some small changes to the shield study addon code? (it removes a couple of lines from the background script to leave the study installed until it expires, which is related to comment 13, and it also adds the "hidden" property to the manifest.json file).
Attachment #9025430 - Flags: review?(rhelmer)
The built extension for v.1.0.1 in zip archive format (created from the github pull request using web-ext build).
Attachment #9025432 - Flags: review?(rhelmer)
Comment on attachment 9025432 [details] storage-init-errors-shield-study-1.0.1.zip Hi Micheal, Could you sign storage-init-errors-shield-study-1.0.1.zip with the key for development? so that QA can verify it (while the github pull request which includes these small changes gets peer-reviewed). Thanks in advance!
Flags: needinfo?(mcooper)
Signed for testing.
Flags: needinfo?(mcooper)
Attachment #9025430 - Flags: review?(rhelmer) → review+
Attachment #9025432 - Flags: review?(rhelmer) → review+
We have finished testing the "IndexedDB" experiment. QA’s recommendation: GREEN - SHIP IT Reasoning: - We haven’t found any issues during testing. Testing Summary: - Full test suite: https://testrail.stage.mozaws.net/index.php?/plans/view/13684 Tested Platforms: - Windows 10 x64 - Windows 7 x64 - Mac 10.14 - Mac 10.13 - Ubuntu 14.04 - Arch Linux 4.16 Tested Firefox versions: - Nightly 65 - Beta 64
Since this has received code review r+ and a green QA status, I've signed it with the prod key. It is now ready to ship.
Depends on: 1509921
[Tracking Requested - why for this release]: Study will run in both beta and nightly.
Summary: [Shield] Study: IndexedDB Storage Initialization Errors → [Shield] Study: IndexedDB Storage Initialization Errors, beta 64, nightly 65
Shell or Luca, please request legal review for this study, here: https://bugzilla.mozilla.org/enter_bug.cgi?product=Legal&component=Product%20-%20data
Flags: needinfo?(sescalante)
Flags: needinfo?(lgreco)
filed legal bug 1511197
Flags: needinfo?(sescalante)
Given the timing wouldn't this be beta 65 / nightly 66 at this point?
r+ science So the science looks good enough. Per the comment above (c8) and studySetup.js this appears to be a single branch study to compare against our standard baseline. It should be noted that this has less scientific validity than a true control/variant a/b test which may be preferable in the future or other circumstances to lessen the potential for assignment/temporal asymmetries. Working on the recipe. I'm going to target Beta 64+ and Nightly 65+ since we're in that awkward launch window. (Clearing resolved NI's above)
Flags: needinfo?(mgrimes)
Flags: needinfo?(lgreco)
Ryan, the team would like to launch this study this week. Can we get your approval? Thanks.
Flags: needinfo?(ryanvm)
Or Julien from Release Management...
Flags: needinfo?(jcristau)
a=me to run on Beta64+ and Nightly65+
Flags: needinfo?(ryanvm)
Flags: needinfo?(jcristau)
Forgot to update, we're live on this since yesterday.
Going to call this fixed for 64 so it drops off my radar for release, as a beta/nightly study.
Josh, please pause enrollment on this study, but leave it running, per Ben's email.
Flags: needinfo?(jgaunt)
Recipes 655 and 656 have been paused accordingly.
Flags: needinfo?(jgaunt)
Recipes 655 and 656 have been disabled.

Marking "resolved,fixed", as this shield study has been completed.

Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: