A call to Database::close() with an open transaction is probably due to some sort of uncaught exception or an HHVM fatal error.[1] Transactional databases normally roll back when a connection is closed with an open transaction rather than committing them, so MediaWiki committing them is unexpected.
We currently have four cases being checked in Database::close():
- Atomic section open → Rollback, and exception is thrown.
- Automatic transaction open, writes were made → Rollback, and exception is thrown.
- Automatic transaction open, no writes were made → Commit, no exception or log message.
- Any other transaction open → Commit, no exception but there is a log message.
In case #3 there should be no appreciable difference between committing and rolling back because no writes were done. Either way it's probably not worth throwing an exception.
Case #4 is the one that's concerning, as it seems reasonably likely to be committing incomplete database changes.
Since I don't see the log message from case #4 in the logs from the past 60 days, I think we can be reasonably sure it's only case #3 being hit (whether because of T204346 or otherwise).
So I propose we make case #4 rollback and throw an exception, while case #3 should rollback without throwing an exception.
[1]: Zend PHP doesn't run destructors on fatal errors, so LoadBalancer->__destruct() doesn't get a chance to try to call Database::close().