Page MenuHomePhabricator

Add an option to make it easier to debug page hangs
ClosedPublic

Authored by epriestley on Sep 10 2014, 7:35 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 14, 12:35 AM
Unknown Object (File)
Fri, Apr 12, 5:52 PM
Unknown Object (File)
Thu, Apr 11, 10:46 AM
Unknown Object (File)
Fri, Mar 29, 5:49 PM
Unknown Object (File)
Fri, Mar 29, 12:39 PM
Unknown Object (File)
Mar 13 2024, 6:19 PM
Unknown Object (File)
Mar 13 2024, 6:19 PM
Unknown Object (File)
Mar 13 2024, 6:19 PM

Details

Summary

Fixes T6044. We've had two cases (both the same install, coincidentally) where pages got hung doing too much data fetching.

When pages hang, we don't get a useful stack trace out of them, since nginx, php-fpm, or PHP eventually terminates things in a non-useful way without any diagnostic information.

The second time (the recent Macros issue) I was able to walk the install through removing limits on nginx, php-fpm, php, and eventually getting a profile by letting the page run for several minutes until the request completed. However, this install is exceptionally technically proficient and this was still a big pain for everyone, and this approach would not have worked if the page actually looped rather than just taking a long time.

Provide debug.time-limit, which should give us a better tool for reacting to this situation: by setting it to a small value (like 10), we'll kill the page after 10 seconds with a trace, before nginx/php-fpm/php/etc can kill it uselessly. Hopefully that will be enough information to find the issue (generally, getting a trace has been 95% of the problem in the two cases we've encountered).

Test Plan

Set this option to 3 and added a sleep loop, saw a termination after 3 seconds with a useful trace.

Diff Detail

Repository
rP Phabricator
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley retitled this revision from to Add an option to make it easier to debug page hangs.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
epriestley added a subscriber: joshuaspence.
  • Same diff, just taking advantage of @joshuaspence's change to fix detection of function calls in if(function_exists(...)) blocks.

Cool, I'm an install!

This looks right to me, but since I don't know php I'll leave it at that (versus actually, say, approving or something :-) )

Haha. I try to just stick to neutral language since some installs don't want us to disclose that they use Phabicator, and others we haven't asked and aren't sure about. It's usually not too helpful to know which install specifically is the culprit/beneficiary of a change.

support/PhabricatorStartup.php
383

Great success!

support/PhabricatorStartup.php
383

awwyiss

btrahan edited edge metadata.
This revision is now accepted and ready to land.Sep 10 2014, 9:31 PM
epriestley updated this revision to Diff 25192.

Closed by commit rPcae59d83456e (authored by @epriestley).