Page MenuHomePhabricator

When search indexers contend for a lock, just yield
ClosedPublic

Authored by epriestley on Jun 22 2018, 5:05 PM.
Tags
None
Referenced Files
F13061669: D19504.diff
Fri, Apr 19, 7:48 PM
Unknown Object (File)
Thu, Apr 11, 8:04 AM
Unknown Object (File)
Tue, Apr 2, 8:44 PM
Unknown Object (File)
Sun, Mar 31, 9:11 AM
Unknown Object (File)
Mar 5 2024, 4:24 AM
Unknown Object (File)
Feb 21 2024, 9:18 PM
Unknown Object (File)
Feb 13 2024, 6:23 PM
Unknown Object (File)
Jan 13 2024, 3:30 AM
Subscribers
None

Details

Summary

Depends on D19503. Ref T13151. See PHI719. If you have something like a script which updates an object in a loop, we can end up queueing many search reindex tasks.

These tasks may reasonably contend for the lock, especially if the object is larger (lots of text and/or lots of comments) and indexing takes a few seconds.

This isn't concerning, and the indexers should converge to good behavior quickly once the updates stop.

Today, they'll spew a bunch of serious-looking lock exceptions into the log. Instead, just yield so it's more clear that there's (normally) no cause for concern here.

Test Plan

Ran bin/search index Txxx --force on a large object in multiple windows with a 0 second lock, saw an explicit yield instead of a lock exception.

Diff Detail

Repository
rP Phabricator
Branch
search3
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 20441
Build 27765: Run Core Tests
Build 27764: arc lint + arc unit

Event Timeline

amckinley added inline comments.
src/applications/search/worker/PhabricatorSearchWorker.php
48

I guess doing anything more complicated like exponential backoff would be overkill, and this at least ensures the indexing will happen within 15 seconds of the update torrent from stopping.

This revision is now accepted and ready to land.Jun 22 2018, 11:22 PM
This revision was automatically updated to reflect the committed changes.