Page MenuHomePhabricator

Fixing the timestamp checks in ApiStashEdit::checkCache() made page save performance substantially worse
Closed, ResolvedPublic

Description

The edit-stashing code in core was designed to be conservative with respect to consistency. Stashed edits were intended to rapidly expire, to ensure that what we commit to the database when a user saves an edit reflects not just the direct changes made by the editor herself but also any immediately preceding changes to the templates and files used on the page.

Due to a bug (cf. T133332), the actual behavior of the code did not match the design: stashed edits expired after five minutes (rather than three seconds, which was the intended expiration). When this was fixed in rMWeb06f5cebad3: Fix timestamp check in ApiStashEdit::checkCache, page save time increased by approximately 15%, or ~100-150ms.

This is tough to swallow, because we have no indication that the previous behavior had a negative impact on editors, and a very clear indication that the current behavior does.

We should consider whether a more relaxed timestamp constraint really does pose a credible risk to user experience or the integrity of content. If it does not, we should revert to the old behavior.

Event Timeline

Change 287306 had a related patch set uploaded (by Aaron Schulz):
Make stashEditFromPreview() call setCacheTime()

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

Change 287306 merged by jenkins-bot:
Make stashEditFromPreview() call setCacheTime()

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

Change 287398 had a related patch set uploaded (by Ori.livneh):
Make stashEditFromPreview() call setCacheTime()

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

Change 287399 had a related patch set uploaded (by Ori.livneh):
Make stashEditFromPreview() call setCacheTime()

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

Change 287398 merged by jenkins-bot:
Make stashEditFromPreview() call setCacheTime()

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

Change 287399 merged by jenkins-bot:
Make stashEditFromPreview() call setCacheTime()

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

aaron triaged this task as High priority.May 10 2016, 12:25 AM

Change 288700 had a related patch set uploaded (by Aaron Schulz):
Make "presumed-fresh" edit stash case cover when users make no other edits

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

Change 288700 merged by jenkins-bot:
Make "presumed-fresh" edit stash case cover when users make intervening edits

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

Change 288741 had a related patch set uploaded (by Aaron Schulz):
Make "presumed-fresh" edit stash case cover when users make intervening edits

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

Change 288741 merged by jenkins-bot:
Make "presumed-fresh" edit stash case cover when users make intervening edits

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

Change 288753 had a related patch set uploaded (by Aaron Schulz):
Improve edit stash hit rate for logged-out users

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

Change 288753 merged by jenkins-bot:
Improve edit stash hit rate for logged-out users

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

Change 289310 had a related patch set uploaded (by Ori.livneh):
Improve edit stash hit rate for logged-out users

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

Change 289310 merged by jenkins-bot:
Improve edit stash hit rate for logged-out users

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

Change 289313 had a related patch set uploaded (by Ori.livneh):
Improve edit stash hit rate for logged-out users

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

Change 289313 merged by jenkins-bot:
Improve edit stash hit rate for logged-out users

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

Closing this and spinning off T136678 for general hit rate improvements (the proven_stale rate is very tiny now).