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.

Revisions and Commits

Restricted Differential Revision

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.

epriestley added a revision: Restricted Differential Revision.Jan 21 2016, 6:19 PM

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.

epriestley closed this task as Resolved by committing Restricted Diffusion Commit.Jan 21 2016, 6:21 PM
epriestley added a commit: Restricted Diffusion Commit.

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.