aaron(a)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();
> }
> } else {
> # Create new article
>
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