jenkins-bot has submitted this change and it was merged.
Change subject: (bug 31044) Make ResourceLoader behave in read-only mode
......................................................................
(bug 31044) Make ResourceLoader behave in read-only mode
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
---
M includes/MessageBlobStore.php
M includes/resourceloader/ResourceLoader.php
M includes/resourceloader/ResourceLoaderFileModule.php
3 files changed, 144 insertions(+), 116 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/MessageBlobStore.php b/includes/MessageBlobStore.php
index 34014e1..c96ea56 100644
--- a/includes/MessageBlobStore.php
+++ b/includes/MessageBlobStore.php
@@ -80,42 +80,45 @@
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' )
- );
+ 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' )
+ );
- 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();
+ 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
+ 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;
}
@@ -140,49 +143,52 @@
// Save the old and new blobs for later
$oldBlob = $row->mr_blob;
$newBlob = self::generateMessageBlob( $module, $lang );
-
- $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__
- );
+ try {
+ $newRow = array(
+ 'mr_resource' => $name,
+ 'mr_lang' => $lang,
+ 'mr_blob' => $newBlob,
+ 'mr_timestamp' => $dbw->timestamp()
+ );
- // 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 );
+ $dbw->replace( 'msg_resource',
+ array( array( 'mr_resource', 'mr_lang' ) ),
+ $newRow, __METHOD__
+ );
- // Delete removed messages, insert added ones
- if ( $removed ) {
- $dbw->delete( 'msg_resource_links', array(
+ // 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__
+ );
+ }
+
+ $newLinksRows = array();
+
+ foreach ( $added as $message ) {
+ $newLinksRows[] = array(
'mrl_resource' => $name,
- 'mrl_message' => $removed
- ), __METHOD__
- );
+ 'mrl_message' => $message
+ );
+ }
+
+ 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" );
}
-
- $newLinksRows = array();
-
- 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
- );
- }
-
return $newBlob;
}
@@ -192,50 +198,58 @@
* @param $key String: message key
*/
public static function updateMessage( $key ) {
- $dbw = wfGetDB( DB_MASTER );
+ 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 );
+ // 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__
- );
+ 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" );
+ }
}
/**
diff --git a/includes/resourceloader/ResourceLoader.php
b/includes/resourceloader/ResourceLoader.php
index 3b48a26..7b87f9d 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -176,6 +176,7 @@
} catch ( Exception $exception ) {
// Return exception as a comment
$result = $this->makeComment( $exception->__toString()
);
+ $this->hasErrors = true;
}
wfProfileOut( __METHOD__ );
@@ -434,6 +435,7 @@
wfProfileIn( __METHOD__ );
$errors = '';
+ $this->hasErrors = false;
// Split requested modules into two groups, modules and missing
$modules = array();
@@ -445,6 +447,7 @@
// 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 );
@@ -459,6 +462,7 @@
} catch( Exception $e ) {
// Add exception to the output as a comment
$errors .= $this->makeComment( $e->__toString() );
+ $this->hasErrors = true;
}
wfProfileIn( __METHOD__.'-getModifiedTime' );
@@ -476,13 +480,11 @@
} 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 ) ) {
@@ -500,6 +502,7 @@
// 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
@@ -514,6 +517,9 @@
}
}
+ // 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;
@@ -525,13 +531,15 @@
* 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
@@ -679,6 +687,7 @@
} catch ( Exception $e ) {
// Add exception to the output as a comment
$exceptions .= $this->makeComment(
$e->__toString() );
+ $this->hasErrors = true;
}
} else {
$blobs = array();
@@ -784,6 +793,7 @@
} 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;
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php
b/includes/resourceloader/ResourceLoaderFileModule.php
index d0c56ae..8b9b727 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -308,15 +308,19 @@
// 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;
}
--
To view, visit https://gerrit.wikimedia.org/r/62534
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idc83a0fe042806263f9337c40ade8c38c56aa3cd
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_20
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
