Matching the actual cap with the largest number we display makes sense to me. So 500? Or 1000, but then also show a link for 1000?

Also, we could not seem to find an option to configure this limit in mediawiki-config. It would be desirable to be able to change it without having to make a change in core. (cc: @RLazarus)

As a temporary fix @BBlack deployed a VCL change that blocks '&limit=2000&action=history'.

As Legoktm had pointed out on IRC the 5000 limit is at includes/pager/IndexPager.php#325.

I regularly use limits of 5000 in all the UIs where I can. I think in one of the special pages it was recently changed to a cap of 1000 or so and it is annoying 😅. It would be preferable to leave it be, at least for the users with apihighlimits permission (even though it is not API, but UI) or something.

I have also used limit=5000 many times (e.g. when searching for edits to a page that touched a specific section, based on the edit summary). The page would take a bit longer to display but never timed out. Maybe this is a recent performance regression?

It may or may not be a regression -- we were getting a fair amount of this traffic during the incident, not just isolated requests. One hypothesis is that we can handle a single heavy history query from the occasional power user here and there, but even a smallish distributed flood of them still constitutes a DOS vector.

In principle we could continue to safely offer the high cap, by adding a fairly sophisticated rate limiter that accounts for request cost, so that expensive requests like this could be charged against a much lower rate limit than ordinary traffic (but still nonzero). That could be based on anything from a rough classification into discrete categories of "small" and "large" requests, to a fully dynamic token-bucket system where tokens are withdrawn according to actual measured resource consumption.

We've kicked those ideas around a few times before, but we don't have the apparatus to build them out properly right now. In the short-to-medium term, if we can make this query more performant, so much the better -- but failing that, I think we need to at least add the config knob @Dzahn mentions, so that we can respond when it's used as an attack vector.

We could also throttle it in MW with User::pingLimiter - it stores throttles in ObjectCache::getLocalClusterInstance.
The throttler is not called from the endpoint in question yet, so it needs a core change. After that the settings are configurable and pretty flexible https://www.mediawiki.org/wiki/Manual:$wgRateLimits

In T284274#7134441, @matmarex wrote:

I have also used limit=5000 many times (e.g. when searching for edits to a page that touched a specific section, based on the edit summary). The page would take a bit longer to display but never timed out. Maybe this is a recent performance regression?

The excimer 60 second timeout is somewhat new. Looking through the timeout tracebacks in logstash there are two common ones:

#10 /srv/mediawiki/php-1.37.0-wmf.7/includes/cache/LinkCache.php(350): Wikimedia\Rdbms\DBConnRef->selectRow(string, array, array, string)
#11 /srv/mediawiki/php-1.37.0-wmf.7/includes/cache/LinkCache.php(299): LinkCache->fetchPageRow(Wikimedia\Rdbms\DBConnRef, Title)
#12 /srv/mediawiki/php-1.37.0-wmf.7/includes/linker/LinkRenderer.php(428): LinkCache->addLinkObj(Title)
#13 /srv/mediawiki/php-1.37.0-wmf.7/includes/linker/LinkRenderer.php(244): MediaWiki\Linker\LinkRenderer->getLinkClasses(Title)
#14 /srv/mediawiki/php-1.37.0-wmf.7/includes/actions/pagers/HistoryPager.php(567): MediaWiki\Linker\LinkRenderer->makeKnownLink(Title, HtmlArmor, array, array)
#15 /srv/mediawiki/php-1.37.0-wmf.7/includes/actions/pagers/HistoryPager.php(362): HistoryPager->curLink(MediaWiki\Revision\RevisionStoreRecord)
#16 /srv/mediawiki/php-1.37.0-wmf.7/includes/actions/pagers/HistoryPager.php(159): HistoryPager->historyLine(stdClass, stdClass, boolean, boolean, boolean)

Why isn't this title being batched and already in the LinkCache?

And then inserting the Thanks link:

