Closed Bug 592678 Opened 12 years ago Closed 4 years ago

Relocate the database file after a VACUUM

Categories

(Toolkit :: Storage, defect)

defect
Not set
normal

Tracking

()

RESOLVED INACTIVE
Tracking Status
blocking2.0 --- -

People

(Reporter: mak, Unassigned)

References

Details

VACUUM defragments the database internal structure, but replaces the database in place, this means it won't help at all with system fragmentation.
We have to figure out a strategy to move a VACUUMed database so that the OS can find a better (less fragmented) place for it.

We could either ask upstream to make vacuum use a new file, or copy a vacuumed database to a new one on shutdown/startup (shutdown is probably easier, since at xpcom-shutdown connections should be closed since all databases depend on having a profile).

See https://bugzilla.mozilla.org/show_bug.cgi?id=541373#c16 and next ones for more details.
possibly blocking final for fragmentation, that is one of the first causes of bad sqlite performances on our side.
blocking2.0: --- → ?
I'm not sure we can do this in a safe manner given http://www.mail-archive.com/sqlite-users@sqlite.org/msg50941.html
making sure the new copy is ready and done before swapping dbs does not look like less safe than what they do currently. The "preserve metadata" stuff is a different question, but it should probably be opt-out (we don't mind it).
Btw, on our side we could do this in a safe enough way, when the db is closed (xpcom-shutdown) just copy the file to a new one, and if the copy ends correctly swap names. There is a small risk in the swap names part if the OS crashes in the middle but should be pretty rare.
I'm not sure we even need to swap names.
1) vacuum-copy places.sqlite to places.sqlite.bak
2) rename places.sqlite.bak to places.sqlite(this doesn't copy the file contents so it's fast)

worst thing that can happen here is that we end up with both places.sqlite and places.sqlite.bak, but we can deal with that. Besides, ntfs supports transactions for this sort of thing, so on windows this might turn out to be really convenient.
(In reply to comment #4)
> I'm not sure we even need to swap names.
> 1) vacuum-copy places.sqlite to places.sqlite.bak
> 2) rename places.sqlite.bak to places.sqlite(this doesn't copy the file
> contents so it's fast)

I guess there is "1.5) delete old places.sqlite" in the middle, that is the risky part?
Never delete in the middle! Never leave no places.sqlite file linked in the directory.

rename(2) is atomic on all Unixes, within a mounted filesystem. It works by atomic directory update, so you either have the old hard link or the new one. No nothing linked state.

/be
(In reply to comment #0)
> We could either ask upstream to make vacuum use a new file, or copy a vacuumed
> database to a new one on shutdown/startup (shutdown is probably easier, since
> at xpcom-shutdown connections should be closed since all databases depend on
> having a profile).
This assumes that we'd want to VACUUM on shutdown, right?
(In reply to comment #7)
> This assumes that we'd want to VACUUM on shutdown, right?

Nope, we could do the 2 operations at any time, vacuum will defragment internal structure. the rename would try to defragment physical file. I don't think we absolutely need to do both at the same time.
(In reply to comment #8)
> (In reply to comment #7)
> > This assumes that we'd want to VACUUM on shutdown, right?
> 
> Nope, we could do the 2 operations at any time, vacuum will defragment internal
> structure. the rename would try to defragment physical file. I don't think we
> absolutely need to do both at the same time.

Just to clarify. You still need to do a copy before a rename. Rename just tweaks a few directory listings. But for places this should be cheap since most of places should already be cached.
The problem that seems to me to be overlooked here is that if you have more than one database connection (easily doable with mozIStorageConnection::clone), each database is going to have a different file handle to a database file.

In addition, SQLite will no longer have the right file handle if we move a new file in place of the old one AFAICT.  This problem is compounded when you have more than one database connection to the same database file.
I don't think we can block on this without a plane for dealing with more than one database.
blocking2.0: ? → -
I think we should just do it at app-update.
We also have to worry about the possibility of something modifying the database while we move it.  Would the live backup API help here?
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.