1296591 - Bound persistent storage by total quota

Status: RESOLVED DUPLICATE of bug 1298329

(Core :: DOM: Core & HTML, defect, P3)

Description

[Anne (:annevk)]

Rather than what we have done in bug 1119462 we should still make sure persistent storage is bound by the total quota we allocate for the browser (currently around 50% of the disk). Otherwise sites can fill up the entire disk (and Firefox will end up getting the blame by the system most likely).

Comment 1

[Andrew Overholt [:overholt]]

I thought we already did this. Jan?

Comment 2

[Jan Varga [:janv]]

No, we didn't. Persistent storage is not limited at all currently.

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

I think we should consider this before shipping persist() function.

Comment 4

[Tom Tung [:tt, :ttung]]

Hi Jan,

I have few questions about this, could you help me to figure them out? Thanks!
 
1. Should we bound persistent storage with the same group quota(groupLimit) and total quota(globalLimit) as non-persistent storage? I mean if we have different limit for different type of storage may cause failure for storage.persist() or revertToNonPersist(..).

2. I intend to reuse originInfo to track persistent storage's usage and also groupInfo if 1 is needed. 
I will have following changes:
   - Add a flag in originInfo to indicate Persistent and few functions (Persist(), ... etc)
           - Create originInfo for all storage types(temporary , default, persistent[maybe not this one?]) to track their quota
           - Prevent Persistent's originInfo from being killed as eviction is called
           - Prompt in CheckTemporaryStorageLimits() if Persistent storage is close to total quota(90%?)
   - Use MaybeUpdateSize() to check whether current total usage for persistent storage is under total limit or group limit(if needed).
           - Modify IsQuotaEnforced() and few conditions to allow persistent storage to be checked in MaybeUpdateSize()

Besides, I need to rebase the code in bug 1298329, too. Do you have any other ideas or concerns about this?

Comment 5

[Jan Varga [:janv]]

This is actually quite a big architecture change, I can't answer your questions right now, I have to think about it first.

Comment 6

[Jan Varga [:janv]]

Ok, here's my proposal:
1. for <profile>/storage/default, we will apply the same limits (group and global) for persisted origins as we do for temporary
- I believe we have to apply the group limit too, otherwise when a persisted origin flips back to temporary, we get into a bad state
- In the end, the only difference between persistent and temporary will be how we do origin eviction when we hit the global limit. Simply said, persisted origins won't participate in origin eviction.

The question is, is 2GB group limit enough for persistent origins ? I guess it's not. We plan to increase the global limit from 50% of free disk space to 90%, maybe we have to do something with the group limit too.

2. for <profile>/permanent, we won't change anything, that storage is intended for chrome, about:home and the non-standard indexedDB.open(name, { storage: persistent })
- the non-standard stuff will be removed once we have fully working estimate(), persist(), .persisted

Anne, do you have any suggestions for the group limit ?

Comment 7

[Anne (:annevk)]

I think when we last discussed this we were no longer sure whether "group" (that's what puts mozilla.com and www.mozilla.com together?) is a concept we want to keep around for storage. Just using origins might be fine and would actually help with some cases so folks don't have to add their domain to the public suffix registry to get more storage for games they host and such.

If we cannot remove the group concept or don't want to I suggest we just clear groups that are larger than the group limit once all their origins are back to "best-effort" (that's how the standard calls "temporary").

Comment 8

[Jan Varga [:janv]]

(In reply to Anne (:annevk) from comment #7)
> I think when we last discussed this we were no longer sure whether "group"
> (that's what puts mozilla.com and www.mozilla.com together?) is a concept we
> want to keep around for storage. Just using origins might be fine and would
> actually help with some cases so folks don't have to add their domain to the
> public suffix registry to get more storage for games they host and such.
>

Ok, but if we really decide to do remove the group limit, I'd like to do it separately.
 
> If we cannot remove the group concept or don't want to I suggest we just
> clear groups that are larger than the group limit once all their origins are
> back to "best-effort" (that's how the standard calls "temporary").

That's acceptable to me, but we don't have to delete entire groups, we can just start deleting origins in the group one by one using the LRU policy.

Ok Tom, I think you now have enough information.

Comment 9

[Jan Varga [:janv]]

Tom, if you still have questions regarding implementation, just ask here.

Comment 10

[Anne (:annevk)]

I agree with all your points. I filed bug 1305665 to discuss removal of group limits.

Comment 11

[Tom Tung [:tt, :ttung]]

(In reply to Jan Varga [:janv] from comment #9)
> Tom, if you still have questions regarding implementation, just ask here.
Thanks, Jan.

Just make sure that I understand. Please correct me if my understanding is wrong.

1. In this bug, we'll only apply the same global limit for persistent and "best-effort" storage. 

2. If we revert the persistent storage back to best-effort storage and make its group usage larger than group limit, we'll delete the origins' storage among the same group until their total group usage are under group limit.

3. We will remove group limit in bug 1305665.

Also, to achieve 1 and 2, I would like to use originInfo to track persistent storage's quota (to track its usage and access time).

My questions are listed below:
- Shall we update the access time when we revert persistent to non-persistent? We decided to update access time because we afraid it might be deleted immediately. However, if we use originInfo to track persistent storage, we won't worry about that. Their access time will be updated when user use it.  

> 2. for <profile>/permanent, we won't change anything, that storage is
- Do we need to include their usage when we calculate the persistent's usage?

Comment 12

[Jan Varga [:janv]]

(In reply to Tom Tung [:tt] from comment #11)
> (In reply to Jan Varga [:janv] from comment #9)
> > Tom, if you still have questions regarding implementation, just ask here.
> Thanks, Jan.
> 
> Just make sure that I understand. Please correct me if my understanding is
> wrong.
> 
> 1. In this bug, we'll only apply the same global limit for persistent and
> "best-effort" storage.

Yeah, basically for all origins in storage/default and storage/temporary.
 
> 
> 2. If we revert the persistent storage back to best-effort storage and make
> its group usage larger than group limit, we'll delete the origins' storage
> among the same group until their total group usage are under group limit.

Yes, something like we do in CheckTemporaryStorageLimits(), that's the case
when we reinitialized storage and we need to check storage with recalculated
limits.

> 
> 3. We will remove group limit in bug 1305665.

More precisely, we'll remove the group limit, but at the same time we will
introduce an origin limit. But I don't think you have to worry about it right
now.

> 
> Also, to achieve 1 and 2, I would like to use originInfo to track persistent
> storage's quota (to track its usage and access time).

Yeah, persisted origins in storage/default will have OriginInfo just like 
non persisted ones.

> 
> My questions are listed below:
> - Shall we update the access time when we revert persistent to
> non-persistent? We decided to update access time because we afraid it might
> be deleted immediately. However, if we use originInfo to track persistent
> storage, we won't worry about that. Their access time will be updated when
> user use it.  

Yeah, I think we don't need to update it when the revert happens.

> 
> > 2. for <profile>/permanent, we won't change anything, that storage is
> - Do we need to include their usage when we calculate the persistent's usage?

Do you mean OriginInfo->mUsage ?
There should be no OriginInfo for storage/permanent

Comment 13

[Tom Tung [:tt, :ttung]]

I would like to do it on bug 1298329 so that I can add some test cases for that.

Comment 14

[Jan Varga [:janv]]

Ok.

Comment 15

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Close as duplicate, since Tom has implemented the scope in bug 1298329.