Page MenuHomePhabricator

Bail out of PhabricatorRepositoryGraphCache more aggressively after cache fills
ClosedPublic

Authored by epriestley on Oct 6 2017, 7:02 PM.
Tags
None
Referenced Files
F14033755: D18692.diff
Sat, Nov 9, 7:50 PM
F14021990: D18692.diff
Wed, Nov 6, 2:16 PM
F14016710: D18692.id44878.diff
Mon, Nov 4, 10:35 AM
F14016709: D18692.id44877.diff
Mon, Nov 4, 10:35 AM
F14016708: D18692.id.diff
Mon, Nov 4, 10:35 AM
F14014301: D18692.diff
Sun, Nov 3, 1:53 AM
F14004883: D18692.id44877.diff
Sun, Oct 27, 4:24 AM
F14003792: D18692.id44878.diff
Sat, Oct 26, 10:50 AM
Subscribers
None

Details

Summary

Ref PHI109. Ref T11786. We currently test elapsed time every 64 iterations (since iterations are normally very fast), but at least one install is seeing the page timeout after 30 seconds.

One reason could be that cache fills may occur, and are likely to be much slower than normal iterations. In an extreme case, we could do 64 cache fills before checking the time. Tweak thing so that we always check the time after doing a cache fill, regardless of how many iterations have elapsed since the last attempt.

Additionally, this API method currently accepts an arbitrary number of paths, but implicitly limits each cache query to 500ms. If more than 60 paths are passed, this may exceed 30s. Only let the cache churn for a maximum of 10s across all paths.

If this is more the latter issue than the former, this might replace the GraphCache timeouts with git timeouts, but at least our understanding of what's going on here will improve.

Test Plan

This is difficult to test convincingly locally, since I can't reproduce the original issue. It still works after these changes, but it worked fine before these changes too.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable