diff --git a/Implementation/DataModel/amcdata_databasemigrator.cpp b/Implementation/DataModel/amcdata_databasemigrator.cpp index 0487feda5..2ad2540c5 100644 --- a/Implementation/DataModel/amcdata_databasemigrator.cpp +++ b/Implementation/DataModel/amcdata_databasemigrator.cpp @@ -122,6 +122,9 @@ namespace AMCData { pStatement4->execute(); } + for (auto pMigrator : m_MigrationClasses) + pMigrator->ensureIndices(pTransaction); + pTransaction->commit(); pTransaction = nullptr; } diff --git a/Implementation/DataModel/amcdata_databasemigrator.hpp b/Implementation/DataModel/amcdata_databasemigrator.hpp index 4d22f1c82..57fe641cd 100644 --- a/Implementation/DataModel/amcdata_databasemigrator.hpp +++ b/Implementation/DataModel/amcdata_databasemigrator.hpp @@ -43,6 +43,10 @@ namespace AMCData { class CDatabaseMigrationClass { public: virtual void increaseSchemaVersion (PSQLTransaction pTransaction, uint32_t nCurrentVersionIndex) = 0; + + // Runs on every start after the schema migration. Must be idempotent (e.g. CREATE INDEX IF NOT EXISTS), + // so indices reach existing databases without a schema version step. + virtual void ensureIndices (PSQLTransaction pTransaction) {} }; typedef std::shared_ptr PDatabaseMigrationClass; diff --git a/Implementation/DataModel/amcdata_databasemigrator_buildjobs.cpp b/Implementation/DataModel/amcdata_databasemigrator_buildjobs.cpp index 1516ff976..5ac3a2268 100644 --- a/Implementation/DataModel/amcdata_databasemigrator_buildjobs.cpp +++ b/Implementation/DataModel/amcdata_databasemigrator_buildjobs.cpp @@ -170,7 +170,21 @@ namespace AMCData { } + void CDatabaseMigrationClass_BuildJobs::ensureIndices(PSQLTransaction pTransaction) + { + if (pTransaction.get() == nullptr) + throw ELibMCDataInterfaceException(LIBMCDATA_ERROR_INVALIDPARAM); + // Every metadata read and duplicate check filters on these three columns. Without an index + // each one scans the whole table, which grows with every execution. + std::string sExecutionMetaDataIndexQuery = "CREATE INDEX IF NOT EXISTS `idx_buildjobexecutionmetadata_exec_key` "; + sExecutionMetaDataIndexQuery += "ON `buildjobexecutionmetadata` (`executionuuid`, `metadatakey`, `active`)"; + pTransaction->executeStatement(sExecutionMetaDataIndexQuery); + + std::string sJobMetaDataIndexQuery = "CREATE INDEX IF NOT EXISTS `idx_buildjobmetadata_job_key` "; + sJobMetaDataIndexQuery += "ON `buildjobmetadata` (`jobuuid`, `metadatakey`, `active`)"; + pTransaction->executeStatement(sJobMetaDataIndexQuery); + } } diff --git a/Implementation/DataModel/amcdata_databasemigrator_buildjobs.hpp b/Implementation/DataModel/amcdata_databasemigrator_buildjobs.hpp index 7c7d514c8..8d060e7c0 100644 --- a/Implementation/DataModel/amcdata_databasemigrator_buildjobs.hpp +++ b/Implementation/DataModel/amcdata_databasemigrator_buildjobs.hpp @@ -39,6 +39,7 @@ namespace AMCData { class CDatabaseMigrationClass_BuildJobs : public CDatabaseMigrationClass { public: void increaseSchemaVersion (PSQLTransaction pTransaction, uint32_t nCurrentVersionIndex) override; + void ensureIndices (PSQLTransaction pTransaction) override; }; } diff --git a/Implementation/UnitTest/amc_unittests_libmcdata_buildjobs.hpp b/Implementation/UnitTest/amc_unittests_libmcdata_buildjobs.hpp index 321016e0f..e5b53a3de 100644 --- a/Implementation/UnitTest/amc_unittests_libmcdata_buildjobs.hpp +++ b/Implementation/UnitTest/amc_unittests_libmcdata_buildjobs.hpp @@ -35,6 +35,7 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include "amc_unittests_libmcdata.hpp" #include "common_utils.hpp" #include "libmcdata_dynamic.hpp" +#include "amcdata_sqlhandler_sqlite.hpp" #include #include @@ -117,6 +118,10 @@ namespace AMCUnitTest { registerTest("JobIteratorGetCurrent", "Job iterator GetCurrent", eUnitTestCategory::utMandatoryPass, std::bind(&CUnitTestGroup_LibMCData_BuildJobs::testJobIteratorGetCurrent, this)); registerTest("JobLongName", "Job with long name", eUnitTestCategory::utMandatoryPass, std::bind(&CUnitTestGroup_LibMCData_BuildJobs::testJobLongName, this)); registerTest("ExecMultipleMetaData", "Execution multiple metadata", eUnitTestCategory::utMandatoryPass, std::bind(&CUnitTestGroup_LibMCData_BuildJobs::testExecMultipleMetaData, this)); + + // Metadata lookups must not scan the whole metadata table + registerTest("MetaDataLookupsUseIndex", "Job and execution metadata lookups are planned via an index", eUnitTestCategory::utMandatoryPass, std::bind(&CUnitTestGroup_LibMCData_BuildJobs::testMetaDataLookupsUseIndex, this)); + registerTest("MetaDataIndicesAddedOnStart", "A database from before the metadata indices gets them on the next start", eUnitTestCategory::utMandatoryPass, std::bind(&CUnitTestGroup_LibMCData_BuildJobs::testMetaDataIndicesAddedOnStart, this)); } void initializeTests() override { @@ -1213,6 +1218,78 @@ namespace AMCUnitTest { assertTrue(pExec->GetMetaDataString(sKey) == sExpectedValue, "Metadata value should match for " + sKey); } } + + // ============= Metadata lookup indices ============= + + // The metadata reads and duplicate checks in libmcdata_buildjob.cpp / libmcdata_buildjobexecution.cpp + std::string metaDataLookupQuery(const std::string& sTableName, const std::string& sOwnerColumn) + { + return "SELECT metadatavalue FROM " + sTableName + " WHERE " + sOwnerColumn + "=? AND metadatakey=? AND active=?"; + } + + std::string queryPlan(const std::string& sDatabaseFile, const std::string& sQuery) + { + AMCData::CSQLHandler_SQLite sqlHandler(sDatabaseFile); + auto pStatement = sqlHandler.prepareStatement("EXPLAIN QUERY PLAN " + sQuery); + std::string sPlan; + while (pStatement->nextRow()) + sPlan += pStatement->getColumnString(4) + "\n"; + return sPlan; + } + + void assertLookupUsesIndex(const std::string& sDatabaseFile, const std::string& sTableName, const std::string& sOwnerColumn) + { + std::string sPlan = queryPlan(sDatabaseFile, metaDataLookupQuery(sTableName, sOwnerColumn)); + assertTrue(sPlan.find("SCAN") == std::string::npos, sTableName + " lookup does not scan the table: " + sPlan); + assertTrue(sPlan.find("USING INDEX idx_" + sTableName + "_") != std::string::npos, sTableName + " lookup uses its metadata index: " + sPlan); + } + + void assertMetaDataLookupsUseIndex(const std::string& sDatabaseFile) + { + assertLookupUsesIndex(sDatabaseFile, "buildjobexecutionmetadata", "executionuuid"); + assertLookupUsesIndex(sDatabaseFile, "buildjobmetadata", "jobuuid"); + } + + void testMetaDataLookupsUseIndex() + { + auto fixture = createFixture("metadata_index"); + assertMetaDataLookupsUseIndex(fixture.m_sBasePath + "/buildjobs.db"); + } + + void testMetaDataIndicesAddedOnStart() + { + auto fixture = createFixture("metadata_indices_start"); + std::string sDatabaseFile = fixture.m_sBasePath + "/buildjobs.db"; + + auto pJob = createValidatedJob(fixture, "Job From Older Build"); + pJob->StoreMetaDataString("jobkey", "jobvalue", getCurrentTimestamp()); + auto pExec = pJob->CreateBuildJobExecution("Execution from older build", fixture.m_sUserUUID, getCurrentTimestamp()); + pExec->StoreMetaDataString("execkey", "execvalue", getCurrentTimestamp()); + std::string sJobUUID = pJob->GetUUID(); + std::string sExecUUID = pExec->GetExecutionUUID(); + + // A database written by a build without the indices: same schema version, no indices. + { + AMCData::CSQLHandler_SQLite sqlHandler(sDatabaseFile); + sqlHandler.prepareStatement("DROP INDEX idx_buildjobexecutionmetadata_exec_key")->execute(); + sqlHandler.prepareStatement("DROP INDEX idx_buildjobmetadata_job_key")->execute(); + } + assertTrue(queryPlan(sDatabaseFile, metaDataLookupQuery("buildjobexecutionmetadata", "executionuuid")).find("SCAN") != std::string::npos, "execution metadata index is gone before the restart"); + assertTrue(queryPlan(sDatabaseFile, metaDataLookupQuery("buildjobmetadata", "jobuuid")).find("SCAN") != std::string::npos, "job metadata index is gone before the restart"); + + // Next server start. Its own data directory, because journal files are named by the second + // and a restart within the same second would collide with the first journal. + std::string sRestartDirectory = fixture.m_sBasePath + "/restart"; + AMCCommon::CUtils::createDirectoryOnDisk(sRestartDirectory); + auto pRestartedModel = fixture.m_pDataWrapper->CreateDataModelInstance(); + pRestartedModel->InitialiseDatabase(sRestartDirectory, LibMCData::eDataBaseType::SqLite, sDatabaseFile); + + assertMetaDataLookupsUseIndex(sDatabaseFile); + + auto pRestartedHandler = pRestartedModel->CreateBuildJobHandler(); + assertTrue(pRestartedHandler->RetrieveJob(sJobUUID)->GetMetaDataString("jobkey") == "jobvalue", "job metadata still readable"); + assertTrue(pRestartedHandler->RetrieveJobExecution(sExecUUID)->GetMetaDataString("execkey") == "execvalue", "execution metadata still readable"); + } }; }