Page MenuHomePhabricator

When the "FeedQuery" on user profile pages overheats, the event is not contained to the reactor core
Closed, ResolvedPublic

Description

See https://discourse.phabricator-community.org/t/single-user-cant-see-profile-page/2955. When a user profile page feed query overheats, it takes the whole page down with it. This is mostly-anticipated fallout from D20294.

Event Timeline

epriestley created this task.

To reproduce this easily:

  • Force every query to overheat, e.g. with the patch below.
  • Load any user profile page.
diff --git a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
index 8780584f94..d7d973b983 100644
--- a/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
+++ b/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php
@@ -332,7 +332,7 @@ abstract class PhabricatorPolicyAwareQuery extends PhabricatorOffsetPagedQuery {
       }
 
       if (!$this->disableOverheating) {
-        if ($overheat_limit && ($total_seen >= $overheat_limit)) {
+        if ($overheat_limit && ($total_seen >= $overheat_limit) || true) {
           $this->isOverheated = true;
 
           if (!$this->returnPartialResultsOnOverheat) {

Screen Shot 2019-07-23 at 3.50.29 PM.png (1×2 px, 312 KB)

The "natural" reproduction case is something like:

  • As Alice, push 1,100 commits to repository X.
  • As Bailey, set the visibility for repository X to "Only Bailey".
  • As any user except Bailey, view Alice's profile.

A more exciting alternate version fo "set the visibility of repository X..." might be "destroy repository X". Currently, we do not have a DestructionEngineExtension for destroying feed stories when an object is destroyed. This is perhaps worth building, but something of an edge case, and the table is GC'd anyway.