401726 - orphaned moz_places records maybe not being deleted

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Dietrich Ayala (:dietrich)]

from Marco:

i was reading out erasehistoryparanoid, the query after adding annotations is

    NS_LITERAL_CSTRING("DELETE FROM moz_places "
      "WHERE id IN (SELECT h.id FROM moz_places h "
      "LEFT OUTER JOIN moz_historyvisits v ON h.id = v.place_id "
      "LEFT OUTER JOIN moz_bookmarks b ON h.id = b.fk "
      "LEFT OUTER JOIN moz_annos a ON h.id = a.place_id "
      "WHERE v.id IS NULL "
      "AND b.id IS NULL "
      "AND a.expiration = ") +
      nsPrintfCString("%d", nsIAnnotationService::EXPIRE_NEVER) +
      NS_LITERAL_CSTRING(" AND a.id IS NULL "
      "AND SUBSTR(h.url,0,6) <> 'place:')"));

i was noticing the
      "AND a.expiration = ") +
      nsPrintfCString("%d", nsIAnnotationService::EXPIRE_NEVER) +
      NS_LITERAL_CSTRING(" AND a.id IS NULL "

how can expiration be expire_never if there is no annotation (a.id IS NULL)? imho this will not find anything..
maybe it should be  a.expiration != EXPIRE_NEVER, and in a block as
AND (a.id IS NULL OR a.expiration != EXPIRE_NEVER ) ?

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Attachment: ExpireHistoryParanoid is not enough paranoid

i don't know if you want to fix this in another patch, but since i had already touched up the code i attach it here

Comment 2

[Dietrich Ayala (:dietrich)]

Drivers: Requesting blocking, as this is a clear contributor to places performance problems. This query is miswritten, ensuring that no moz_places entries will get deleted from the table at shutdown, causing slow queries, contributing to our overlarge sqlite files, leading to cache overflow, etc.

before:
59mb places.sqlite
83521 records in moz_places
121713 records in moz_historyvisits

apply patch, startup, shutdown

after:
16mb places.sqlite
10281 records in moz_places (-80000!)
121713 records in moz_historyvisits

Basically, expiration was working, but never removing the orphaned records in moz_places, thereby also keeping them indexed as well.

Comment 3

[Dietrich Ayala (:dietrich)]

Comment on attachment 286779 [details] [diff] [review]
ExpireHistoryParanoid is not enough paranoid

Thanks, as mentioned in my previous comment, this fix works.

Note: We need to examine EraseVisits also, as that doesn't seem to be doing it's job either. I'll spin that off to another bug.

Comment 4

[Dietrich Ayala (:dietrich)]

(In reply to comment #2)
> before:
> 59mb places.sqlite
> 83521 records in moz_places
> 121713 records in moz_historyvisits
> 
> apply patch, startup, shutdown
> 
> after:
> 16mb places.sqlite

I left a step out: I did a manual VACUUM after the shutdown, to see what effect it had on file size after expiration was working properly. Users with profiles from <A8 will not get the reduced file size, as incremental vacuum was introduced in that release, and does not affect sqlite files created before then. Users >=A8 will get reduced file size, but not as dramatic as this, since incremental vacuum does not also defragment and repack.

The end result should still be dramatically smaller moz_places tables and file sizes than users are currently experiencing.

Comment 5

[Dietrich Ayala (:dietrich)]

Note that the first shutdown after applying this patch can take a while (up to 30 secs for my 70mb db).

After that first time it's negligible even with my un-vacuumed 70mb db.

Comment 6

[Dietrich Ayala (:dietrich)]

a=mconnor over irc

Checking in toolkit/components/places/src/nsNavHistoryExpire.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryExpire.cpp,v  <--  nsNavHistoryExpire.cpp
new revision: 1.18; previous revision: 1.17
done

Comment 7

[Bill Gianopoulos [:WG9s]]

This seems to cause shutdown hangs under Linux.

Comment 8

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv