From a761f733848b4263c54cdfda96ea1f47febe8c64 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 21 Dec 2015 11:04:08 -0800 Subject: [PATCH] Allow index extensions to skip indexing if the object has not changed Summary: Fixes T9890. This allows IndexExtensions to emit an object version. Before we build indexes, we check if the indexed version is the same as the current version. If it is, we just don't call that extension. T9890 has a case where this is useful: a script went crazy and posted thousands of comments to a single task. Without versioning, that results in the same comments being indexed over and over again. With versioning, most of the queue could just exit without doing any work. Test Plan: - Added a `sleep(1)` to the actual indexing, used `bin/search index --background` to queue up a lot of tasks, ran them with `bin/phd debug task`, saw them complete very quickly with only one actual index operation performed. - Used `bin/search index --trace` and `bin/search index --trace --background` to observe the behavior of queries against the index version store, which looked sensible. - Made comments/transactions, saw versions update. - Used `bin/remove destroy`, verified index versions were purged. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9890 Differential Revision: https://secure.phabricator.com/D14845 --- .../autopatches/20151221.search.1.version.sql | 7 +++ src/__phutil_library_map__.php | 4 ++ ...habricatorFulltextIndexEngineExtension.php | 59 +++++++++++++++++++ ...IndexVersionDestructionEngineExtension.php | 25 ++++++++ .../search/index/PhabricatorIndexEngine.php | 57 +++++++++++++++++- .../storage/PhabricatorSearchIndexVersion.php | 26 ++++++++ 6 files changed, 175 insertions(+), 3 deletions(-) create mode 100644 resources/sql/autopatches/20151221.search.1.version.sql create mode 100644 src/applications/search/engineextension/PhabricatorSearchIndexVersionDestructionEngineExtension.php create mode 100644 src/applications/search/storage/PhabricatorSearchIndexVersion.php diff --git a/resources/sql/autopatches/20151221.search.1.version.sql b/resources/sql/autopatches/20151221.search.1.version.sql new file mode 100644 index 0000000000..dca993f1ce --- /dev/null +++ b/resources/sql/autopatches/20151221.search.1.version.sql @@ -0,0 +1,7 @@ +CREATE TABLE {$NAMESPACE}_search.search_indexversion ( + id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT, + objectPHID VARBINARY(64) NOT NULL, + extensionKey VARCHAR(64) NOT NULL COLLATE {$COLLATE_TEXT}, + version VARCHAR(128) NOT NULL COLLATE {$COLLATE_TEXT}, + UNIQUE KEY `key_object` (objectPHID, extensionKey) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 59f98069e4..409d7c6215 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3042,6 +3042,8 @@ phutil_register_library_map(array( 'PhabricatorSearchEngineTestCase' => 'applications/search/engine/__tests__/PhabricatorSearchEngineTestCase.php', 'PhabricatorSearchField' => 'applications/search/field/PhabricatorSearchField.php', 'PhabricatorSearchHovercardController' => 'applications/search/controller/PhabricatorSearchHovercardController.php', + 'PhabricatorSearchIndexVersion' => 'applications/search/storage/PhabricatorSearchIndexVersion.php', + 'PhabricatorSearchIndexVersionDestructionEngineExtension' => 'applications/search/engineextension/PhabricatorSearchIndexVersionDestructionEngineExtension.php', 'PhabricatorSearchManagementIndexWorkflow' => 'applications/search/management/PhabricatorSearchManagementIndexWorkflow.php', 'PhabricatorSearchManagementInitWorkflow' => 'applications/search/management/PhabricatorSearchManagementInitWorkflow.php', 'PhabricatorSearchManagementWorkflow' => 'applications/search/management/PhabricatorSearchManagementWorkflow.php', @@ -7407,6 +7409,8 @@ phutil_register_library_map(array( 'PhabricatorSearchEngineTestCase' => 'PhabricatorTestCase', 'PhabricatorSearchField' => 'Phobject', 'PhabricatorSearchHovercardController' => 'PhabricatorSearchBaseController', + 'PhabricatorSearchIndexVersion' => 'PhabricatorSearchDAO', + 'PhabricatorSearchIndexVersionDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension', 'PhabricatorSearchManagementIndexWorkflow' => 'PhabricatorSearchManagementWorkflow', 'PhabricatorSearchManagementInitWorkflow' => 'PhabricatorSearchManagementWorkflow', 'PhabricatorSearchManagementWorkflow' => 'PhabricatorManagementWorkflow', diff --git a/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php b/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php index 19837043a5..887a5c4ee6 100644 --- a/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php +++ b/src/applications/search/engineextension/PhabricatorFulltextIndexEngineExtension.php @@ -9,6 +9,23 @@ final class PhabricatorFulltextIndexEngineExtension return pht('Fulltext Engine'); } + public function getIndexVersion($object) { + $version = array(); + + if ($object instanceof PhabricatorApplicationTransactionInterface) { + // If this is a normal object with transactions, we only need to + // reindex it if there are new transactions (or comment edits). + $version[] = $this->getTransactionVersion($object); + $version[] = $this->getCommentVersion($object); + } + + if (!$version) { + return null; + } + + return implode(':', $version); + } + public function shouldIndexObject($object) { return ($object instanceof PhabricatorFulltextInterface); } @@ -27,4 +44,46 @@ final class PhabricatorFulltextIndexEngineExtension $engine->buildFulltextIndexes(); } + private function getTransactionVersion($object) { + $xaction = $object->getApplicationTransactionTemplate(); + + $xaction_row = queryfx_one( + $xaction->establishConnection('r'), + 'SELECT id FROM %T WHERE objectPHID = %s + ORDER BY id DESC LIMIT 1', + $xaction->getTableName(), + $object->getPHID()); + if (!$xaction_row) { + return 'none'; + } + + return $xaction_row['id']; + } + + private function getCommentVersion($object) { + $xaction = $object->getApplicationTransactionTemplate(); + + try { + $comment = $xaction->getApplicationTransactionCommentObject(); + } catch (Exception $ex) { + return 'none'; + } + + $comment_row = queryfx_one( + $comment->establishConnection('r'), + 'SELECT c.id FROM %T x JOIN %T c + ON x.phid = c.transactionPHID + WHERE x.objectPHID = %s + ORDER BY c.id DESC LIMIT 1', + $xaction->getTableName(), + $comment->getTableName(), + $object->getPHID()); + if (!$comment_row) { + return 'none'; + } + + return $comment_row['id']; + } + + } diff --git a/src/applications/search/engineextension/PhabricatorSearchIndexVersionDestructionEngineExtension.php b/src/applications/search/engineextension/PhabricatorSearchIndexVersionDestructionEngineExtension.php new file mode 100644 index 0000000000..83112dfd50 --- /dev/null +++ b/src/applications/search/engineextension/PhabricatorSearchIndexVersionDestructionEngineExtension.php @@ -0,0 +1,25 @@ +establishConnection('w'), + 'DELETE FROM %T WHERE objectPHID = %s', + $table->getTableName(), + $object->getPHID()); + } + +} diff --git a/src/applications/search/index/PhabricatorIndexEngine.php b/src/applications/search/index/PhabricatorIndexEngine.php index dda9af0d41..1dde3ce9ab 100644 --- a/src/applications/search/index/PhabricatorIndexEngine.php +++ b/src/applications/search/index/PhabricatorIndexEngine.php @@ -45,8 +45,8 @@ final class PhabricatorIndexEngine extends Phobject { if (idx($parameters, 'force')) { $current_versions = array(); } else { - // TODO: Load current indexed versions. - $current_versions = array(); + $keys = array_keys($versions); + $current_versions = $this->loadIndexVersions($keys); } foreach ($versions as $key => $version) { @@ -78,7 +78,7 @@ final class PhabricatorIndexEngine extends Phobject { $extension->indexObject($this, $object); } - // TODO: Save new index versions. + $this->saveIndexVersions($this->versions); return $this; } @@ -96,4 +96,55 @@ final class PhabricatorIndexEngine extends Phobject { return $extensions; } + private function loadIndexVersions(array $extension_keys) { + if (!$extension_keys) { + return array(); + } + + $object = $this->getObject(); + $object_phid = $object->getPHID(); + + $table = new PhabricatorSearchIndexVersion(); + $conn_r = $table->establishConnection('w'); + + $rows = queryfx_all( + $conn_r, + 'SELECT * FROM %T WHERE objectPHID = %s AND extensionKey IN (%Ls)', + $table->getTableName(), + $object_phid, + $extension_keys); + + return ipull($rows, 'version', 'extensionKey'); + } + + private function saveIndexVersions(array $versions) { + if (!$versions) { + return; + } + + $object = $this->getObject(); + $object_phid = $object->getPHID(); + + $table = new PhabricatorSearchIndexVersion(); + $conn_w = $table->establishConnection('w'); + + $sql = array(); + foreach ($versions as $key => $version) { + $sql[] = qsprintf( + $conn_w, + '(%s, %s, %s)', + $object_phid, + $key, + $version); + } + + queryfx( + $conn_w, + 'INSERT INTO %T (objectPHID, extensionKey, version) + VALUES %Q + ON DUPLICATE KEY UPDATE version = VALUES(version)', + $table->getTableName(), + implode(', ', $sql)); + } + } diff --git a/src/applications/search/storage/PhabricatorSearchIndexVersion.php b/src/applications/search/storage/PhabricatorSearchIndexVersion.php new file mode 100644 index 0000000000..702b1ea4d6 --- /dev/null +++ b/src/applications/search/storage/PhabricatorSearchIndexVersion.php @@ -0,0 +1,26 @@ + false, + self::CONFIG_COLUMN_SCHEMA => array( + 'extensionKey' => 'text64', + 'version' => 'text128', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_object' => array( + 'columns' => array('objectPHID', 'extensionKey'), + 'unique' => true, + ), + ), + ) + parent::getConfiguration(); + } + +}