Made database optimization faster using pragma analysis_limit

src/libs/services/database/impl/Db.cpp

@@ -28,70 +28,62 @@
 
 namespace Database {
 
-// Session living class handling the database and the login
+    // Session living class handling the database and the login
-Db::Db(const std::filesystem::path& dbPath, std::size_t connectionCount)
+    Db::Db(const std::filesystem::path& dbPath, std::size_t connectionCount)
-{
+    {
         LMS_LOG(DB, INFO) << "Creating connection pool on file " << dbPath.string();
 
-	std::unique_ptr<Wt::Dbo::backend::Sqlite3> connection {std::make_unique<Wt::Dbo::backend::Sqlite3>(dbPath.string())};
+        auto connection{ std::make_unique<Wt::Dbo::backend::Sqlite3>(dbPath.string()) };
-//	connection->setProperty("show-queries", "true");
+        //	connection->setProperty("show-queries", "true");
         connection->executeSql("pragma journal_mode=WAL");
         connection->executeSql("pragma synchronous=normal");
+        connection->executeSql("pragma analysis_limit=1000"); // to help make analyze command faster
 
-	auto connectionPool = std::make_unique<Wt::Dbo::FixedSqlConnectionPool>(std::move(connection), connectionCount);
+        auto connectionPool{ std::make_unique<Wt::Dbo::FixedSqlConnectionPool>(std::move(connection), connectionCount) };
-	connectionPool->setTimeout(std::chrono::seconds(10));
+        connectionPool->setTimeout(std::chrono::seconds{ 10 });
 
         _connectionPool = std::move(connectionPool);
-}
+    }
 
-Db::~Db()
+    void Db::executeSql(const std::string& sql)
-{
+    {
-	LMS_LOG(DB, DEBUG) << "Optimizing db...";
+        ScopedConnection connection{ *_connectionPool };
-	executeSql("pragma optimize");
-	LMS_LOG(DB, DEBUG) << "Optimizing db DONE";
-}
-
-void
-Db::executeSql(const std::string& sql)
-{
-	ScopedConnection connection {*_connectionPool};
         connection->executeSql(sql);
-}
+    }
 
-Session&
+    Session& Db::getTLSSession()
-Db::getTLSSession()
+    {
-{
+        static thread_local Session* tlsSession{};
-	static thread_local Session* tlsSession {};
 
         if (!tlsSession)
         {
-		auto newSession {std::make_unique<Session>(*this)};
+            auto newSession{ std::make_unique<Session>(*this) };
             tlsSession = newSession.get();
 
             {
-			std::scoped_lock lock {_tlsSessionsMutex};
+                std::scoped_lock lock{ _tlsSessionsMutex };
                 _tlsSessions.push_back(std::move(newSession));
             }
         }
 
         return *tlsSession;
-}
+    }
 
-Db::ScopedConnection::ScopedConnection(Wt::Dbo::SqlConnectionPool& pool)
+    Db::ScopedConnection::ScopedConnection(Wt::Dbo::SqlConnectionPool& pool)
-: _connectionPool {pool}
+        : _connectionPool{ pool }
-, _connection {_connectionPool.getConnection()}
+        , _connection{ _connectionPool.getConnection() }
-{
+    {
-}
+    }
 
-Db::ScopedConnection::~ScopedConnection()
+    Db::ScopedConnection::~ScopedConnection()
-{
+    {
         _connectionPool.returnConnection(std::move(_connection));
-}
+    }
 
-Wt::Dbo::SqlConnection* Db::ScopedConnection::operator->() const
+    Wt::Dbo::SqlConnection* Db::ScopedConnection::operator->() const
-{
+    {
         return _connection.get();
-}
+    }
 
 } // namespace Database
 

src/libs/services/database/impl/Migration.cpp

@@ -654,7 +654,6 @@ CREATE TABLE IF NOT EXISTS "track_backup" (
             {41, migrateFromV41},
         };
 
-        while (1)
         {
             auto uniqueTransaction{ session.createUniqueTransaction() };
 
@@ -670,19 +669,14 @@ CREATE TABLE IF NOT EXISTS "track_backup" (
                 throw LmsException{ outdatedMsg };
             }
 
-            if (version == LMS_DATABASE_VERSION)
+            if (version > LMS_DATABASE_VERSION)
-            {
-                LMS_LOG(DB, INFO) << "Lms database version " << LMS_DATABASE_VERSION << ": up to date!";
-                return;
-            }
-            else if (version > LMS_DATABASE_VERSION)
-            {
                 throw LmsException{ "Server binary outdated, please upgrade it to handle this database" };
-            }
 
             if (version < migrationFunctions.begin()->first)
                 throw LmsException{ outdatedMsg };
 
+            while (version < LMS_DATABASE_VERSION)
+            {
                 LMS_LOG(DB, INFO) << "Migrating database from version " << version << " to " << version + 1 << "...";
 
                 auto itMigrationFunc{ migrationFunctions.find(version) };
@@ -694,4 +688,5 @@ CREATE TABLE IF NOT EXISTS "track_backup" (
                 LMS_LOG(DB, INFO) << "Migration complete to version " << version;
             }
         }
+    }
 }

