Page MenuHomePhabricator

Integrate Diviner with global search
ClosedPublic

Authored by joshuaspence on Jun 1 2015, 1:32 AM.
Tags
None
Referenced Files
F14054353: D13090.diff
Sat, Nov 16, 12:45 AM
F14042569: D13090.diff
Tue, Nov 12, 4:29 AM
F14025839: D13090.diff
Thu, Nov 7, 8:48 PM
F14018558: D13090.id31589.diff
Tue, Nov 5, 3:52 PM
F14006063: D13090.id31748.diff
Mon, Oct 28, 12:25 AM
F14002853: D13090.id31748.diff
Fri, Oct 25, 11:11 PM
F13995059: D13090.id31614.diff
Wed, Oct 23, 10:24 AM
F13989198: D13090.id31603.diff
Mon, Oct 21, 6:51 PM
Subscribers

Details

Summary

Fixes T7458. Integrates Diviner into ApplicationSearch by indexing DivinerLiveBook and DivinerLiveSymbol search documents. Depends on D13157.

Test Plan

Ran ./bin/search index --all --type BOOK and ./bin/search index --all --type ATOM and then searched for various symbols via global search.

Diff Detail

Repository
rP Phabricator
Branch
master
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6418
Build 6440: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Integrate Diviner with global search.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

Looks good so far.

src/applications/diviner/search/DivinerBookSearchIndexer.php
35–37

Why not index projects in this case?

src/applications/diviner/search/DivinerBookSearchIndexer.php
35–37

IIUC, books don't have projects at the moment, unless they inherit projects from a repository.

We could maybe add projects to books, either through some edit interface or by specifying the projects in the book file.

Ohhh, I see.

We should add explicit projects to books, not inherit them from the repository.

See T6973 for some discussion of why objects don't inherit projects from related objects, e.g. T6973#91925 specifically.

joshuaspence edited edge metadata.

Further progress

joshuaspence marked 2 inline comments as done.

Fake timestamps

I'm not really sure why this isn't working...

>>> [761] <query> START TRANSACTION
<<< [761] <query> 837 us
>>> [762] <query> INSERT INTO `worker_taskdata` (`data`) VALUES ('{\"documentPHID\":\"PHID-ATOM-va4pfyiv3gaospsi7sx4\",\"context\":null}')
<<< [762] <query> 296 us
>>> [763] <query> INSERT INTO `lisk_counter` (counterName, counterValue) VALUES
          ('worker_activetask', LAST_INSERT_ID(1))
        ON DUPLICATE KEY UPDATE
          counterValue = LAST_INSERT_ID(counterValue + 1)
<<< [763] <query> 346 us
>>> [764] <query> INSERT INTO `worker_activetask` (`failureTime`, `taskClass`, `leaseOwner`, `leaseExpires`, `failureCount`, `dataID`, `priority`, `objectPHID`, `id`) VALUES (NULL, 'PhabricatorSearchWorker', NULL, NULL, '0', '10850', '4000', NULL, '10850')
<<< [764] <query> 259 us
>>> [765] <query> SAVEPOINT Aphront_Savepoint_1
<<< [765] <query> 175 us
>>> [766] <query> DELETE FROM `worker_activetask` WHERE id = 10850
<<< [766] <query> 332 us
>>> [767] <query> INSERT INTO `worker_archivetask` (`duration`, `result`, `taskClass`, `leaseOwner`, `leaseExpires`, `failureCount`, `dataID`, `priority`, `objectPHID`, `id`, `dateCreated`, `dateModified`) VALUES ('0', '0', 'PhabricatorSearchWorker', NULL, NULL, '0', '10850', '4000', NULL, '10850', '1433162891', '1433162891')
<<< [767] <query> 236 us
>>> [768] <query> COMMIT
<<< [768] <query> 1,012 us
>>> [769] <query> SELECT * FROM `diviner_livesymbol` WHERE (phid IN ('PHID-ATOM-3xsmfxfmucssmkyy3pei')) AND (isDocumentable = 1) AND (graphHash IS NOT NULL) ORDER BY `id` DESC 
<<< [769] <query> 492 us
[2015-06-01 22:48:11] PHLOG: 'Unable to build document PHID-ATOM-3xsmfxfmucssmkyy3pei with indexer DivinerAtomSearchIndexer.' at [/usr/src/phabricator/src/applications/search/index/PhabricatorSearchDocumentIndexer.php:90]
[2015-06-01 22:48:11] EXCEPTION: (Exception) Unable to load object by PHID 'PHID-ATOM-3xsmfxfmucssmkyy3pei'! at [<phabricator>/src/applications/search/index/PhabricatorSearchDocumentIndexer.php:39]
arcanist(head=master, ref.master=0b1acf0dc02a), phabricator(head=master, ref.master=02b4ce04882b), phutil(head=master, ref.master=ee6d0c3fe26e)
  #0 phlog(Exception) called at [<phabricator>/src/applications/search/index/PhabricatorSearchDocumentIndexer.php:91]
  #1 PhabricatorSearchDocumentIndexer::indexDocumentByPHID(string, NULL) called at [<phabricator>/src/applications/search/index/PhabricatorSearchIndexer.php:24]
  #2 PhabricatorSearchIndexer::indexDocumentByPHID(string, NULL) called at [<phabricator>/src/applications/search/worker/PhabricatorSearchWorker.php:12]
  #3 PhabricatorSearchWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:117]
  #4 PhabricatorWorker::scheduleTask(string, array, array) called at [<phabricator>/src/applications/search/index/PhabricatorSearchIndexer.php:14]
  #5 PhabricatorSearchIndexer::queueDocumentForIndexing(string) called at [<phabricator>/src/applications/search/management/PhabricatorSearchManagementIndexWorkflow.php:98]
  #6 PhabricatorSearchManagementIndexWorkflow::execute(PhutilArgumentParser) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:406]
  #7 PhutilArgumentParser::parseWorkflowsFull(array) called at [<phutil>/src/parser/argument/PhutilArgumentParser.php:301]
  #8 PhutilArgumentParser::parseWorkflows(array) called at [<phabricator>/scripts/search/manage_search.php:21]
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence updated this object.

Retested after landing D13157 and appears to be working.

epriestley edited edge metadata.
This revision is now accepted and ready to land.Jun 4 2015, 9:29 PM
src/applications/diviner/publisher/DivinerLivePublisher.php
130–131 ↗(On Diff #31782)

Maybe we can make this smarter? I run Diviner on a cron on our install, so this will certainly keep the daemons busy.

In theory we only rebuild atoms that (might) have changed, so this shouldn't reindex everything all the time. I'm not sure how well that theory holds up in practice.

This revision was automatically updated to reflect the committed changes.

Yeah, it seems that you are correct.