1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00

Remove massive "rule applied" query

Summary:
Herald rules may be marked as "one-time". We track this by writing a row with
<ruleID, phid> when we apply a rule.

However, the current test for rule application involves loading every <ruleID,
*> pair. We also always write this row even for rules which are not one-time, so
if there are 100 rules, we'll load 1,000,000 rows after processing 10,000
objects.

Instead, load only the <phid, *> pairs, which are guaranteed to be bounded to at
most the number of rules.

I'll follow up with a diff that causes us to write rows only for one-time rules,
and deletes all historic rows which are not associated with one-time rules.

Test Plan:
Grepped for callsites to loadAllByContentTypeWithFullData(). Ran
rules in test console.

Reviewers: nh, btrahan, jungejason

Reviewed By: nh

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1483
This commit is contained in:
epriestley 2012-01-24 19:03:30 -08:00
parent 067c7f8a74
commit 3142fe4419
5 changed files with 31 additions and 23 deletions

View file

@ -0,0 +1,2 @@
ALTER TABLE phabricator_herald.herald_ruleapplied
ADD KEY (phid);

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -89,7 +89,8 @@ class HeraldTestConsoleController extends HeraldController {
}
$rules = HeraldRule::loadAllByContentTypeWithFullData(
$adapter->getHeraldTypeName());
$adapter->getHeraldTypeName(),
$object->getPHID());
$engine = new HeraldEngine();
$effects = $engine->applyRules($rules, $adapter);

View file

@ -28,7 +28,9 @@ class HeraldEngine {
public static function loadAndApplyRules(HeraldObjectAdapter $object) {
$content_type = $object->getHeraldTypeName();
$rules = HeraldRule::loadAllByContentTypeWithFullData($content_type);
$rules = HeraldRule::loadAllByContentTypeWithFullData(
$content_type,
$object->getPHID());
$engine = new HeraldEngine();
$effects = $engine->applyRules($rules, $object);

View file

@ -32,7 +32,10 @@ class HeraldRule extends HeraldDAO {
private $ruleApplied = array(); // phids for which this rule has been applied
public static function loadAllByContentTypeWithFullData($content_type) {
public static function loadAllByContentTypeWithFullData(
$content_type,
$object_phid) {
$rules = id(new HeraldRule())->loadAllWhere(
'contentType = %s',
$content_type);
@ -53,17 +56,18 @@ class HeraldRule extends HeraldDAO {
$applied = queryfx_all(
id(new HeraldRule())->establishConnection('r'),
'SELECT * FROM %T WHERE ruleID in (%Ld)',
self::TABLE_RULE_APPLIED, $rule_ids
);
'SELECT * FROM %T WHERE phid = %s',
self::TABLE_RULE_APPLIED,
$object_phid);
$applied = ipull($applied, null, 'ruleID');
$conditions = mgroup($conditions, 'getRuleID');
$actions = mgroup($actions, 'getRuleID');
$applied = igroup($applied, 'ruleID');
foreach ($rules as $rule) {
$rule->attachAllRuleApplied(idx($applied, $rule->getID(), array()));
$rule->setRuleApplied($object_phid, isset($applied[$rule->getID()]));
$rule->attachConditions(idx($conditions, $rule->getID(), array()));
$rule->attachActions(idx($actions, $rule->getID(), array()));
}
@ -72,26 +76,24 @@ class HeraldRule extends HeraldDAO {
}
public function getRuleApplied($phid) {
// defaults to false because (ruleID, phid) pairs not in the db imply
// a rule that's not been applied before
return idx($this->ruleApplied, $phid, false);
if (idx($this->ruleApplied, $phid) === null) {
throw new Exception("Call setRuleApplied() before getRuleApplied()!");
}
return $this->ruleApplied[$phid];
}
public function setRuleApplied($phid) {
$this->ruleApplied[$phid] = true;
public function setRuleApplied($phid, $applied) {
$this->ruleApplied[$phid] = $applied;
return $this;
}
public function attachAllRuleApplied(array $applied) {
// turn array of array(ruleID, phid) into array of ruleID => true
$this->ruleApplied = array_fill_keys(ipull($applied, 'phid'), true);
}
public static function saveRuleApplied($rule_id, $phid) {
queryfx(
id(new HeraldRule())->establishConnection('w'),
'INSERT IGNORE INTO %T (phid, ruleID) VALUES (%s, %d)',
self::TABLE_RULE_APPLIED, $phid, $rule_id
);
self::TABLE_RULE_APPLIED,
$phid,
$rule_id);
}
public function loadConditions() {

View file

@ -1,7 +1,7 @@
<?php
/*
* Copyright 2011 Facebook, Inc.
* Copyright 2012 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@ -32,7 +32,8 @@ class PhabricatorRepositoryCommitHeraldWorker
$commit->getID());
$rules = HeraldRule::loadAllByContentTypeWithFullData(
HeraldContentTypeConfig::CONTENT_TYPE_COMMIT);
HeraldContentTypeConfig::CONTENT_TYPE_COMMIT,
$commit->getPHID());
$adapter = new HeraldCommitAdapter(
$repository,