From ed61d71935cafc39359e893662e1e09eba7f8001 Mon Sep 17 00:00:00 2001 From: kn Date: Mon, 26 Oct 2026 10:10:36 +0100 Subject: [PATCH] Index build job metadata lookups Every Has/Get/StoreMetaDataString on buildjobmetadata and buildjobexecutionmetadata filters on (owner uuid, metadatakey, active), but neither table has an index, so each lookup scans the whole table. The tables grow with every job and execution, so opening a job's details gets slower as the build history grows: in a synthetic database with 460k metadata rows, reading 46 keys took 2.4 s, and 0.7 ms with the indices. The indices are created by a new CDatabaseMigrationClass::ensureIndices hook that runs after the schema migration on every start (CREATE INDEX IF NOT EXISTS). Existing databases get them on their next start without a schema version step, and no table or data changes. Unit tests check the query plans on a new database and on a database created without the indices. --- .../DataModel/amcdata_databasemigrator.cpp | 3 + .../DataModel/amcdata_databasemigrator.hpp | 4 + .../amcdata_databasemigrator_buildjobs.cpp | 14 ++++ .../amcdata_databasemigrator_buildjobs.hpp | 1 + .../amc_unittests_libmcdata_buildjobs.hpp | 77 +++++++++++++++++++ 5 files changed, 99 insertions(+) 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"); + } }; }