#13 /srv/mediawiki/php-1.37.0-wmf.7/extensions/Thanks/includes/Hooks.php(64): MediaWiki\Revision\RevisionStore->getPreviousRevision(MediaWiki\Revision\RevisionStoreRecord)
#14 /srv/mediawiki/php-1.37.0-wmf.7/includes/HookContainer/HookContainer.php(330): MediaWiki\Extension\Thanks\Hooks::insertThankLink(MediaWiki\Revision\RevisionStoreRecord, array, MediaWiki\Revision\RevisionStoreRecord, User)
#15 /srv/mediawiki/php-1.37.0-wmf.7/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#16 /srv/mediawiki/php-1.37.0-wmf.7/includes/HookContainer/HookRunner.php(1998): MediaWiki\HookContainer\HookContainer->run(string, array)
#17 /srv/mediawiki/php-1.37.0-wmf.7/includes/actions/pagers/HistoryPager.php(494): MediaWiki\HookContainer\HookRunner->onHistoryTools(MediaWiki\Revision\RevisionStoreRecord, array, MediaWiki\Revision\RevisionStoreRecord, User)
#18 /srv/mediawiki/php-1.37.0-wmf.7/includes/actions/pagers/HistoryPager.php(159): HistoryPager->historyLine(stdClass, stdClass, boolean, boolean, boolean)

I'm not really opposed to adding a "uihighlimits" to match "apihighlimits" as @Base suggested but it doesn't feel super great.

In T284274#7135279, @Pchelolo wrote:

We could also throttle it in MW with User::pingLimiter - it stores throttles in ObjectCache::getLocalClusterInstance.
The throttler is not called from the endpoint in question yet, so it needs a core change. After that the settings are configurable and pretty flexible https://www.mediawiki.org/wiki/Manual:$wgRateLimits

I'm not sure this would work since the requests were coming from all over the place, not just one IP or a single range of IPs.

Does this task really need the view policy when apparently everyone knows about it already?

In T284274#7140940, @tstarling wrote:

Does this task really need the view policy when apparently everyone knows about it already?

I made it private because it triggered pages and we responded to it in an incident-like fashion. I'll make it public now though.

In T284274#7139737, @Legoktm wrote:

Why isn't this title being batched and already in the LinkCache?

History page calls RevisionStore::getRelativeRevision whichs calls Title::castFromPageReference and that ends up in Title::resetArticleID which removes the (preloaded) link via LinkCache::clearLink

Looks like a issue from https://gerrit.wikimedia.org/r/c/mediawiki/core/+/673140 where a newFromRow is replaced by the cast, newFromRow does not reset id

I used https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/701995 to generate a log of queries. I logged about 350 queries for an English Wikipedia history view with limit=50. It's pretty bad. I'm treating this bug as "history is slow" not "5000 is too many", at least until we've addressed some of the low-hanging fruit.

Change 702000 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@master] Add statsd timing for actions

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

Change 702005 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/extensions/Thanks@master] Fix history tools performance

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

Change 702000 merged by jenkins-bot:

[mediawiki/core@master] Add statsd timing for actions

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

Change 702010 had a related patch set uploaded (by Tim Starling; author: Tim Starling):

[mediawiki/core@wmf/1.37.0-wmf.11] Add statsd timing for actions

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

Change 702010 merged by jenkins-bot:

[mediawiki/core@wmf/1.37.0-wmf.11] Add statsd timing for actions

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

Mentioned in SAL (#wikimedia-operations) [2021-06-29T06:46:18Z] <tstarling@deploy1002> Synchronized php-1.37.0-wmf.11/includes/MediaWiki.php: Add statsd action timing metric T284274 (duration: 00m 58s)

Change 702005 merged by jenkins-bot:

[mediawiki/extensions/Thanks@master] Fix history tools performance

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

Low-hanging fruit in ascending order by altitude:

• Thanks (done above).
• GlobalUserPage (T101348). I wrote up some ideas at T88644.
• Linker::formatComment(). Maybe it just looks high up because nobody has touched it for so long. We need to be able to format a batch of comments, instead of just doing one at a time, so that we can do batch existence checks. The issue dates back to 2004, but I don't think there's a bug for it.

• Linker::formatComment(). Maybe it just looks high up because nobody has touched it for so long. We need to be able to format a batch of comments, instead of just doing one at a time, so that we can do batch existence checks. The issue dates back to 2004, but I don't think there's a bug for it.

I created T285917 for this.