Page MenuHomePhorge
Create Task
Maniphest T15642

Feed Transaction Logs: Exception: Query overheated: examined more than 1,010 raw rows without finding 101 visible objects.
Open, Needs TriagePublic
Actions

Description

Steps to reproduce:

  1. PHP 8.2.10; Phorge at 91faf16cace9308440087d64f17c068e04313c50
  2. Create 900 tasks with a View Policy set to User1
  3. Log into Phorge as User2
  4. As User2, go to see all transaction logs in the Feed at http://phorge.localhost/feed/transactions/query/all/

Actual outcome:

[2023-09-21 11:08:47] EXCEPTION: (Exception) Query (of class "ManiphestTransactionQuery") overheated: examined more than 1,010 raw rows without finding 101 visible objects. at [<phorge>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:342]
arcanist(head=customOAuthUrlencodeNull, ref.master=df6c315ace5f, ref.customOAuthUrlencodeNull=c69b9749027f), phorge(head=master, ref.master=91faf16cace9)
  #0 <#2> PhabricatorPolicyAwareQuery::execute() called at [<phorge>/src/applications/feed/query/PhabricatorFeedTransactionQuery.php:107]
  #1 <#2> PhabricatorFeedTransactionQuery::loadPage() called at [<phorge>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:251]
  #2 <#2> PhabricatorPolicyAwareQuery::execute() called at [<phorge>/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php:400]
  #3 <#2> PhabricatorCursorPagedPolicyAwareQuery::executeWithCursorPager(AphrontCursorPagerView) called at [<phorge>/src/applications/search/engine/PhabricatorApplicationSearchEngine.php:1038]
  #4 <#2> PhabricatorApplicationSearchEngine::executeQuery(PhabricatorFeedTransactionQuery, AphrontCursorPagerView) called at [<phorge>/src/applications/search/controller/PhabricatorApplicationSearchController.php:256]
  #5 <#2> PhabricatorApplicationSearchController::processSearchRequest() called at [<phorge>/src/applications/search/controller/PhabricatorApplicationSearchController.php:91]
  #6 <#2> PhabricatorApplicationSearchController::processRequest() called at [<phorge>/src/aphront/AphrontController.php:29]
  #7 <#2> AphrontController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/AphrontController.php:71]
  #8 <#2> AphrontController::delegateToController(PhabricatorApplicationSearchController) called at [<phorge>/src/applications/search/engine/PhabricatorApplicationSearchEngine.php:50]
  #9 <#2> PhabricatorApplicationSearchEngine::buildResponse() called at [<phorge>/src/applications/feed/controller/PhabricatorFeedTransactionListController.php:13]
  #10 <#2> PhabricatorFeedTransactionListController::handleRequest(AphrontRequest) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:284]
  #11 phlog(Exception) called at [<phorge>/src/aphront/handler/PhabricatorDefaultRequestExceptionHandler.php:41]
  #12 PhabricatorDefaultRequestExceptionHandler::handleRequestThrowable(AphrontRequest, Exception) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:751]
  #13 AphrontApplicationConfiguration::handleThrowable(Exception) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:296]
  #14 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phorge>/src/aphront/configuration/AphrontApplicationConfiguration.php:203]
  #15 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phorge>/webroot/index.php:35]

See also:

Upstreamed from https://phabricator.wikimedia.org/T344232

Revisions and Commits

rP Phorge
AcceptedD25501 Feed: Do not query and display data of uninstalled applications
D25500rP064424c1b755 Add getQueryApplicationClass() to *TransactionQuery.php classes

Related Objects

Event Timeline

aklapper created this task.Sep 21 2023, 11:12
Herald added subscribers: Cigaryno, Matthew, chris, tobiaswiese. · View Herald TranscriptSep 21 2023, 11:12
aklapper updated the task description. (Show Details)Nov 1 2023, 22:25

However, it would make sense not to query transactions in applications which once were installed and are now uninstalled and not accessible anymore anyway.
In downstream https://phabricator.wikimedia.org/T344232 , the exception is Query (of class "ConpherenceTransactionQuery") overheated.
However we uninstalled Conpherence more than four years ago in https://phabricator.wikimedia.org/T127640 (but of course the DB data is still around).

If anyone knows how to best determine the application class related to a TransactionQuery class, please share. See this proof of concept patch (which would require adding a getApplicationClassName() method to many *TransactionQuery.php classes):