src/libs/services/database/impl/Session.cpp

@@ -46,9 +46,9 @@
 namespace Database
 {
 
-Session::Session(Db& db)
+    Session::Session(Db& db)
-: _db {db}
+        : _db{ db }
-{
+    {
         _session.setConnectionPool(_db.getConnectionPool());
 
         _session.mapClass<VersionInfo>("version_info");
@@ -69,64 +69,65 @@ Session::Session(Db& db)
         _session.mapClass<TrackList>("tracklist");
         _session.mapClass<TrackListEntry>("tracklist_entry");
         _session.mapClass<User>("user");
-}
+    }
 
-UniqueTransaction::UniqueTransaction(RecursiveSharedMutex& mutex, Wt::Dbo::Session& session)
+    UniqueTransaction::UniqueTransaction(RecursiveSharedMutex& mutex, Wt::Dbo::Session& session)
-: _lock {mutex},
+        : _lock{ mutex },
- _transaction {session}
+        _transaction{ session }
-{
+    {
-}
+    }
 
-SharedTransaction::SharedTransaction(RecursiveSharedMutex& mutex, Wt::Dbo::Session& session)
+    SharedTransaction::SharedTransaction(RecursiveSharedMutex& mutex, Wt::Dbo::Session& session)
-: _lock {mutex},
+        : _lock{ mutex },
- _transaction {session}
+        _transaction{ session }
-{
+    {
-}
+    }
 
-void
+    void Session::checkUniqueLocked()
-Session::checkUniqueLocked()
+    {
-{
         assert(_db.getMutex().isUniqueLocked());
-}
+    }
 
-void
+    void Session::checkSharedLocked()
-Session::checkSharedLocked()
+    {
-{
         assert(_db.getMutex().isSharedLocked());
-}
+    }
 
-UniqueTransaction
+    UniqueTransaction Session::createUniqueTransaction()
-Session::createUniqueTransaction()
+    {
-{
+        return UniqueTransaction{ _db.getMutex(), _session };
-	return UniqueTransaction {_db.getMutex(), _session};
+    }
-}
 
-SharedTransaction
+    SharedTransaction Session::createSharedTransaction()
-Session::createSharedTransaction()
+    {
-{
+        return SharedTransaction{ _db.getMutex(), _session };
-	return SharedTransaction {_db.getMutex(), _session};
+    }
-}
 
-void
+    void Session::prepareTables()
-Session::prepareTables()
+    {
-{
+        LMS_LOG(DB, INFO) << "Preparing tables...";
-	// Creation case
+
+        // Initial creation case
         try
         {
             _session.createTables();
-
             LMS_LOG(DB, INFO) << "Tables created";
         }
         catch (Wt::Dbo::Exception& e)
+        {
+            LMS_LOG(DB, DEBUG) << "Cannot create tables: " << e.what();
+            if (std::string_view{ e.what() }.find("already exists") == std::string_view::npos)
             {
                 LMS_LOG(DB, ERROR) << "Cannot create tables: " << e.what();
+                throw e;
+            }
         }
 
         Migration::doDbMigration(*this);
 
         // Indexes
         {
-		auto uniqueTransaction {createUniqueTransaction()};
+            auto uniqueTransaction{ createUniqueTransaction() };
             _session.execute("CREATE INDEX IF NOT EXISTS artist_name_idx ON artist(name)");
             _session.execute("CREATE INDEX IF NOT EXISTS artist_sort_name_nocase_idx ON artist(sort_name COLLATE NOCASE)");
             _session.execute("CREATE INDEX IF NOT EXISTS artist_mbid_idx ON artist(mbid)");
@@ -170,21 +171,20 @@ Session::prepareTables()
 
         // Initial settings tables
         {
-		auto uniqueTransaction {createUniqueTransaction()};
+            auto uniqueTransaction{ createUniqueTransaction() };
 
             ScanSettings::init(*this);
         }
-}
+    }
 
-void
+    void Session::analyze()
-Session::optimize()
-{
-	LMS_LOG(DB, DEBUG) << "Optimizing db...";
     {
-		auto uniqueTransaction {createUniqueTransaction()};
+        LMS_LOG(DB, INFO) << "Analyzing database...";
+        {
+            auto uniqueTransaction{ createUniqueTransaction() };
             _session.execute("ANALYZE");
         }
-	LMS_LOG(DB, DEBUG) << "Optimized db!";
+        LMS_LOG(DB, INFO) << "Database Analyze complete";
-}
+    }
 
 } // namespace Database

src/libs/services/database/include/services/database/Db.hpp

@@ -27,23 +27,20 @@
 
 namespace Database {
 
-class Session;
+    class Session;
-class Db
+    class Db
-{
+    {
     public:
         Db(const std::filesystem::path& dbPath, std::size_t connectionCount = 10);
-		~Db();
-
-		Db(const Db&) = delete;
-		Db(Db&&) = delete;
-		Db& operator=(const Db&) = delete;
-		Db& operator=(Db&&) = delete;
 
         Session& getTLSSession();
 
         void executeSql(const std::string& sql);
 
     private:
+        Db(const Db&) = delete;
+        Db& operator=(const Db&) = delete;
+
         friend class Session;
 
         RecursiveSharedMutex& getMutex() { return _sharedMutex; }
@@ -55,14 +52,12 @@ class Db
             ScopedConnection(Wt::Dbo::SqlConnectionPool& pool);
             ~ScopedConnection();
 
-				ScopedConnection(const ScopedConnection& ) = delete;
-				ScopedConnection(ScopedConnection&& ) = delete;
-				ScopedConnection& operator=(const ScopedConnection& ) = delete;
-				ScopedConnection& operator=(ScopedConnection&& ) = delete;
-
             Wt::Dbo::SqlConnection* operator->() const;
 
         private:
+            ScopedConnection(const ScopedConnection&) = delete;
+            ScopedConnection& operator=(const ScopedConnection&) = delete;
+
             Wt::Dbo::SqlConnectionPool& _connectionPool;
             std::unique_ptr<Wt::Dbo::SqlConnection> _connection;
         };
@@ -72,7 +67,7 @@ class Db
 
         std::mutex _tlsSessionsMutex;
         std::vector<std::unique_ptr<Session>> _tlsSessions;
-};
+    };
 
 } // namespace Database
 

