jenkins-bot has submitted this change and it was merged.
Change subject: resourceloader: Shorten cache expiry if 'version' query doesn't
match
......................................................................
resourceloader: Shorten cache expiry if 'version' query doesn't match
Versioned load.php requests (load.php?modules=..&version=..) are highly
cacheable due to being versioned. When a module changes, the startup module
delivers new metadata to the client which naturally results in a cache miss
by producing a different url.
However, during upgrades and other deployments it was possible for a
multi-server installation to pollute edge caches with outdated content
in the following scenario:
* Deployment starts for an upgrade from version A to version B.
* Server 1 (on version B) gets a request for the startup manifest.
Client receives new manifest with version B information
and makes a versioned request for version B.
* Server 2 (still on version A) responds to this request with the
version it has (which is version A).
* Edge cache will store version A under the new url for version B.
This commit changes the last point by setting a low max-age when a request
has a 'version' hash mismatch.
Test plan:
* Look for a request in your browser like:
'/load.php?..modules=jquery%2Cmediawiki..&version=..'
* Verify it has a high Cache-Control max-age.
* Manipulate the 'version' parameter and verify it gets a low max-age.
Bug: T117587
Change-Id: Iba89c09b7b71d9fd2a8ff3ebe2618e26ea9daddf
---
M includes/resourceloader/ResourceLoader.php
1 file changed, 45 insertions(+), 9 deletions(-)
Approvals:
Aaron Schulz: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/resourceloader/ResourceLoader.php
b/includes/resourceloader/ResourceLoader.php
index 9ce2c5a..d9cd0ee 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -610,17 +610,49 @@
*
* @since 1.26
* @param ResourceLoaderContext $context
- * @param array $modules List of ResourceLoaderModule objects
+ * @param string[] $modules List of known module names
* @return string Hash
*/
- public function getCombinedVersion( ResourceLoaderContext $context,
array $modules ) {
- if ( !$modules ) {
+ public function getCombinedVersion( ResourceLoaderContext $context,
array $moduleNames ) {
+ if ( !$moduleNames ) {
return '';
}
$hashes = array_map( function ( $module ) use ( $context ) {
return $this->getModule( $module )->getVersionHash(
$context );
- }, $modules );
- return self::makeHash( implode( $hashes ) );
+ }, $moduleNames );
+ return self::makeHash( implode( '', $hashes ) );
+ }
+
+ /**
+ * Get the expected value of the 'version' query parameter.
+ *
+ * This is used by respond() to set a short Cache-Control header for
requests with
+ * information newer than the current server has. This avoids pollution
of edge caches.
+ * Typically during deployment. (T117587)
+ *
+ * This MUST match return value of `mw.loader#getCombinedVersion()`
client-side.
+ *
+ * @since 1.28
+ * @param ResourceLoaderContext $context
+ * @param string[] $modules List of module names
+ * @return string Hash
+ */
+ public function getExpectedVersionQuery( ResourceLoaderContext $context
) {
+ // As of MediaWiki 1.28, the server and client use the same
algorithm for combining
+ // version hashes. There is no technical reason for this to be
same, and for years the
+ // implementations differed. If getCombinedVersion in PHP (used
for StartupModule and
+ // E-Tag headers) differs in the future from getCombinedVersion
in JS (used for 'version'
+ // query parameter), then this method must continue to match
the JS one.
+ $moduleNames = [];
+ foreach ( $context->getModules() as $name ) {
+ if ( !$this->getModule( $name ) ) {
+ // If a versioned request contains a missing
module, the version is a mismatch
+ // as the client considered a module (and
version) we don't have.
+ return '';
+ }
+ $moduleNames[] = $name;
+ }
+ return $this->getCombinedVersion( $context, $moduleNames );
}
/**
@@ -759,10 +791,14 @@
*/
protected function sendResponseHeaders( ResourceLoaderContext $context,
$etag, $errors ) {
$rlMaxage = $this->config->get( 'ResourceLoaderMaxage' );
- // If a version wasn't specified we need a shorter expiry time
for updates
- // to propagate to clients quickly
- // If there were errors, we also need a shorter expiry time so
we can recover quickly
- if ( is_null( $context->getVersion() ) || $errors ) {
+ // Use a short cache expiry so that updates propagate to
clients quickly, if:
+ // - No version specified (shared resources, e.g. stylesheets)
+ // - There were errors (recover quickly)
+ // - Version mismatch (T117587, T47877)
+ if ( is_null( $context->getVersion() )
+ || $errors
+ || $context->getVersion() !==
$this->getExpectedVersionQuery( $context )
+ ) {
$maxage = $rlMaxage['unversioned']['client'];
$smaxage = $rlMaxage['unversioned']['server'];
// If a version was specified we can use a longer expiry time
since changing
--
To view, visit https://gerrit.wikimedia.org/r/308290
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iba89c09b7b71d9fd2a8ff3ebe2618e26ea9daddf
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: BBlack <bbl...@wikimedia.org>
Gerrit-Reviewer: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Gilles <gdu...@wikimedia.org>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
