Closed Bug 617779 Opened 9 years ago Closed 9 years ago

Downgrading from places branch to trunk wrongly assumes the database is corrupt.

Categories

(Toolkit :: Places, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- beta8+
status1.9.2 --- unaffected

People

(Reporter: matjk7, Assigned: mak)

References

Details

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (Windows NT 6.1; rv:2.0b8pre) Gecko/20101208 Firefox/4.0b8pre
Build Identifier: 

This is probably bug 423126 striking again.

Reproducible: Always

Steps to Reproduce:
1.Create some bookmarks on trunk
2.Switch to a places branch build
3.Switch back to trunk build
Actual Results:  
All favicons of bookmarks are lost.

Expected Results:  
All favicons of bookmarks are kept.
blocking2.0: --- → ?
We aren't nuking any tables though...  Would you be willing to send me your places.sqlite file?  If not, I can figure out some things to ask and have you run it locally.
The other fun question is if you have Sync enabled or not.  If you do, there's an issue with Sync and the Places branch (being tracked in bug 616001).  I haven't advertised the nightlies yet because of this issue.

I'm not sure if that would be the cause of this, but it could be (all your bookmark guids would change, and Sync would get confused).
I'll send you my places.sqlite file in a moment. And no, I'm not using Sync.
I emailed you my places.sqlite file. One thing I noticed which might be related to this, is that my profile had a places.sqlite.corrupt file as well.
(In reply to comment #4)
> I emailed you my places.sqlite file. One thing I noticed which might be related
> to this, is that my profile had a places.sqlite.corrupt file as well.
Could you send that as well please?
(In reply to comment #5)
> Could you send that as well please?
Done.
it's pretty clear that central is bailing out WHILE starting up the branch db. I can reproduce the same.

All the filed bugs about moving from branch to central and missing history or icons are just due to this. Since central bails out it thinks the db is corrupt and replaces it (thus the .corrupt file creation) importing bookmarks from the json backup.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Migrating from places branch back to trunk/3.6 loses favicons in bookmarks → Migrating from places branch back to trunk/3.6 wrongly assumes the database is corrupt.
from my debugging, looks like we fail to PRAGMA journal_mode = TRUNCATE
(In reply to comment #8)
> from my debugging, looks like we fail to PRAGMA journal_mode = TRUNCATE
I was pretty sure I had tested this :(
and looks like if a -wal file exists, chaning journal_mode fails...
Or better, it creates a 0-bytes -wal and a almost-0-filled -shm files on startup (we are in wal mode now, so this is expected). Then it bails out when trying to change the journal mode with SQLITE_ERROR.
We need to get a fix in for this into beta 8 since we are targeting beta 9 having the places merge.  We should also fix this on branch.

Our other option is to revert our WAL journal mode at every shutdown, which then incurs a cost of switching it at startup.  This is not ideal.
blocking1.9.2: --- → ?
blocking2.0: ? → beta8+
Assignee: nobody → mak77
Status: NEW → ASSIGNED
Ok, I can confirm this is due to the fact we have a not-finalized statement when we try to change the journal mode. Correctly scoping the statement solved the issue.
The patch is easy, unfortunately this has to go back to old branches :(
Summary: Migrating from places branch back to trunk/3.6 wrongly assumes the database is corrupt. → Downgrading from places branch to trunk or 3.6 wrongly assumes the database is corrupt.
Attached patch patch v1.0Splinter Review
Attachment #496523 - Flags: review?(sdwilsh)
sadly we don't have a harness that allows to run a test on mixed versions of the browser.
These are simple steps to reproduce:

1. Create a new profile with a places branch build
2. Visit a page and check the history entry is added to the history menu
3. Close browser
4. Open the same profile with a mozilla-central (or 3.6) build
5. Check that history entry is still there and no places.sqlite.corrupt file has been created in the profile folder
Flags: in-testsuite-
Comment on attachment 496523 [details] [diff] [review]
patch v1.0

r=sdwilsh
Attachment #496523 - Flags: review?(sdwilsh)
Attachment #496523 - Flags: review+
Attachment #496523 - Flags: approval1.9.2.14?
So, looking at 1.9.2 code now (since I was recalling it would have been different) sounds like 1.9.2 does not have this bug.

Matheus, did you reproduce the dataloss on Firefox 3.6 before of after having seen it on central? Is it possible you tried central, and AFTER THAT Firefox 3.6?
Comment on attachment 496523 [details] [diff] [review]
patch v1.0

temporarily removing the 1.9.2 approval request, since it is unaffected from my tests. Waiting for more details from the reporter.
Attachment #496523 - Flags: approval1.9.2.14?
blocking1.9.2: ? → ---
http://hg.mozilla.org/mozilla-central/rev/f2ac473bec70
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
Duplicate of this bug: 617783
Duplicate of this bug: 617792
Summary: Downgrading from places branch to trunk or 3.6 wrongly assumes the database is corrupt. → Downgrading from places branch to trunk wrongly assumes the database is corrupt.
Mak's assumption is correct, downgrading from places branch directly to 3.6 does not corrupt the database.
Thank you for taking time to confirm it, it's appreciated.

So we won't be able to downgrade:
- beta 9 (or later) to beta 7 (or previous).
- nightly builds after places branch merge to nightly builds before this fix.
We will have to relnote this on beta9, but I'm not adding a relnote keyword here since it should not be added to beta8 that is where this bug has been fixed.

But we can safely downgrade:
- beta 9 (or later) to beta 8.
- any beta or nightly to 3.6.x.
You need to log in before you can comment on or make changes to this bug.