I don't see a problem with selectDomain() itself. It looks like a query in EchoUserLocator::locateArticleCreator was really slow and triggered an excimer timeout.

This happened within DeferredUpdates, which tries to catch errors and move on to other updates, which included more database updates. TimeoutExceptions should probably make DeferredUpdates abort the remaining updates so things can shutdown. The main point of the excimer timeouts is to limit run time, give a chance to log, and maybe give a clean error to the client. Catching/ignoring them undermines some of that and runs the risk of hitting uglier fpm/apache timeouts.

In T328043#8576032, @aaron wrote:

I don't see a problem with selectDomain() itself. It looks like a query in EchoUserLocator::locateArticleCreator was really slow and triggered an excimer timeout.

This happened within DeferredUpdates, which tries to catch errors and move on to other updates, […]. The main point of the excimer timeouts is to limit run time, give a chance to log, and maybe give a clean error to the client. Catching/ignoring them undermines some of that and runs the risk of hitting uglier fpm/apache timeouts.

I think if the main response portion, pre-send update, or earlier post-send update times out, we should still let try to let other updates run, especially ones that might run hooks and/or queue jobs. However, I could be convinced otherwise if that's the only benefit left.

As it stands though, I lean toward not changing it because it seems orthogonal to the observed problem.

• Even if we stop forego most updates after a timeout in the main portion, there may still be a trivial Rdbms query triggered by the skin or as part of processing a late hook or Job creation that needs some basic data for its parameters. This data is typically cachable in Memcached, but can just as well cache-miss.
• The problem here seems first and foremost that the Rdbms object is corrupting its state and thus not fulfilling its contract. As part of a related production error task we developed (not insignificant) complexity in the form of RequestTimeout and critical sections, to prevent this. Do we want to make that actually work or should we consider removing that if it isn't working? If remove it, how do we mitigate the production error?

The advantage of excimer timers is that they can happen at anytime in PHP code. It's tricky to avoid corrupting the state of objects though. We really should just be cleaning up and shutting down. If we want to handle it more gracefully in Database, one could:

• Rely on the fact that Zend C wrapper methods do not get preempted by excimer. DatabaseMysli::doSingleStatementQuery could catch the error, stage it, and have it rethrown (this gets more complicated with nested critical sections and/or when a section include several queries). It seems too messy.
• DeferredUpdates could use some LBFactory method to discard all DB connections when a timeout exceptions are caught.
• MWExceptionHandler::rollbackPrimaryChanges could discard such DB connections.

It could also help to:

• set max_statement_time on the session level (different for web vs job vs cli).
• set MYSQLI_OPT_READ_TIMEOUT for regular DB connections (different for web vs job vs cli). This would match the idea in CommonSettings.php for $wgHTTPMaxTimeout /$wgHTTPMaxConnectTimeout and $wgRequestTimeLimit;

MWExceptionHandler::rollbackPrimaryChanges could discard such DB connections.

Per todays triage meeting, let's go with this one.

Change 906451 had a related patch set uploaded (by Aaron Schulz; author: Aaron Schulz):

[mediawiki/core@master] rdbms: make Database::flushSession() handle critical section errors

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

Change 906451 merged by jenkins-bot:

[mediawiki/core@master] rdbms: make Database::flushSession() handle critical section errors

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