(bug 31044) Make ResourceLoader behave in read-only mode
authorCatrope <roan.kattouw@gmail.com>
Tue, 15 Jan 2013 23:32:10 +0000 (15:32 -0800)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 16 Jan 2013 03:10:26 +0000 (03:10 +0000)
When the database is in read-only mode or writes fail for some other
reason, ResourceLoader should make every attempt to still serve JS and
CSS correctly.

* Surround a bunch of DB write code in MessageBlobStore.php in try-catch
  blocks.
* Surround the DB write in ResourceLoaderFileModule.php in a try-catch
  block rather than checking wfReadOnly(), because the DB may fail for
  other reasons.
* In ResourceLoader::respond() and helpers, set a short cache timeout on
  responses that include commented-out error messages.

Change-Id: Idc83a0fe042806263f9337c40ade8c38c56aa3cd

includes/MessageBlobStore.php
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderFileModule.php

index 09561bd..6322be7 100644 (file)
@@ -80,42 +80,45 @@ class MessageBlobStore {
                        return false;
                }
 
-               $dbw = wfGetDB( DB_MASTER );
-               $success = $dbw->insert( 'msg_resource', array(
-                               'mr_lang' => $lang,
-                               'mr_resource' => $name,
-                               'mr_blob' => $blob,
-                               'mr_timestamp' => $dbw->timestamp()
-                       ),
-                       __METHOD__,
-                       array( 'IGNORE' )
-               );
-
-               if ( $success ) {
-                       if ( $dbw->affectedRows() == 0 ) {
-                               // Blob was already present, fetch it
-                               $blob = $dbw->selectField( 'msg_resource', 'mr_blob', array(
-                                               'mr_resource' => $name,
-                                               'mr_lang' => $lang,
-                                       ),
-                                       __METHOD__
-                               );
-                       } else {
-                               // Update msg_resource_links
-                               $rows = array();
+               try {
+                       $dbw = wfGetDB( DB_MASTER );
+                       $success = $dbw->insert( 'msg_resource', array(
+                                       'mr_lang' => $lang,
+                                       'mr_resource' => $name,
+                                       'mr_blob' => $blob,
+                                       'mr_timestamp' => $dbw->timestamp()
+                               ),
+                               __METHOD__,
+                               array( 'IGNORE' )
+                       );
 
-                               foreach ( $module->getMessages() as $key ) {
-                                       $rows[] = array(
-                                               'mrl_resource' => $name,
-                                               'mrl_message' => $key
+                       if ( $success ) {
+                               if ( $dbw->affectedRows() == 0 ) {
+                                       // Blob was already present, fetch it
+                                       $blob = $dbw->selectField( 'msg_resource', 'mr_blob', array(
+                                                       'mr_resource' => $name,
+                                                       'mr_lang' => $lang,
+                                               ),
+                                               __METHOD__
+                                       );
+                               } else {
+                                       // Update msg_resource_links
+                                       $rows = array();
+
+                                       foreach ( $module->getMessages() as $key ) {
+                                               $rows[] = array(
+                                                       'mrl_resource' => $name,
+                                                       'mrl_message' => $key
+                                               );
+                                       }
+                                       $dbw->insert( 'msg_resource_links', $rows,
+                                               __METHOD__, array( 'IGNORE' )
                                        );
                                }
-                               $dbw->insert( 'msg_resource_links', $rows,
-                                       __METHOD__, array( 'IGNORE' )
-                               );
                        }
+               } catch ( Exception $e ) {
+                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
                }
-
                return $blob;
        }
 
@@ -141,48 +144,51 @@ class MessageBlobStore {
                $oldBlob = $row->mr_blob;
                $newBlob = self::generateMessageBlob( $module, $lang );
 
-               $newRow = array(
-                       'mr_resource' => $name,
-                       'mr_lang' => $lang,
-                       'mr_blob' => $newBlob,
-                       'mr_timestamp' => $dbw->timestamp()
-               );
+               try {
+                       $newRow = array(
+                               'mr_resource' => $name,
+                               'mr_lang' => $lang,
+                               'mr_blob' => $newBlob,
+                               'mr_timestamp' => $dbw->timestamp()
+                       );
 
-               $dbw->replace( 'msg_resource',
-                       array( array( 'mr_resource', 'mr_lang' ) ),
-                       $newRow, __METHOD__
-               );
+                       $dbw->replace( 'msg_resource',
+                               array( array( 'mr_resource', 'mr_lang' ) ),
+                               $newRow, __METHOD__
+                       );
 
-               // Figure out which messages were added and removed
-               $oldMessages = array_keys( FormatJson::decode( $oldBlob, true ) );
-               $newMessages = array_keys( FormatJson::decode( $newBlob, true ) );
-               $added = array_diff( $newMessages, $oldMessages );
-               $removed = array_diff( $oldMessages, $newMessages );
+                       // Figure out which messages were added and removed
+                       $oldMessages = array_keys( FormatJson::decode( $oldBlob, true ) );
+                       $newMessages = array_keys( FormatJson::decode( $newBlob, true ) );
+                       $added = array_diff( $newMessages, $oldMessages );
+                       $removed = array_diff( $oldMessages, $newMessages );
 
-               // Delete removed messages, insert added ones
-               if ( $removed ) {
-                       $dbw->delete( 'msg_resource_links', array(
-                                       'mrl_resource' => $name,
-                                       'mrl_message' => $removed
-                               ), __METHOD__
-                       );
-               }
+                       // Delete removed messages, insert added ones
+                       if ( $removed ) {
+                               $dbw->delete( 'msg_resource_links', array(
+                                               'mrl_resource' => $name,
+                                               'mrl_message' => $removed
+                                       ), __METHOD__
+                               );
+                       }
 
-               $newLinksRows = array();
+                       $newLinksRows = array();
 
-               foreach ( $added as $message ) {
-                       $newLinksRows[] = array(
-                               'mrl_resource' => $name,
-                               'mrl_message' => $message
-                       );
-               }
+                       foreach ( $added as $message ) {
+                               $newLinksRows[] = array(
+                                       'mrl_resource' => $name,
+                                       'mrl_message' => $message
+                               );
+                       }
 
-               if ( $newLinksRows ) {
-                       $dbw->insert( 'msg_resource_links', $newLinksRows, __METHOD__,
-                                array( 'IGNORE' ) // just in case
-                       );
+                       if ( $newLinksRows ) {
+                               $dbw->insert( 'msg_resource_links', $newLinksRows, __METHOD__,
+                                       array( 'IGNORE' ) // just in case
+                               );
+                       }
+               } catch ( Exception $e ) {
+                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
                }
-
                return $newBlob;
        }
 
@@ -192,50 +198,58 @@ class MessageBlobStore {
         * @param $key String: message key
         */
        public static function updateMessage( $key ) {
-               $dbw = wfGetDB( DB_MASTER );
-
-               // Keep running until the updates queue is empty.
-               // Due to update conflicts, the queue might not be emptied
-               // in one iteration.
-               $updates = null;
-               do {
-                       $updates = self::getUpdatesForMessage( $key, $updates );
-
-                       foreach ( $updates as $k => $update ) {
-                               // Update the row on the condition that it
-                               // didn't change since we fetched it by putting
-                               // the timestamp in the WHERE clause.
-                               $success = $dbw->update( 'msg_resource',
-                                       array(
-                                               'mr_blob' => $update['newBlob'],
-                                               'mr_timestamp' => $dbw->timestamp() ),
-                                       array(
-                                               'mr_resource' => $update['resource'],
-                                               'mr_lang' => $update['lang'],
-                                               'mr_timestamp' => $update['timestamp'] ),
-                                       __METHOD__
-                               );
+               try {
+                       $dbw = wfGetDB( DB_MASTER );
+
+                       // Keep running until the updates queue is empty.
+                       // Due to update conflicts, the queue might not be emptied
+                       // in one iteration.
+                       $updates = null;
+                       do {
+                               $updates = self::getUpdatesForMessage( $key, $updates );
+
+                               foreach ( $updates as $k => $update ) {
+                                       // Update the row on the condition that it
+                                       // didn't change since we fetched it by putting
+                                       // the timestamp in the WHERE clause.
+                                       $success = $dbw->update( 'msg_resource',
+                                               array(
+                                                       'mr_blob' => $update['newBlob'],
+                                                       'mr_timestamp' => $dbw->timestamp() ),
+                                               array(
+                                                       'mr_resource' => $update['resource'],
+                                                       'mr_lang' => $update['lang'],
+                                                       'mr_timestamp' => $update['timestamp'] ),
+                                               __METHOD__
+                                       );
 
-                               // Only requeue conflicted updates.
-                               // If update() returned false, don't retry, for
-                               // fear of getting into an infinite loop
-                               if ( !( $success && $dbw->affectedRows() == 0 ) ) {
-                                       // Not conflicted
-                                       unset( $updates[$k] );
+                                       // Only requeue conflicted updates.
+                                       // If update() returned false, don't retry, for
+                                       // fear of getting into an infinite loop
+                                       if ( !( $success && $dbw->affectedRows() == 0 ) ) {
+                                               // Not conflicted
+                                               unset( $updates[$k] );
+                                       }
                                }
-                       }
-               } while ( count( $updates ) );
+                       } while ( count( $updates ) );
 
-               // No need to update msg_resource_links because we didn't add
-               // or remove any messages, we just changed their contents.
+                       // No need to update msg_resource_links because we didn't add
+                       // or remove any messages, we just changed their contents.
+               } catch ( Exception $e ) {
+                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
+               }
        }
 
        public static function clear() {
                // TODO: Give this some more thought
                // TODO: Is TRUNCATE better?
-               $dbw = wfGetDB( DB_MASTER );
-               $dbw->delete( 'msg_resource', '*', __METHOD__ );
-               $dbw->delete( 'msg_resource_links', '*', __METHOD__ );
+               try {
+                       $dbw = wfGetDB( DB_MASTER );
+                       $dbw->delete( 'msg_resource', '*', __METHOD__ );
+                       $dbw->delete( 'msg_resource_links', '*', __METHOD__ );
+               } catch ( Exception $e ) {
+                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
+               }
        }
 
        /**
index b4bd98c..5abe226 100644 (file)
@@ -176,6 +176,7 @@ class ResourceLoader {
                } catch ( Exception $exception ) {
                        // Return exception as a comment
                        $result = $this->makeComment( $exception->__toString() );
+                       $this->hasErrors = true;
                }
 
                wfProfileOut( __METHOD__ );
@@ -435,6 +436,7 @@ class ResourceLoader {
 
                wfProfileIn( __METHOD__ );
                $errors = '';
+               $this->hasErrors = false;
 
                // Split requested modules into two groups, modules and missing
                $modules = array();
@@ -446,6 +448,7 @@ class ResourceLoader {
                                // This is a security issue, see bug 34907.
                                if ( $module->getGroup() === 'private' ) {
                                        $errors .= $this->makeComment( "Cannot show private module \"$name\"" );
+                                       $this->hasErrors = true;
                                        continue;
                                }
                                $modules[$name] = $this->getModule( $name );
@@ -460,6 +463,7 @@ class ResourceLoader {
                } catch( Exception $e ) {
                        // Add exception to the output as a comment
                        $errors .= $this->makeComment( $e->__toString() );
+                       $this->hasErrors = true;
                }
 
                wfProfileIn( __METHOD__.'-getModifiedTime' );
@@ -477,14 +481,12 @@ class ResourceLoader {
                        } catch ( Exception $e ) {
                                // Add exception to the output as a comment
                                $errors .= $this->makeComment( $e->__toString() );
+                               $this->hasErrors = true;
                        }
                }
 
                wfProfileOut( __METHOD__.'-getModifiedTime' );
 
-               // Send content type and cache related headers
-               $this->sendResponseHeaders( $context, $mtime );
-
                // If there's an If-Modified-Since header, respond with a 304 appropriately
                if ( $this->tryRespondLastModified( $context, $mtime ) ) {
                        wfProfileOut( __METHOD__ );
@@ -501,6 +503,7 @@ class ResourceLoader {
                // response in a comment if we're in debug mode.
                if ( $context->getDebug() && strlen( $warnings = ob_get_contents() ) ) {
                        $response = $this->makeComment( $warnings ) . $response;
+                       $this->hasErrors = true;
                }
 
                // Save response to file cache unless there are errors
@@ -515,6 +518,9 @@ class ResourceLoader {
                        }
                }
 
+               // Send content type and cache related headers
+               $this->sendResponseHeaders( $context, $mtime, $this->hasErrors );
+
                // Remove the output buffer and output the response
                ob_end_clean();
                echo $response;
@@ -526,13 +532,15 @@ class ResourceLoader {
         * Send content type and last modified headers to the client.
         * @param $context ResourceLoaderContext
         * @param $mtime string TS_MW timestamp to use for last-modified
+        * @param $error bool Whether there are commented-out errors in the response
         * @return void
         */
-       protected function sendResponseHeaders( ResourceLoaderContext $context, $mtime ) {
+       protected function sendResponseHeaders( ResourceLoaderContext $context, $mtime, $errors ) {
                global $wgResourceLoaderMaxage;
                // If a version wasn't specified we need a shorter expiry time for updates
                // to propagate to clients quickly
-               if ( is_null( $context->getVersion() ) ) {
+               // If there were errors, we also need a shorter expiry time so we can recover quickly
+               if ( is_null( $context->getVersion() ) || $errors ) {
                        $maxage  = $wgResourceLoaderMaxage['unversioned']['client'];
                        $smaxage = $wgResourceLoaderMaxage['unversioned']['server'];
                // If a version was specified we can use a longer expiry time since changing
@@ -680,6 +688,7 @@ class ResourceLoader {
                        } catch ( Exception $e ) {
                                // Add exception to the output as a comment
                                $exceptions .= $this->makeComment( $e->__toString() );
+                               $this->hasErrors = true;
                        }
                } else {
                        $blobs = array();
@@ -785,6 +794,7 @@ class ResourceLoader {
                        } catch ( Exception $e ) {
                                // Add exception to the output as a comment
                                $exceptions .= $this->makeComment( $e->__toString() );
+                               $this->hasErrors = true;
 
                                // Register module as missing
                                $missing[] = $name;
index fa84843..fad0027 100644 (file)
@@ -312,15 +312,19 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                // Collect referenced files
                $this->localFileRefs = array_unique( $this->localFileRefs );
                // If the list has been modified since last time we cached it, update the cache
-               if ( $this->localFileRefs !== $this->getFileDependencies( $context->getSkin() ) && !wfReadOnly() ) {
-                       $dbw = wfGetDB( DB_MASTER );
-                       $dbw->replace( 'module_deps',
-                               array( array( 'md_module', 'md_skin' ) ), array(
-                                       'md_module' => $this->getName(),
-                                       'md_skin' => $context->getSkin(),
-                                       'md_deps' => FormatJson::encode( $this->localFileRefs ),
-                               )
-                       );
+               try {
+                       if ( $this->localFileRefs !== $this->getFileDependencies( $context->getSkin() ) ) {
+                               $dbw = wfGetDB( DB_MASTER );
+                               $dbw->replace( 'module_deps',
+                                       array( array( 'md_module', 'md_skin' ) ), array(
+                                               'md_module' => $this->getName(),
+                                               'md_skin' => $context->getSkin(),
+                                               'md_deps' => FormatJson::encode( $this->localFileRefs ),
+                                       )
+                               );
+                       }
+               } catch ( Exception $e ) {
+                       wfDebug( __METHOD__ . " failed to update DB: $e\n" );
                }
                return $styles;
        }