Adding some people from Anti-Harassment, since we're working on this

The USE INDEX statement only supports one index. However, the current indexes that exist on cu_changes (and cu_log_event / cu_private_event in the future) that would apply here are for users (rows with an actor ID), IPs and XFF IPs. This means that trying to tell the optimiser to use the index depending on the WHERE conditions does not seem to be possible.

I can see some solutions to this:

1. Use separate queries / union queries to split the queries into three (one for users, IPs and XFF IPs) that each are told to use the appropriate index
2. Determine, based on some kind of rules, which is the "best" index for the situation (i.e. which of the three to use). Then also estimate the query count and prevent it from being run if both of the following apply:
   1. The estimated count exceeds a particular count
   2. The query includes more than one of users, IPs and XFF IPs
3. Attempt to make an index that can be used over the three

Solution 1 would attempt to solve this issue by splitting the query such that the correct index can be applied for each type of target (user, IP and XFF IP). This is probably the best solution.

Solution 2 is less ideal, but would require less code. It attempts to find the best index, probably by counting the number of targets per type and then suggesting the index with that type to the optimiser. This fails when multiple types of target are included in the investigation, as the index only helps for one of the three.

Solution 3 could be done, but the root of the three indexes used by Special:CheckUser are the actor ID, IP and XFF IP.

Thanks for the suggestions @Dreamy_Jazz .

1. Use separate queries / union queries to split the queries into three (one for users, IPs and XFF IPs) that each are told to use the appropriate index

This is sort of similar to what we do in the CompareService (IPs and User agents tab). We query cu_changes for each actor separately, but with a limit of CheckUserInvestigateMaximumRowCount / count( $targets ). (We then check whether any of the targets had more rows than the limit, and show a warning listing any that did.)

It's not ideal because of (a) the missing results and (b) we're not really sorting on time any more (one prolific target's rows might all be more recent than another target's and yet they won't all be found).

I agree though that this is probably the best quick(ish) fix here.

The fundamental problem is that we want to filter on a mixture of IPs and actors, and then sort by time.

There isn't a single index that can help with all of this:

• Indexing by actor would help find the relevant actor-rows efficiently, but all other rows would still need to be scanned for the IPs, then all the relevant rows would need to be sorted by timestamp
• Similarly, indexing by IP would help find the relevant IP-rows efficiently, but all other rows would still need to be scanned for the actors, then all the relevant rows would need to be sorted by timestamp
• Indexing by (actor, IP) or (IP, actor) wouldn't help, since we have an OR condition, so the left-hand column can't be used to narrow the range
• If the engine uses merge indexes, then it can find the IP-rows and the actor-rows efficiently using two separate indexes, but it would still have to sort all the relevant rows by time

The potential for attack seems small enough here that I'd like to move this to the backlog for the moment. But if this timeout is commonly encountered and annoying users of Special:Investigate then we could rethink the priority. Is that OK with you @Niharika?

Recent example at this logstash single document had the query take 75.9 seconds.

DatGuy hit this while running checks on the English Wikipedia. Adding them to this task.

I can hit this with just adding a user with a low edit count and an IP to the investigation on enwiki. In T329200, this query will be duplicated twice so that it is run on the three result tables. As such, the performance of this query needs to improve now before changes can occur.

Proposed patch:

Code review: +2

Non-blocking comment, fine to do in a follow-up:

if ( $dbr->unionSupportsOrderAndLimit() ) {
	$queryBuilder->orderBy( 'cuc_timestamp', SelectQueryBuilder::SORT_DESC )
		->limit( 501 );
}

Maybe we could pass in max( IndexPager::mLimitsShown ) here.

Deploying this now.

Patch deployed.

Change #1025723 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@master] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025723

Change #1025622 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_42] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025622

Change #1025723 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025723

Change #1025623 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_41] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025623

Change #1025622 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_42] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025622

Change #1025623 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_41] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025623

Change #1025756 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_40] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025756

Change #1025761 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_39] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025761

Change #1025756 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_40] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025756

Change #1025761 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_39] SECURITY: Fix slow query for Special:Investigate Timeline tab

https://gerrit.wikimedia.org/r/1025761

This can be made public now.

I've tested this myself on production and verified that the query does not time out or cause a TransactionProfiler warning. However, testing this on a local wiki or patch demo will not be possible as it needs the size of the production databases to cause the problem that this security patch fixed.

Furthermore, this mode will be modified again in T347102 and other tasks. As such I am electing to skip QA, as this will be indirectly tested through the other tickets.

Need to make a patch to address the comment raised during the review of this security patch.

Change #1026097 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@master] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

https://gerrit.wikimedia.org/r/1026097

Change #1026097 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

https://gerrit.wikimedia.org/r/1026097

Change #1026041 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_42] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

https://gerrit.wikimedia.org/r/1026041

Change #1026042 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_41] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

https://gerrit.wikimedia.org/r/1026042

Change #1026043 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_40] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

https://gerrit.wikimedia.org/r/1026043

Change #1026044 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_39] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

https://gerrit.wikimedia.org/r/1026044

Change #1026176 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@master] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1026176

Change #1026176 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1026176

Above removal done in fc7f21c9 local commit on the server. Re-downgrading and unmarking as a blocker.

Change #1026041 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@REL1_42] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

Reason:

https://gerrit.wikimedia.org/r/1026041

Change #1026042 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@REL1_41] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

Reason:

https://gerrit.wikimedia.org/r/1026042

Change #1026043 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@REL1_40] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

Reason:

https://gerrit.wikimedia.org/r/1026043

Change #1026044 abandoned by Dreamy Jazz:

[mediawiki/extensions/CheckUser@REL1_39] Use IndexPager::mLimit for 'Timeline' mode SQL subquery limits

Reason:

https://gerrit.wikimedia.org/r/1026044

Change #1030878 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_42] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030878

Change #1030884 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_41] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030884

Change #1030878 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_42] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030878

Change #1030972 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_40] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030972

Change #1030973 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@REL1_39] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030973

Change #1030884 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_41] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030884

Change #1030972 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_40] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030972

Change #1030973 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_39] Skip query in 'Timeline' mode if there are no filtered targets

https://gerrit.wikimedia.org/r/1030973