592678 - Relocate the database file after a VACUUM

Status: RESOLVED INACTIVE

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Marco Bonardo [:mak]]

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.

Comment 1

[Marco Bonardo [:mak]]

possibly blocking final for fragmentation, that is one of the first causes of bad sqlite performances on our side.

Comment 2

[Shawn Wilsher :sdwilsh]

I'm not sure we can do this in a safe manner given http://www.mail-archive.com/[e-mail]/msg50941.html

Comment 3

[Marco Bonardo [:mak]]

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.

Comment 4

[(dormant account)]

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.

Comment 5

[Marco Bonardo [:mak]]

(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?

Comment 6

[Brendan Eich [:brendan]]

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

Comment 7

[Shawn Wilsher :sdwilsh]

(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?

Comment 8

[Marco Bonardo [:mak]]

(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.

Comment 9

[(dormant account)]

(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.

Comment 10

[Shawn Wilsher :sdwilsh]

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.

Comment 11

[Shawn Wilsher :sdwilsh]

I don't think we can block on this without a plane for dealing with more than one database.

Comment 12

[(dormant account)]

I think we should just do it at app-update.

Comment 13

[Shawn Wilsher :sdwilsh]

We also have to worry about the possibility of something modifying the database while we move it.  Would the live backup API help here?

Comment 14

[BMO Automation]

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.