[wp-trac] [WordPress Trac] #34560: Server error after saving many revisions of a large page in editor & possible fix

[WordPress Trac]

#34560: Server error after saving many revisions of a large page in editor &
possible fix
-------------------------+------------------------------------
 Reporter:  pdfernhout   |      Owner:
     Type:  enhancement  |     Status:  new
 Priority:  normal       |  Milestone:  Awaiting Review
Component:  Editor       |    Version:  4.3.1
 Severity:  normal       |   Keywords:  efficiency scalability
  Focuses:               |
-------------------------+------------------------------------
 **Version:** This issue was seen under WordPress 4.3.1, but it likely
 applies to any modern version of WordPress, even as far back as
 [https://github.com/WordPress/WordPress/blob/2.9-branch/wp-admin/edit-
 form-advanced.php 2.9] or maybe earlier.

 **Expected Behavior:** A WordPress user should be able to happily create
 thousands of revisions of pages (or blog posts) containing megabytes of
 HTML text, limited only by the amount of server memory needed to contain
 the specific edited revision of the page or post (minus baseline memory
 needs). So, for example, if you have a 256MB PHP memory limit, you should
 be able in theory to edit pages of 100MB or more for thousands of
 revisions (ignoring non-memory constraints like CPU usage, database
 configuration, communication timeouts, or browser-side limitations).
 Certainly you should be able to edit a page or post of under 1MB for
 hundreds of revisions given 256MB of allowed PHP memory (or even just with
 a 32MB memory limit given few plugins). While editing in WordPress will
 have limits based on available memory or CPU use or communications
 timeouts and the memory demand of the current revision being edited, those
 editing limits should not be substantially affected by the number of
 revisions of a post previously saved or the sizes of previously saved
 revisions.

 **Exhibited Behavior:** Right now, you will unhappily see "500 server
 error" and "white screen of death" results eventually if you, say, edit a
 400K page for hundreds of revisions, even with a 256MB server memory
 limit. Here is an example error from a server with 256MB memory limit
 resulting from trying to edit a page after saving about 350 revisions of a
 page that grew to be 437,042 bytes in size:
 {{{
 [02-Nov-2015 14:40:42 UTC] PHP Fatal error:  Allowed memory size of
 268435456 bytes exhausted (tried to allocate 437043 bytes) in
 /usr/home/narrafirma/public_html/narrafirma.com/wp-includes/wp-db.php on
 line 1389
 }}}

 **Steps to reproduce:** Create a new WordPress page. Put 500K or so of
 text in it (or any other large amount, the smaller the amount, the more
 revisions needed to be saved). Save the page for hundreds of revisions.
 Failure will occur when the number of revisions times the post size
 exceeds the PHP available memory minus the core memory need of
 WordPress+plugins. To speed this failure process, reduce the memory
 available to PHP.

 **Likely cause:** The file edit-form-advanced.php has the following
 section of code which retrieves all revisions for a post using
 wp_get_post_revisions and then counts the result (twice) to display a
 count of revisions on the page. While the current approach has a certain
 elegant simplicity and is easy to understand, the approach is
 unfortunately computationally inefficient because retrieving all the
 revisions of a post just to count them potentially consumes a lot of
 memory, a lot of CPU time, and a lot of database bandwidth when there are
 many revisions of a post if some are of significant size.

 From [https://github.com/WordPress/WordPress/blob/4.3.1/wp-admin/edit-
 form-advanced.php edit-form-advanced.php (4.3.1)]:
 {{{
 $publish_callback_args = null;
 if ( post_type_supports($post_type, 'revisions') && 'auto-draft' !=
 $post->post_status ) {
         $revisions = wp_get_post_revisions( $post_ID );

         // We should aim to show the revisions metabox only when there are
 revisions.
         if ( count( $revisions ) > 1 ) {
                 reset( $revisions ); // Reset pointer for key()
                 $publish_callback_args = array( 'revisions_count' =>
 count( $revisions ), 'revision_id' => key( $revisions ) );
                 add_meta_box('revisionsdiv', __('Revisions'),
 'post_revisions_meta_box', null, 'normal', 'core');
         }
 }
 }}}
 **Suggested fix:** Retrieve the count of revisions in some other way with
 custom SQL. Alternatively, add an option in when retrieving revisions via
 "wp_get_post_revisions" (in "wp-includes/revision.php") to only retrieve
 the metadata for the revision (like size, timestamp, user, and so on) and
 use that option in this code. Incidentally, another minor optimization for
 this code fragment may be to store the count of revisions in the code
 above so the counting process is not done twice (depending perhaps on how
 count is implemented versus variable allocation costs in PHP).

 **Caveats:** I have only tested this error condition for a page, but I am
 assuming this issue will apply to blog posts as well (or any kind of posts
 with revisions). I am not sure what other implications there would be from
 such a change, like if the revisions' contents were needed elsewhere later
 for comparisons -- although such a situation could potentially be dealt
 with by selectively loading specific revision's contents as needed for
 diff comparisons, even if all the metadata might be needed up front.

 **Other potential benefits of the proposed fix:** Beyond avoiding server
 errors and white screens, making the proposed change may speed up opening
 long posts and pages for editing by reducing server load substantially.
 When editing even small posts with a few revisions, this change will still
 likely speed up the initial opening of the editor at least slightly.

 **Use cases:** An example situations of a large page with multiple
 revisions is where a team uses a WordPress page to track component version
 deployment status of a large system. Another use case is when a WordPress
 page grows into a book-length document through repeated editing by someone
 :-) who is also used to saving a lot to avoid losing work from browser
 crashes.

 **Additional suggestion for further code review:** All uses of
 "wp_get_post_revisions" and any similar functions, as well as all uses of
 "count", could be reviewed in the WordPress codebase looking for similar
 computational inefficiencies.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/34560>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform