From cadb22479b4063aa8921172119a50f1641733847 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Fri, 4 Sep 2026 10:59:32 -0400 Subject: [PATCH] Bug 2064759 - Setup auto-vacuum for new installs I added this to the `open_database` code so that it will run for all components. I considered making this optional, but I didn't because I don't see a good reason for any component to not do this. Like I say in the comments, even if you don't run maintenance now you might want to do it in the future. --- CHANGELOG.md | 6 ++++++ components/places/src/db/db.rs | 9 +-------- components/places/src/storage/mod.rs | 13 ++----------- components/support/sql/src/lib.rs | 2 +- components/support/sql/src/maintenance.rs | 2 +- components/support/sql/src/open_database.rs | 20 ++++++++++++++++++++ 6 files changed, 31 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2676e0f1f2..3452268cff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,12 @@ - Added `blocks: Vec` to `ffi::MozAdsRequestOptions`, `AdsClient::request*_ads`, `MARSClient::fetch_ads`, `mars::AdRequest`, and `mars::AdRequest::try_new`. This is serialized and passed to MARS so that it can remove blocks server-side. - `shutdown` no longer requires a full `AdsClient` lock (at the cost of no longer shutting down the sqlite db), and telemetry is no longer cloned in the `MozAdsClientBuilder` functions. +### sql_support + +- All databases are initialized with `PRAGMA auto_vacuum=incremental`. + This avoids having to do a full vacuum on the first `sql_support::run_maintenance` call. + (https://bugzilla.mozilla.org/show_bug.cgi?id=2064759) + # v156.0 (_2026-08-27_) ## ✨ What's Changed ✨ diff --git a/components/places/src/db/db.rs b/components/places/src/db/db.rs index 37ffa54fcd..7a1e8084a2 100644 --- a/components/places/src/db/db.rs +++ b/components/places/src/db/db.rs @@ -57,14 +57,7 @@ impl ConnectionInitializer for PlacesInitializer { Ok(schema::upgrade_from(tx, version)?) } - fn prepare(&self, conn: &Connection, db_empty: bool) -> open_database::Result<()> { - // If this is an empty DB, setup incremental auto-vacuum now rather than wait for the first - // run_maintenance_vacuum() call. It should be much faster now with an empty DB. - if db_empty && !matches!(self.conn_type, ConnectionType::ReadOnly) { - conn.execute_one("PRAGMA auto_vacuum=incremental")?; - conn.execute_one("VACUUM")?; - } - + fn prepare(&self, conn: &Connection, _db_empty: bool) -> open_database::Result<()> { let initial_pragmas = " -- The value we use was taken from Desktop Firefox, and seems necessary to -- help ensure good performance on autocomplete-style queries. diff --git a/components/places/src/storage/mod.rs b/components/places/src/storage/mod.rs index 8a97e4f260..3a741cf961 100644 --- a/components/places/src/storage/mod.rs +++ b/components/places/src/storage/mod.rs @@ -11,7 +11,7 @@ pub mod history_metadata; pub mod tags; use crate::db::PlacesDb; -use crate::error::{warn, Error, InvalidPlaceInfo, Result}; +use crate::error::{Error, InvalidPlaceInfo, Result}; use crate::ffi::HistoryVisitInfo; use crate::ffi::TopFrecentSiteInfo; use crate::frecency::{calculate_frecency, DEFAULT_FRECENCY_SETTINGS}; @@ -272,16 +272,7 @@ pub fn run_maintenance_prune( /// Kotlin wrapper code (This is needed because we only have access to the Glean API in Kotlin and /// it supports a stop-watch style API, not recording specific values). pub fn run_maintenance_vacuum(conn: &PlacesDb) -> Result<()> { - let auto_vacuum_setting: u32 = conn.conn_ext_query_one("PRAGMA auto_vacuum")?; - if auto_vacuum_setting == 2 { - // Ideally, we run an incremental vacuum to delete 2 pages - conn.execute_one("PRAGMA incremental_vacuum(2)")?; - } else { - // If auto_vacuum=incremental isn't set, configure it and run a full vacuum. - warn!("run_maintenance_vacuum: Need to run a full vacuum to set auto_vacuum=incremental"); - conn.execute_one("PRAGMA auto_vacuum=incremental")?; - conn.execute_one("VACUUM")?; - } + sql_support::maintenance::vacuum(conn)?; Ok(()) } diff --git a/components/support/sql/src/lib.rs b/components/support/sql/src/lib.rs index 632b822171..3ae779946e 100644 --- a/components/support/sql/src/lib.rs +++ b/components/support/sql/src/lib.rs @@ -12,7 +12,7 @@ pub mod debug_tools; mod each_chunk; mod lazy; -mod maintenance; +pub mod maintenance; mod maybe_cached; pub mod open_database; mod repeat; diff --git a/components/support/sql/src/maintenance.rs b/components/support/sql/src/maintenance.rs index ad01577333..f45305b26e 100644 --- a/components/support/sql/src/maintenance.rs +++ b/components/support/sql/src/maintenance.rs @@ -17,7 +17,7 @@ pub fn run_maintenance(conn: &Connection) -> Result<()> { } /// Run vacuum on the DB -fn vacuum(conn: &Connection) -> Result<()> { +pub fn vacuum(conn: &Connection) -> Result<()> { let auto_vacuum_setting: u32 = conn.conn_ext_query_one("PRAGMA auto_vacuum")?; if auto_vacuum_setting == 2 { // Ideally, we run an incremental vacuum to delete 2 pages diff --git a/components/support/sql/src/open_database.rs b/components/support/sql/src/open_database.rs index ec715c1633..1a4b4bb71c 100644 --- a/components/support/sql/src/open_database.rs +++ b/components/support/sql/src/open_database.rs @@ -154,6 +154,10 @@ fn do_open_database_with_flags>( if open_flags.contains(OpenFlags::SQLITE_OPEN_READ_WRITE) { let mut write_schema_version = true; + if db_empty { + // Need to run this before starting a transaction, since it executes VACUUM. + init_for_maintenance(&conn)?; + } let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; if db_empty { debug!("{}: initializing new database", CI::NAME); @@ -204,6 +208,22 @@ pub fn open_memory_database_with_flags( open_database_with_flags(":memory:", flags, conn_initializer) } +fn init_for_maintenance(conn: &Connection) -> Result<()> { + // Enable incremental auto-vacuum. This stores some additional data to enable auto-vacuum, + // but requires an explicit `PRAGMA incremental_vacuum` to be run rather than auto-vacuuming + // after each transaction. The `run_maintenance()` function performs an auto-vacuum. + // + // This is generally the best setting for components. Even if you're not calling + // `run_maintenance()` now, it's worth it to collect the data to avoid needing a full vacuum + // when you do. + conn.execute_one("PRAGMA auto_vacuum=incremental")?; + // Also call `VACUUM` to ensure the previous PRAGMA takes effect. + // This is not needed for a fresh database with 0 tables, which is probably what we have now. + // However, VACUUM will be a no-op in that case anyways. + conn.execute_one("VACUUM")?; + Ok(()) +} + // Attempt to handle failure when opening the database. // // Returns: