Update moz_origins index column order to match desktop - #7521
Conversation
This change ports the fix from https://bugzilla.mozilla.org/show_bug.cgi?id=2025999 which replaces the (prefix, host) unique index on moz_origins with a (host, prefix) index instead. This improves the performance of the index by putting the higher-cardinality column first, and also makes queries which don't filter on prefix eligible for the index. Similar to the fix on desktop, we create a new table and copy the data over due to sqlite limitations on modifying constraints. However, we can't use defer_foreign_keys like we do on desktop, since the foreign key here is ON DELETE CASCADE -- therefore we have to also temporarily null out the foreign key references and copy them back afterward.
ec21654 to
47dd6a4
Compare
|
Thanks for this!
Thanks for this too, and the performance does scare me a little - 2s for a migration on a phone that's probably fairly recent and probably not an outlier in terms of size sounds bad. The fact Lina already did for the 17 migration does make me think we should consider it. When you say risky, do you just mean that if we do something dumb we might destroy the DB, or is there something more subtle here? |
Actually smaller than our maintenance target of 75MiB |
This PR serves as a follow up to #7484 as well as a port of the fix in desktop for bug 2025999.
This doesn't add any new indexes, like #7484 did. Instead, it just reverses the column order of the existing (prefix, host) unique index on moz_origins with a (host, prefix) index instead. In addition to mitigating the bad query plan from bug 2056115, this change also has the advantage of improved performance due to putting the higher-cardinality column first, and also makes queries which don't filter on prefix eligible for the index.
Similar to the fix on desktop, we create a new table and copy the data over due to sqlite limitations on modifying constraints. However, we can't use defer_foreign_keys like we do on desktop, since the foreign key here is ON DELETE CASCADE -- therefore we have to also temporarily null out the foreign key references and copy them back afterward. This makes the migration much slower and generate much more WAL than the respective migration on desktop.
As an alternative, we could modify
sqlite_schemadirectly: this is more risky, but far more efficient: only ~76ms and ~0MB WAL as opposed to ~2060ms and ~22MB WAL on my actual 58MB places DB. It also has some precedent in this codebase (migration 17 does it), unlike desktop. An example of that approach can be seen here: shawnz@e157a2bPull Request checklist
[ci full]to the PR title.