Closed Bug 429336 Opened 14 years ago Closed 14 years ago

Upgrade to sqlite 3.5.8

Categories

(Toolkit :: Storage, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
mozilla1.9

People

(Reporter: sdwilsh, Assigned: sdwilsh)

Details

Attachments

(1 file)

patching coming soonish.  conversation with schrep indicated that we should take this...
Flags: blocking1.9?
Flags: blocking1.9? → blocking1.9+
Attached patch v1.0Splinter Review
sqlite changes omitted - file is way to big to attach with them
Assignee: nobody → sdwilsh
Status: NEW → ASSIGNED
Attachment #316045 - Flags: review?(shaver)
Comment on attachment 316045 [details] [diff] [review]
v1.0

r=shaver, godspeed
Attachment #316045 - Flags: review?(shaver) → review+
Comment on attachment 316045 [details] [diff] [review]
v1.0

also seeking approval to land the sqlite changes, which are too big to attach.
Attachment #316045 - Flags: approval1.9?
Comment on attachment 316045 [details] [diff] [review]
v1.0

a1.9=beltzner
Attachment #316045 - Flags: approval1.9? → approval1.9+
Checking in configure.in;
new revision: 1.1982; previous revision: 1.1981
Checking in db/sqlite3/README.MOZILLA;
new revision: 1.26; previous revision: 1.25
Checking in db/sqlite3/src/sqlite3.c;
new revision: 1.17; previous revision: 1.16
Checking in db/sqlite3/src/sqlite3.h;
new revision: 1.20; previous revision: 1.19

and to stop us from constantly changing the $Id$ in the files...
11 mozilla $ cvs admin -ko db/sqlite3/src/sqlite3.*
RCS file: /cvsroot/mozilla/db/sqlite3/src/sqlite3.c,v
done
RCS file: /cvsroot/mozilla/db/sqlite3/src/sqlite3.h,v
done
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
and a bustage fix because the def file contained something we don't have anymore...

Checking in src/sqlite.def;
new revision: 1.10; previous revision: 1.9
Note: per discussion in the SQLite users mailing list <http://www.mail-archive.com/sqlite-users%40sqlite.org/msg33263.html>, 3.5.8 contains a performance regression for left outer joins from the fix for SQLite ticket #3015 <http://www.sqlite.org/cvstrac/tktview?tn=3015>.

We use some of those (almost exclusively in places) <http://mxr.mozilla.org/mozilla/search?string=left%20outer%20join>, so we should be on the lookout for effects on our performance and track the work to fix the regression closely.
Backed this out on behalf of sdwilsh, to see if it caused the mac & linux tinderbox oranges. (with "reftest 0/0/0")

Checking in configure.in;
/cvsroot/mozilla/configure.in,v  <--  configure.in
new revision: 1.1983; previous revision: 1.1982
done
Checking in db/sqlite3/README.MOZILLA;
/cvsroot/mozilla/db/sqlite3/README.MOZILLA,v  <--  README.MOZILLA
new revision: 1.27; previous revision: 1.26
done
Checking in db/sqlite3/src/sqlite.def;
/cvsroot/mozilla/db/sqlite3/src/sqlite.def,v  <--  sqlite.def
new revision: 1.11; previous revision: 1.10
done
Checking in db/sqlite3/src/sqlite3.c;
/cvsroot/mozilla/db/sqlite3/src/sqlite3.c,v  <--  sqlite3.c
new revision: 1.18; previous revision: 1.17
done
Checking in db/sqlite3/src/sqlite3.h;
/cvsroot/mozilla/db/sqlite3/src/sqlite3.h,v  <--  sqlite3.h
new revision: 1.21; previous revision: 1.20
done
Status: RESOLVED → REOPENED
OS: Mac OS X → All
Hardware: PC → All
Resolution: FIXED → ---
I do not have time to deal with this.
Assignee: sdwilsh → nobody
Status: REOPENED → NEW
During the brief period this was in the tree, Firefox took a few extra seconds to start up, and froze on shutdown.
Mac reftests went back to normal after this was backed out.  (Linux is still running)

Broken cycle (pre-backout):
  http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1208458696.1208463548.18519.gz
Working cycle (post-backout):
  http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1208466480.1208469284.32094.gz
Linux went back to normal as well.
I'm not sure, but I think the update also was causing hangs when opening the Library, either bookmarks or history.  History was really a long delay, Vista went non-responding on several occasions, and with the Library open, opening any other tabs, or refreshing them produced non-repsonding.

My Places.sqlite is 4.8 meg, and takes normally about 2 secs to load, it was taking upwards of 40+ secs, most of the time spent 'Not Repsonding'. 

I have gone back to build:
Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041702 Minefield/3.0pre Firefox/3.0 ID:2008041702 
which was stable, now I'm crashing on restore after being minimized for an unknown period of time, and this is in the Event Viewer:

Faulting application firefox.exe, version 1.9.0.3027, time stamp 0x48072c3c, faulting module sqlite3.dll, version 3.5.4.1, time stamp 0x48072c99, exception code 0xc0000005, fault offset 0x0000d872, process id 0xd00, application start time 0x01c8a0cd622c93e0. 

Thus my fears that the update may have corrupted something.

Confirmed, the backout of this update no longer is causing the 'Not repsonding' and my History is back to 'Normal' open time, about 2 secs on this old tired machine. 

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008041717 Minefield/3.0pre Firefox/3.0 ID:2008041717 <-Latest hourly - the SQLite update.

I compiled and ran reftests both with and without 3.5.8.  As far as I can tell, the reftests ran without incident both times, although I didn't get output from them in either case, so it's hard to know for sure.
(In reply to comment #15)
> I compiled and ran reftests both with and without 3.5.8.  As far as I can tell,
> the reftests ran without incident both times, although I didn't get output from
> them in either case, so it's hard to know for sure.

Erm, but that's because I was running them in an opt build.  I reran both in debug builds, which did output results, and both builds passed all reftests.

So I'm not sure what the issue is.  Maybe the left outer join performance regression that probably caused the places slowness on startup caused the test infrastructure to time out?  I don't have a better idea.
Given comment 7 and later and the phase of the release should we punt an upgrade until a future version?  Any driving reason we need 3.5.8 (+ fix for 3015) now?
Assignee: nobody → sdwilsh
This seems really really scary.  Given comments 6-14, I'd vote to not take this at this point.

Re-nom if you disagree.
Flags: blocking1.9+ → blocking1.9-
(In reply to comment #17)
> Given comment 7 and later and the phase of the release should we punt an
> upgrade until a future version?  Any driving reason we need 3.5.8 (+ fix for
> 3015) now?

3.5.8 fixes "a bug in the SQLITE_SECURE_DELETE option that was causing Firefox crashes."  That bug is what caused 3.5.7 to be particularly crashy for us, but the bug is present in 3.5.4.2 and could cause crashes there too.

However, if we aren't experiencing a statistically noticeable number of such crashes, then this could wait for a future release, as the other fixes we'd pick up are either not particularly important/relevant or only affect extensions.


(In reply to comment #18)
> This seems really really scary.  Given comments 6-14, I'd vote to not take this
> at this point.

That seems reasonable to me, given where we are in the cycle.  Let's circle around to this on a future point release.
I agree.
Status: NEW → RESOLVED
Closed: 14 years ago14 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.