Closed Bug 1867095 Opened 3 months ago Closed 3 months ago

Cached quota information is not used when last access time information mismatches

Categories

(Core :: Storage: Quota Manager, defect, P1)

defect

Tracking

()

VERIFIED FIXED
122 Branch
Tracking Status
relnote-firefox --- 120+
firefox120 + verified
firefox121 + verified
firefox122 --- verified

People

(Reporter: janv, Assigned: janv)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

This seems to be causing persistent startup slowdown in FF 120.

Attachment #9365912 - Attachment description: Bug 1867095 - Make it possible to used cached quota information even when last access time information mismatches; r=#dom-storage → Bug 1867095 - Make it possible to use cached quota information even when last access time information mismatches; r=#dom-storage
Summary: Make it possible to used cached quota information even when last access time information mismatches → Make it possible to use cached quota information even when last access time information mismatches
Summary: Make it possible to use cached quota information even when last access time information mismatches → Cached quota information is not used when last access time information mismatches
Pushed by jvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/451a622b2028
Make it possible to use cached quota information even when last access time information mismatches; r=dom-storage-reviewers,asuth
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch

Comment on attachment 9365912 [details]
Bug 1867095 - Make it possible to use cached quota information even when last access time information mismatches; r=#dom-storage

Taking in 121 beta 5 as we intend to take it in our 120 dot release.

Attachment #9365912 - Flags: approval-mozilla-beta+

Comment on attachment 9365912 [details]
Bug 1867095 - Make it possible to use cached quota information even when last access time information mismatches; r=#dom-storage

Unsetting the beta approval to allow Jan to provide more information and especially STRs.

Flags: needinfo?(jvarga)
Attachment #9365912 - Flags: approval-mozilla-beta+

The fix can be tested by going to about:telemetry after startup and checking QM_QUOTA_INFO_LOAD_TIME_V0. The value went down from ~2000 to ~100 (msecs) for me after the fix. I have ~30K files in my profile.

Note that the value can be one time higher after updating FF (when build id changes).

Flags: needinfo?(jvarga)

Comment on attachment 9365912 [details]
Bug 1867095 - Make it possible to use cached quota information even when last access time information mismatches; r=#dom-storage

Beta/Release Uplift Approval Request

  • User impact if declined: Users would still observe persistent startup delay caused by full storage initialization.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce:
  1. Run a build without the fix with some existing profile which contains many files (at least several thousands) in <profile>/storage (a profile which is known to cause persistent startup delay)
  2. Wait several seconds for storage initialization to finish
  3. Go to about:telemetry and check QM_QUOTA_INFO_LOAD_TIME_V0. The value should be greater than 1000 msecs
  4. Quit the app
  5. Run a build with the fix included with the same profile
  6. Wait several seconds for storage initialization to finish
  7. Quit the app
  8. Run the same build with the fix included with the same profile
  9. Wait a second
  10. Go to about:telemetry and check QM_QUOTA_INFO_LOAD_TIME_V0. The value should be an order of magnitude less than the previous value
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Very simple and safe patch
  • String changes made/needed: None
  • Is Android affected?: Yes
Attachment #9365912 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Attachment #9365912 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]
Blocks: 1867035
Attached image Telemetry example

Managed to reproduce the issue on Firefox 120.0. "QM_QUOTA_INFO_LOAD_TIME_V0" value was above 2500.
The issue is fixed on Firefox 122.0a1 (2023-11-29) and on Firefox 121.0b5 on macOS 11.6. After the second restart the value of "QM_QUOTA_INFO_LOAD_TIME_V0" dropped to 20.

Comment on attachment 9365912 [details]
Bug 1867095 - Make it possible to use cached quota information even when last access time information mismatches; r=#dom-storage

Approved for 120.0.1 dot release

Attachment #9365912 - Flags: approval-mozilla-release+
Duplicate of this bug: 1867035

Verified as fixed on Firefox 120.0.1 on macOS 11.6.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+

Is this a regression by bug 1858067? (Posting here as bug 1867035 is duped.)

Keywords: regression
Regressed by: 1858067

Yeah, it seems that's it. There was a patch which cleaned up the interaction of operations for saving origin access time and shutdown, https://phabricator.services.mozilla.com/D187877. Before the patch, the operation could sometimes fail due to ongoing shutdown (likely causing a failure during loading of quota information after next startup, causing a startup delay). The patch fixed that, so the access time was then reliably stored for all origins even if shutdown was ongoing. However, the additional IO during shutdown caused that the improvements for reducing shutdown time done in bug 1733107 were negated, so we decided to avoid saving of origin access time if shutdown already started. Unfortunately the patch for bug 1858067 done that only for metadata files and not for cached information. Ideally, we would have a test for catching regressions like this. Automatic watching of telemetry might help, but it usually takes several days to see noticeable changes in collected data.

You need to log in before you can comment on or make changes to this bug.