1120945 - HTTP cache v2: maximum number of entries is limited to 13106 on FAT32

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Michal Novotny [:michal]]

There is a limit of 65534 directory entries in a single directory on FAT32. Filename of every cache entry file occupies 5 directory entries so the total limit is 13106 files in a directory. We seem to hit this limit since the default maximum cache size is 350MB which gives an average 27kB per entry and I see an average between 15-20kB in my profiles.

When there is no space for the new file nsILocalFile::OpenNSPRFileDesc() returns NS_ERROR_FILE_NO_DEVICE_SPACE. When we get this error we should evict some existing entry and try to create the file again.

Comment 1

[Michal Novotny [:michal]]

Attachment: fix

Comment 2

[Jason Duell]

After chatting with Michal I think our plan is to test this manually on some windown/fat32 box (can you do it Honza?).  Automated test seems like a lot of work for what should be a simple patch.

Comment 3

[Honza Bambas (:mayhemer)]

Filed bug 1125164

Comment 4

[Honza Bambas (:mayhemer)]

Comment on attachment 8549597 [details] [diff] [review]
fix

Review of attachment 8549597 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, this could work.  Thanks.

Comment 5

[Michal Novotny [:michal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/17934cf98597

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/17934cf98597

Comment 7

[Michal Novotny [:michal]]

Comment on attachment 8549597 [details] [diff] [review]
fix

Approval Request Comment
[Feature/regressing bug #]: the bug is present in cache v2 from the beginning
[User impact if declined]: new entries are not cached on FAT32 when the entry directory is full
[Describe test coverage new/current, TreeHerder]: tested manually
[Risks and why]: low
[String/UUID change made/needed]: none

Comment 8

[Jason Duell]

> new entries are not cached on FAT32 when the entry directory is full

+1 from the peanut gallery for uplift.  The cache not caching new entries means that HTTP caching is basically broken for all FAT32 users (of which there are many on Windows XP).

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Michal, could you provide a way for QA to reproduce this bug to test the fix? Thanks

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bc4ccdc56652
https://hg.mozilla.org/releases/mozilla-beta/rev/608e290ece63

Comment 11

[Jason Duell]

Sylvestre: I'm guessing the way to check is to take a build w/o the fix, run it on a FAT32 profile, browse until you hit the cache file limit (about:cache can tell you how many cache entries are on disk if you click "list cache entries" in the "Disk" section), and then you should be able to verify that going to a new URI doesn't cause a new cache entry to get made (by searching through the list of cache entries).

That said, I'm guessing Michal has done this manually himself already while discovering/fixing the bug, so I don't know if it's worth your time to reproduce it.

Comment 12

[Michal Novotny [:michal]]

Filling the cache might take a while, but you can copy the entry files from another profile. I would also recommend to remove index (cache2/index and cache2/index.log) before you start FF after you've copied the entry files to the profile.

Comment 13

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

(In reply to Jason Duell [:jduell] (needinfo? me) from comment #11)
> Sylvestre: I'm guessing the way to check is to take a build w/o the fix, run
> it on a FAT32 profile, browse until you hit the cache file limit
> (about:cache can tell you how many cache entries are on disk if you click
> "list cache entries" in the "Disk" section), and then you should be able to
> verify that going to a new URI doesn't cause a new cache entry to get made
> (by searching through the list of cache entries).
> 
> That said, I'm guessing Michal has done this manually himself already while
> discovering/fixing the bug, so I don't know if it's worth your time to
> reproduce it.

Given that this seems rather intensive to test and was manually verified by Michal, we'll likely not going to get to it in Firefox 36 due to intense work in other areas, so removing the keyword for now.

Thank you for taking the time to provide testing info, we always appreciate that!

Comment 14

[Honza Bambas (:mayhemer)]

Also see bug 1125164 comment 8.