Write fewer "applied" rows and clean up excess historical rows

Summary:
  - Only write the <ruleID, phid> row if the rule is a one-time rule.
  - Delete all the rows for rules which aren't one-time.

NOTE: This is probably like several million rows for Facebook and could take a
while.

Test Plan:
Added some one-time and every-time rules, ran them against objects, verified
only relevant rows were inserted.
Ran upgrade script against a database with one-time and every-time "ruleapplied"
rows, got the irrelevant rows removed.

Reviewers: nh, btrahan, jungejason

Reviewed By: btrahan

CC: aran, epriestley

Differential Revision: https://secure.phabricator.com/D1484

resources/sql/patches/102.heraldcleanup.php

@@ -0,0 +1,51 @@
+<?php
+
+/*
+ * 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.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+echo "Cleaning up old Herald rule applied rows...\n";
+
+$rules = id(new HeraldRule())->loadAll();
+foreach ($rules as $key => $rule) {
+  $first_policy = HeraldRepetitionPolicyConfig::toInt(
+    HeraldRepetitionPolicyConfig::FIRST);
+  if ($rule->getRepetitionPolicy() != $first_policy) {
+    unset($rules[$key]);
+  }
+}
+
+$conn_w = id(new HeraldRule())->establishConnection('w');
+
+$clause = '';
+if ($rules) {
+  $clause = qsprintf(
+    $conn_w,
+    'WHERE ruleID NOT IN (%Ld)',
+    mpull($rules, 'getID'));
+}
+
+echo "This may take a moment";
+do {
+  queryfx(
+    $conn_w,
+    'DELETE FROM %T %Q LIMIT 1000',
+    HeraldRule::TABLE_RULE_APPLIED,
+    $clause);
+  echo ".";
+} while ($conn_w->getAffectedRows());
+
+echo "\n";
+echo "Done.\n";

src/applications/herald/controller/test/HeraldTestConsoleController.php

@@ -96,7 +96,7 @@ class HeraldTestConsoleController extends HeraldController {
           $effects = $engine->applyRules($rules, $adapter);
 
           $dry_run = new HeraldDryRunAdapter();
-          $engine->applyEffects($effects, $dry_run);
+          $engine->applyEffects($effects, $dry_run, $rules);
 
           $xscript = $engine->getTranscript();
 

src/applications/herald/engine/engine/HeraldEngine.php

@@ -34,7 +34,7 @@ class HeraldEngine {
 
     $engine = new HeraldEngine();
     $effects = $engine->applyRules($rules, $object);
-    $engine->applyEffects($effects, $object);
+    $engine->applyEffects($effects, $object, $rules);
 
     return $engine->getTranscript();
   }
@@ -118,7 +118,11 @@ class HeraldEngine {
     return $effects;
   }
 
-  public function applyEffects(array $effects, HeraldObjectAdapter $object) {
+  public function applyEffects(
+    array $effects,
+    HeraldObjectAdapter $object,
+    array $rules) {
+
     $this->transcript->setDryRun($object instanceof HeraldDryRunAdapter);
 
     $xscripts = $object->applyHeraldEffects($effects);
@@ -132,18 +136,52 @@ class HeraldEngine {
     }
 
     if (!$this->transcript->getDryRun()) {
+
+      $rules = mpull($rules, null, 'getID');
+      $applied_ids = array();
+      $first_policy = HeraldRepetitionPolicyConfig::toInt(
+        HeraldRepetitionPolicyConfig::FIRST);
+
       // Mark all the rules that have had their effects applied as having been
       // executed for the current object.
       $rule_ids = mpull($xscripts, 'getRuleID');
+
       foreach ($rule_ids as $rule_id) {
         if (!$rule_id) {
           // Some apply transcripts are purely informational and not associated
           // with a rule, e.g. carryover emails from earlier revisions.
           continue;
         }
-        HeraldRule::saveRuleApplied($rule_id, $object->getPHID());
+
+        $rule = idx($rules, $rule_id);
+        if (!$rule) {
+          continue;
+        }
+
+        if ($rule->getRepetitionPolicy() == $first_policy) {
+          $applied_ids[] = $rule_id;
+        }
+      }
+
+      if ($applied_ids) {
+        $conn_w = id(new HeraldRule())->establishConnection('w');
+        $sql = array();
+        foreach ($applied_ids as $id) {
+          $sql[] = qsprintf(
+            $conn_w,
+            '(%s, %d)',
+            $object->getPHID(),
+            $id);
+        }
+        queryfx(
+          $conn_w,
+          'INSERT IGNORE INTO %T (phid, ruleID) VALUES %Q',
+          HeraldRule::TABLE_RULE_APPLIED,
+          implode(', ', $sql));
       }
     }
+
+    die("DERP");
   }
 
   public function getTranscript() {

src/applications/herald/engine/engine/__init__.php

@@ -16,6 +16,8 @@ phutil_require_module('phabricator', 'applications/herald/storage/transcript/bas
 phutil_require_module('phabricator', 'applications/herald/storage/transcript/condition');
 phutil_require_module('phabricator', 'applications/herald/storage/transcript/object');
 phutil_require_module('phabricator', 'applications/herald/storage/transcript/rule');
+phutil_require_module('phabricator', 'storage/qsprintf');
+phutil_require_module('phabricator', 'storage/queryfx');
 
 phutil_require_module('phutil', 'utils');
 

src/applications/herald/storage/rule/HeraldRule.php

@@ -87,15 +87,6 @@ class HeraldRule extends HeraldDAO {
     return $this;
   }
 
-  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);
-  }
-
   public function loadConditions() {
     if (!$this->getID()) {
       return array();

src/applications/repository/worker/herald/PhabricatorRepositoryCommitHeraldWorker.php

@@ -42,7 +42,7 @@ class PhabricatorRepositoryCommitHeraldWorker
     $engine = new HeraldEngine();
 
     $effects = $engine->applyRules($rules, $adapter);
-    $engine->applyEffects($effects, $adapter);
+    $engine->applyEffects($effects, $adapter, $rules);
 
     $email_phids = $adapter->getEmailPHIDs();
     if (!$email_phids) {