src/libs/services/database/include/services/database/Session.hpp

@@ -52,12 +52,8 @@ namespace Database
     class Session
     {
     public:
-			Session (Db& database);
+        Session(Db& database);
-
+        ~Session();
-			Session(const Session&) = delete;
-			Session(Session&&) = delete;
-			Session& operator=(const Session&) = delete;
-			Session& operator=(Session&&) = delete;
 
         [[nodiscard]] UniqueTransaction createUniqueTransaction();
         [[nodiscard]] SharedTransaction createSharedTransaction();
@@ -65,7 +61,7 @@ namespace Database
         void checkUniqueLocked();
         void checkSharedLocked();
 
-			void optimize();
+        void analyze();
 
         void prepareTables(); // need to run only once at startup
 
@@ -77,7 +73,7 @@ namespace Database
         {
             checkUniqueLocked();
 
-				typename Object::pointer res {Object::create(*this, std::forward<Args>(args)...)};
+            typename Object::pointer res{ Object::create(*this, std::forward<Args>(args)...) };
             getDboSession().flush();
 
             if (res->hasOnPostCreated())
@@ -87,6 +83,9 @@ namespace Database
         }
 
     private:
+        Session(const Session&) = delete;
+        Session& operator=(const Session&) = delete;
+
         Db& _db;
         Wt::Dbo::Session	_session;
     };

src/libs/services/database/test/Common.cpp

@@ -60,7 +60,7 @@ DatabaseFixture::SetUpTestCase()
 	{
 		Database::Session s {_tmpDb->getDb()};
 		s.prepareTables();
-		s.optimize();
+		s.analyze();
 
 		// remove default created entries
 		{

src/libs/services/scanner/impl/ScannerService.cpp

@@ -299,8 +299,7 @@ ScannerService::scan(bool forceScan)
 
 	LMS_LOG(DBUPDATER, INFO) << "Scan " << (_abortScan ? "aborted" : "complete") << ". Changes = " << stats.nbChanges() << " (added = " << stats.additions << ", removed = " << stats.deletions << ", updated = " << stats.updates << "), Not changed = " << stats.skips << ", Scanned = " << stats.scans << " (errors = " << stats.errors.size() << "), features fetched = " << stats.featuresFetched << ",  duplicates = " << stats.duplicates.size();
 
-	// TODO make it a scan step
+	_dbSession.analyze();
-	_dbSession.optimize();
 
 	if (!_abortScan)
 	{

src/lms/main.cpp

@@ -238,7 +238,10 @@ int main(int argc, char* argv[])
 		{
 			Database::Session session {database};
 			session.prepareTables();
-			session.optimize();
+
+			 // force optimize in case scanner aborted during a large import:
+			 // queries may be too slow to even be able to relaunch a scan sing the web interface
+			session.analyze();
 		}
 
 		UserInterface::LmsApplicationManager appManager;