aaron@svn.wikimedia.org wrote:
Revision: 40752 Author: aaron Date: 2008-09-12 15:03:46 +0000 (Fri, 12 Sep 2008)
[...]
@@ -1519,7 +1519,6 @@ } } $user->incEditCount();
$dbw->commit(); } } else { $revision = null;@@ -1541,6 +1540,7 @@
# Update links tables, site stats, etc. $this->editUpdates( $text, $summary, $isminor, $now, $revisionId, $changed );
$dbw->commit(); }
This causes a regression to a situation we saw a few years ago, when as a team, we were still working out how to optimise MySQL locking. Updates to the site_stats table are serialized while waiting for various other updates and PHP code to complete, limiting the total possible edit rate of the wiki and risking telescoping lock contention and total site failure. This is exactly what happened on Saturday, except that we were luckier than we were in the past, and the ES master failed before the core master, so we still had r/o service.
There are good reasons why the transaction brackets are the way they are. Please don't change any more of them without discussing the issues in depth with Brion or Domas or me.
This revision apparently also caused bug 15656. doEdit() is vulnerable to duplicate revision insertion between the first getContent() call and the commit. The duration of this critical section was greatly increased by this revision. The root problem is that commitRollback() determines the last editor using Title::newFromRevision(), which only uses the slave, so the window in which duplicate rollbacks are possible can easily stretch out to seconds. But with a short main transaction, the effect was a silently ignored null edit, rather than a duplicate entry in the page history.
-- Tim Starling