[core] Enable incremental vacuum for Offline DB

Thus we avoid re-creating the whole database and keeping the backup file as it happens on calling VACUUM.
mapbox · Oct 22, 2019 · 2326e2d · 2326e2d
1 parent 0ca96fd
commit 2326e2d
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 11 deletions.
diff --git a/platform/default/include/mbgl/storage/offline_database.hpp b/platform/default/include/mbgl/storage/offline_database.hpp
@@ -108,6 +108,7 @@ class OfflineDatabase : private util::noncopyable {
     void migrateToVersion6();
     void cleanup();
     bool disabled();
+    void vacuum();
 
     mapbox::sqlite::Statement& getStatement(const char *);
 

diff --git a/platform/default/src/mbgl/storage/offline_database.cpp b/platform/default/src/mbgl/storage/offline_database.cpp
@@ -140,11 +140,12 @@ void OfflineDatabase::removeExisting() {
 void OfflineDatabase::removeOldCacheTable() {
     assert(db);
     db->exec("DROP TABLE IF EXISTS http_cache");
-    db->exec("VACUUM");
+    vacuum();
 }
 
 void OfflineDatabase::createSchema() {
     assert(db);
+    vacuum();
     db->exec("PRAGMA journal_mode = DELETE");
     db->exec("PRAGMA synchronous = FULL");
     mapbox::sqlite::Transaction transaction(*db);
@@ -155,7 +156,7 @@ void OfflineDatabase::createSchema() {
 
 void OfflineDatabase::migrateToVersion3() {
     assert(db);
-    db->exec("VACUUM");
+    vacuum();
     db->exec("PRAGMA user_version = 3");
 }
 
@@ -181,6 +182,15 @@ void OfflineDatabase::migrateToVersion6() {
     transaction.commit();
 }
 
+void OfflineDatabase::vacuum() {
+    if (getPragma<int64_t>("PRAGMA auto_vacuum") != 2 /*INCREMENTAL*/) {
+        db->exec("PRAGMA auto_vacuum = INCREMENTAL");
+        db->exec("VACUUM");
+    } else {
+        db->exec("PRAGMA incremental_vacuum");
+    }
+}
+
 mapbox::sqlite::Statement& OfflineDatabase::getStatement(const char* sql) {
     if (!db) {
         initialize();
@@ -683,7 +693,7 @@ std::exception_ptr OfflineDatabase::clearAmbientCache() try {
 
     resourceQuery.run();
 
-    db->exec("VACUUM");
+    vacuum();
 
     return nullptr;
 } catch (const mapbox::sqlite::Exception& ex) {
@@ -871,7 +881,7 @@ std::exception_ptr OfflineDatabase::deleteRegion(OfflineRegion&& region) try {
 
     evict(0);
     assert(db);
-    db->exec("VACUUM");
+    vacuum();
 
     // Ensure that the cached offlineTileCount value is recalculated.
     offlineMapboxTileCount = {};
@@ -1218,7 +1228,7 @@ std::exception_ptr OfflineDatabase::setMaximumAmbientCacheSize(uint64_t size) {
 
         if (databaseSize > maximumAmbientCacheSize) {
             evict(0);
-            db->exec("VACUUM");
+            vacuum();
         }
 
         return nullptr;

diff --git a/test/fixtures/offline_database/no_auto_vacuum.db b/test/fixtures/offline_database/no_auto_vacuum.db
diff --git a/test/storage/offline_database.test.cpp b/test/storage/offline_database.test.cpp
@@ -92,6 +92,14 @@ static std::vector<std::string> databaseTableColumns(const std::string& path, co
     return columns;
 }
 
+static int databaseAutoVacuum(const std::string& path) {
+    mapbox::sqlite::Database db = mapbox::sqlite::Database::open(path, mapbox::sqlite::ReadOnly);
+    mapbox::sqlite::Statement stmt{db, "pragma auto_vacuum"};
+    mapbox::sqlite::Query query{stmt};
+    query.run();
+    return query.get<int>(0);
+}
+
 namespace fixture {
 
 const Resource resource{ Resource::Style, "mapbox://test" };
@@ -1360,13 +1368,45 @@ TEST(OfflineDatabase, MigrateFromV5Schema) {
 
     EXPECT_EQ(6, databaseUserVersion(filename));
 
-    EXPECT_EQ((std::vector<std::string>{ "id", "url_template", "pixel_ratio", "z", "x", "y",
-                                         "expires", "modified", "etag", "data", "compressed",
-                                         "accessed", "must_revalidate" }),
+    EXPECT_EQ((std::vector<std::string>{"id",
+                                        "url_template",
+                                        "pixel_ratio",
+                                        "z",
+                                        "x",
+                                        "y",
+                                        "expires",
+                                        "modified",
+                                        "etag",
+                                        "data",
+                                        "compressed",
+                                        "accessed",
+                                        "must_revalidate"}),
               databaseTableColumns(filename, "tiles"));
-    EXPECT_EQ((std::vector<std::string>{ "id", "url", "kind", "expires", "modified", "etag", "data",
-                                         "compressed", "accessed", "must_revalidate" }),
-              databaseTableColumns(filename, "resources"));
+    EXPECT_EQ(
+        (std::vector<std::string>{
+            "id", "url", "kind", "expires", "modified", "etag", "data", "compressed", "accessed", "must_revalidate"}),
+        databaseTableColumns(filename, "resources"));
+
+    EXPECT_EQ(0u, log.uncheckedCount());
+}
+
+TEST(OfflineDatabase, IncrementalVacuum) {
+    FixtureLog log;
+    deleteDatabaseFiles();
+    util::copyFile(filename, "test/fixtures/offline_database/no_auto_vacuum.db");
+    EXPECT_EQ(0, databaseAutoVacuum(filename));
+
+    {
+        OfflineDatabase db(filename);
+        db.setMaximumAmbientCacheSize(0);
+
+        auto regions = db.listRegions().value();
+        for (auto& region : regions) {
+            db.deleteRegion(std::move(region));
+        }
+    }
+
+    EXPECT_EQ(2, databaseAutoVacuum(filename));
 
     EXPECT_EQ(0u, log.uncheckedCount());
 }