Page MenuHomePhabricator

Remove search indexes upon object deletion
Needs RevisionPublic

Authored by joshuaspence on Jun 2 2015, 11:45 AM.
Tags
None
Referenced Files
F14004325: D13111.diff
Sat, Oct 26, 6:32 PM
F13963831: D13111.id31783.diff
Tue, Oct 15, 6:08 PM
Unknown Object (File)
Sep 21 2024, 6:10 AM
Unknown Object (File)
Sep 2 2024, 3:42 AM
Unknown Object (File)
Aug 22 2024, 11:07 AM
Unknown Object (File)
Aug 8 2024, 1:50 AM
Unknown Object (File)
Jul 23 2024, 4:59 AM
Unknown Object (File)
Jul 19 2024, 7:38 AM
Subscribers

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

Remove search indexes when an object is destroyed. This was hinted at by a TODO comment, and generally seems reasonable.

Test Plan
  1. Set search.elastic.host and ran ./bin/search init and ./bin/search index --all.
  2. At this point the search index exists in both MySQL and ElasticSearch.
  3. Removed an object with ./bin/remove destroy.
  4. Queried phabricator_search.search_document for the removed PHID.
  5. Queried ElasticSearch with curl -XGET http://elasticsearch:9200/phabricator/TASK/PHID-TASK-7uv6xhcexixtroqvssqw/.

Diff Detail

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

Event Timeline

joshuaspence retitled this revision from to Remove search indexes upon object deletion.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

Not really related to this diff... but maybe object destruction should be queued for taskmaster daemons instead of handled synchronously.

joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
epriestley edited edge metadata.

This stuff should be implemented as a DestructionEngineExtension now, but seems basically correct/reasonable to me.

The extension can test for $object instanceof PhabricatorFulltextInterface to detect whether or not it may have fulltext storage.

This revision now requires changes to proceed.Dec 22 2015, 4:20 PM