P24 T15642
1diff --git a/src/applications/feed/query/PhabricatorFeedTransactionQuery.php b/src/applications/feed/query/PhabricatorFeedTransactionQuery.php
2index d0a9f53e35..42ee332c1c 100644
3--- a/src/applications/feed/query/PhabricatorFeedTransactionQuery.php
4+++ b/src/applications/feed/query/PhabricatorFeedTransactionQuery.php
5@@ -155,6 +155,20 @@ final class PhabricatorFeedTransactionQuery
6 ->setAncestorClass('PhabricatorApplicationTransactionQuery')
7 ->execute();
8
9+ // Remove TransactionQuery classes that belong to previously installed apps
10+ // which got uninstalled in the meantime: Increases query performance and
11+ // decreases likeliness that a "Query Overheated" error will be displayed.
12+ // See https://secure.phabricator.com/T13133, https://we.phorge.it/T15642
13+ foreach ($queries as $key => $query) {
14+ if (method_exists($query, 'getApplicationClassName')) {
15+ $application_class_name = $query->getApplicationClassName();
16+ $application = new $application_class_name();
17+ if (!$application->isInstalled()) {
18+ unset($queries[$key]);
19+ }
20+ }
21+ }
22+
23 $type_map = array();
24
25 // If we're querying for specific transaction PHIDs, we only need to
26diff --git a/src/applications/maniphest/query/ManiphestTransactionQuery.php b/src/applications/maniphest/query/ManiphestTransactionQuery.php
27index 9826107b77..49efe36a20 100644
28--- a/src/applications/maniphest/query/ManiphestTransactionQuery.php
29+++ b/src/applications/maniphest/query/ManiphestTransactionQuery.php
30@@ -7,4 +7,8 @@ final class ManiphestTransactionQuery
31 return new ManiphestTransaction();
32 }
33
34+ public function getApplicationClassName() {
35+ return 'PhabricatorManiphestApplication';
36+ }
37+
38 }

valerio.bozzolan subscribed.Nov 2 2023, 08:08
20after4 subscribed.Nov 13 2023, 14:58

Maybe this is a clue?

/source/phorge/browse/master/src/applications/transactions/storage/PhabricatorModularTransactionType.php$407

protected function requireApplicationCapability($capability) {
    $application_class = $this->getEditor()->getEditorApplicationClass();
    $application = newv($application_class, array());
}

so something like this might work?

$xaction = $query->getTemplateApplicationTransaction();
$application_class = $xaction->getEditor()->getEditorApplicationClass();
$application = newv($application_class, array());
aklapper added a comment.Nov 29 2023, 19:37

so something like this might work?

I don't see how to get any editor information from within that class. (Bad getter call: getEditor)

20after4 added a comment.EditedDec 18 2023, 19:07

TLDR; I think @aklapper is on the right track: I did some more digging and it looks like something similar to P24 is probably the only reasonably easy solution.

A more complex but maybe more correct solution would be to make the PhutilClassMapQuery infrastructure to be aware of application installed/uninstalled state,
or make an alternate version like PhabricatorInstalledClassMapQuery which returns only matching classes for installed apps.

Then you could do

id(new PhabricatorInstalledClassMapQuery())
  ->setAncestorClass('PhabricatorApplicationTransactionQuery')
  ->execute();

To make any of that work though, there needs to be a strong link between all the classes related to an app. The idiomatic way to do that is apparently to add getApplicationClass method to any classes which don't already have it.

aklapper added a comment.Dec 25 2023, 20:28

Also looked into very hackish $application_of_query = preg_split('/(?=[A-Z])/', get_class($query)); but the *TransactionQuery.php class naming scheme is too inconsistent, e.g. PhabricatorSpacesTransactionQuery instead of SpacesTransactionQuery (same for Slowvote, Phurl, Owners, Badges, Calendar, Packages, Paste, maybe more).

To make any of that work though, there needs to be a strong link between all the classes related to an app. The idiomatic way to do that is apparently to add getApplicationClass method to any classes which don't already have it.

Every *TransactionQuery class extends PhabricatorApplicationTransactionQuery.
Initially I would have proposed to name it getApplicationClassName() to be consistent with all *SearchEngine.php classes.
But I found public function getQueryApplicationClass() in https://we.phorge.it/source/phorge/browse/master/src/applications/transactions/query/PhabricatorApplicationTransactionQuery.php$295-298
with a TODO comment (and getQueryApplicationClass() implementations in many *Query.php files which are not *TransactionQuery.php files), so I'd go for that name, and afterwards make that TODO function in PhabricatorApplicationTransactionQuery.php abstract.

aklapper added a revision: D25500: Add getQueryApplicationClass() to *TransactionQuery.php classes.Dec 25 2023, 20:29
aklapper mentioned this in D25500: Add getQueryApplicationClass() to *TransactionQuery.php classes.
aklapper added a revision: D25501: Feed: Do not query and display data of uninstalled applications.Dec 25 2023, 21:20
aklapper mentioned this in D25501: Feed: Do not query and display data of uninstalled applications.
aklapper added a commit: rP064424c1b755: Add getQueryApplicationClass() to *TransactionQuery.php classes.Jan 28 2024, 16:10
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0