1671932 - Asynchronous non-blocking temporary storage initialization tolerating broken origins

Status: ASSIGNED

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

Description

[Jan Varga [:janv]]

We are currently facing two major problems:

1. Temporary storage initialization errors
2. Slow temporary storage initialization in some cases

The temporary storage initialization must be currently successfully finished before a quota client can access files on disk (doesn't apply for the persistent repository).

If we could make the initialization asynchronous then we would mitigate the two major problems in a big way.

I'm still not 100% sure if it is feasible, but so far it looks good.

Comment 1

[Jan Varga [:janv]]

We are now fully working on this, it's our top priority.

Comment 2

[Simon Giesecke [:sg] [he/him]]

I think the description and summary should make one thing explicit that is currently implicit only: At the moment, storage initialization succeeds or fails atomically for all quota clients and origins. This bug intends to be change that so the success/failure is partial (per origin?). This is an important point, because this could be implemented (addressing problem number 1) independently from making initialization asynchronous (which addresses problem number 2). However, this bug specifically suggests to make both changes in one go.

Comment 3

[Jan Varga [:janv]]

That's true (partially), but it can't be easily implemented independently. I'll try to provide more details about that since I'm focusing on this more right now.

Comment 4

[Jan Varga [:janv]]

The main point here is that we currently allow only 100% accurate quota management. We want to change it so we will allow "inaccurate" usage tracking for the time the temporary storage is being initialized. So quota clients will be able to use storage even if the initialization is not finished yet. When the initialization finishes and we see that more data has been written than it would be allowed with synchronous initialization, we will evict some origins. After that we will have 100% accurate quota management/usage tracking again.
Once we do necessary changes for initializing origins asynchronously, then they naturally won't be able to break entire temporary storage initialization. The quota management will stay in "not 100% accurate" mode since we couldn't get exact usage for broken origins, The broken origins will stay uninitialized with some files on disk and they won't be included in overall usage calculations. The fact that we leave some extra files on disk which are not included in the usage calculations shouldn't be a big problem. We already had to make an exception for LSNG which tracks only logical size of the database. So the total physical size of all files doesn't have to match the usage we internally use for quota checks. We only allow to use 50% of free disk space anyway, so there should be a lot of space in reserve.

Comment 5

[Simon Giesecke [:sg] [he/him]]

As part of this, the comment in QuotaManager::CollectOriginsForEviction should be addressed, see https://phabricator.services.mozilla.com/D101182#inline-580859.

Comment 8

[Daniele "Mte90" Scasciafratte]

As I wrote on 1741865 if you need to profile or to test some firefox patched let me know as I have that issue only on my workstation since 2 years...

Comment 10

[Jan Varga [:janv]]

Attachment: Bug 1671932 - Rename EnsureTemporaryStorageIsInitialized to EnsureTemporaryStorageIsInitializedInternal; r=#dom-storage

One of the goals of bug 1671932 is to call EnsureTemporaryStorageIsInitialized
only from InitTemporaryStorageOp. Calling from other places including quota
clients will be disallowed by changing the method to a private method. The
private nature of the method should be emphasized by adding the Internal
suffix.

Changes done in this patch:

• IsTemporaryStorageInitialized renamed to
  IsTemporaryStorageInitializedInternal
• EnsureTemporaryStorageIsInitialized renamed to
  EnsureTemporaryStorageIsInitializedInternal

Depends on D186781

Comment 11

[Phabricator Automation]

Comment on attachment 9353390 [details]
Bug 1671932 - Rename EnsureTemporaryStorageIsInitialized to EnsureTemporaryStorageIsInitializedInternal; r=#dom-storage

Revision D188332 was moved to bug 1808294. Setting attachment 9353390 [details] to obsolete.

Comment 12

[footmenkingdom]

Tracked by Reddit: https://old.reddit.com/r/firefox/comments/17u92cj/firefox_first_startup_high_disk_usage_io_wait/

This affects user experience and 2023 user experience is the key.

Comment 13

[Jan Varga [:janv]]

We are working really hard on this bug.