Page MenuHomePhabricator

HarbormasterBuildArtifact should have an index on buildTargetPHID
Closed, ResolvedPublic

Description

We were experiencing long page loads when looking at Harbormaster builds (as in ~10 seconds in some cases). When I used the DarkConsole to view the queries, I could see some of the queries were taking several seconds when executing:

SELECT * FROM `harbormaster_buildartifact` WHERE (buildTargetPHID IN ('PHID-HMBT-74ihxry2sq3pikmjeu3u')) ORDER BY `id` DESC

We have 210221 build artifacts in our system as of writing, so this query was taking several seconds. I added an index on this table (I executed this through the console rather than patching Phabricator's storage definition of HarbormasterBuildArtifact to avoid more merge conflicts):

create index buildartifact_targetphid ON harbormaster_buildartifact (buildTargetPHID);

Once I added this index, the query dropped down to ~480us.

It looks like HEAD doesn't have an index on the buildTargetPHID column; this should be added because of the significant performance benefit when you have a lot of build artifacts in the system.

Event Timeline

hach-que updated the task description. (Show Details)

Page loads in Harbormaster for us are almost instant after adding this index.

In the buildable page that lists all the builds, I was experiencing 6 second load times until I added:

create index buildartifact_targetphid ON harbormaster_buildunitmessage (buildTargetPHID);

as well. This dropped the SELECT * FROM harbormaster_buildunitmessage WHERE buildTargetPHID IN (...); query from 6 seconds to 1ms.

I can't reproduce the second issue; that index should already exist (or bin/storage adjust should create it if it does not). Here it is in the source:

HarbormasterBuildUnitMessage.php
...
      self::CONFIG_KEY_SCHEMA => array(
        'key_target' => array(
          'columns' => array('buildTargetPHID'),
        ),
      ),
...

It exists on this host and the query is keyed:

        mysql> show create table harbormaster_buildunitmessage\G
        *************************** 1. row ***************************
               Table: harbormaster_buildunitmessage
        Create Table: CREATE TABLE `harbormaster_buildunitmessage` (
          `id` int(10) unsigned NOT NULL AUTO_INCREMENT,
          `buildTargetPHID` varbinary(64) NOT NULL,
          `engine` varchar(255) COLLATE utf8mb4_bin NOT NULL,
          `namespace` varchar(255) COLLATE utf8mb4_bin NOT NULL,
          `name` varchar(255) COLLATE utf8mb4_bin NOT NULL,
          `result` varchar(32) COLLATE utf8mb4_bin NOT NULL,
          `duration` double DEFAULT NULL,
          `properties` longtext COLLATE utf8mb4_bin NOT NULL,
          `dateCreated` int(10) unsigned NOT NULL,
          `dateModified` int(10) unsigned NOT NULL,
          PRIMARY KEY (`id`),
>>>>>>    KEY `key_target` (`buildTargetPHID`)
        ) ENGINE=InnoDB AUTO_INCREMENT=562664 DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_bin
        1 row in set (0.00 sec)

"EXPLAIN" output:

mysql> explain SELECT * FROM `harbormaster_buildunitmessage` WHERE buildTargetPHID IN ('PHID-HMBT-rehrhtip6hjoaxev2n5w', 'PHID-HMBT-fkn5kcmppboqhulnzjuu');
+----+-------------+-------------------------------+-------+---------------+------------+---------+------+------+-------------+
| id | select_type | table                         | type  | possible_keys | key        | key_len | ref  | rows | Extra       |
+----+-------------+-------------------------------+-------+---------------+------------+---------+------+------+-------------+
|  1 | SIMPLE      | harbormaster_buildunitmessage | range | key_target    | key_target | 66      | NULL |  306 | Using where |
+----+-------------+-------------------------------+-------+---------------+------------+---------+------+------+-------------+
1 row in set (0.00 sec)

Artifact table is definitely missing a key, though.

HarbormasterBuildTarget->loadArtifact() also probably does a table scan when it doesn't need to, but if that isn't causing any problems I'll leave it alone for now until we hit it for real.

Ah, I didn't check if HEAD had an index for the unit table. We're about 8 months behind because we can't upgrade until we resolve all of the conflicts caused by the different Drydock architecture.