The url-classifier relies on synchronous=OFF to get decent update performance on its large transactions. This opens up the possibility of db corruption in the case of a badly-timed power failure. There's no user data lost, we just need to rebuild the db from the server. Right now we only reset the db if the corruption is detected when the database is open, which isn't always the case. Attached patch will recover from corruption detected during the update or lookup process. As it stands right now, if corruption is detected during the update/lookup processes, the update/lookup will fail silently, and it won't fix itself.

Comment on attachment 321795 [details] [diff] [review] v1 Looks ok, but wouldn't it be easier to just do a test lookup after the open to see if the table is corrupt and reset at that point? This change seems a lot more invasive.

A single lookup isn't guaranteed to come across any given corrupted page. Between all lookups and updates, though, we should detect corruption reasonably quickly.

I see, I guess we could scan all the tables at startup, but that sounds worse. Ok, seems like the better approach.

Comment on attachment 321795 [details] [diff] [review] v1 OK, so there's a simpler way to do this. After corruption is detected, every major call to sqlite functions returns SQLITE_CORRUPT. So we shouldn't really have to do all this weird propagation/saving, we can just check lastError at the end of an update process. Patch coming up...

I actually don't think this blocks based on the impact analysis in comment 0, but no reason why we shouldn't take the patch. Do we need bake-time on mozilla-central first?

This fix is a lot simpler.

Comment on attachment 326625 [details] [diff] [review] better fix looks good; r+

Pushed to mozilla-central as changeset caeba7562e49.

Oops, this doesn't build on windows, backed out until I can find the right way to fix it.

Updated patch, was missing a REQUIRES.

Pushed to mozilla-central as df6e6f3ac380

Comment on attachment 333897 [details] [diff] [review] updated patch. Lost my approval flag with the updated patch...

Comment on attachment 333897 [details] [diff] [review] updated patch. Approved for 1.9.0.2. Please land in CVS. a=ss

cvsroot/mozilla/toolkit/components/url-classifier/src/Makefile.in,v <-- Makefile.in new revision: 1.22; previous revision: 1.21 done Checking in src/nsUrlClassifierDBService.cpp; /cvsroot/mozilla/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp,v <-- nsUrlClassifierDBService.cpp new revision: 1.80; previous revision: 1.79 done