User: Remove redundant wgClockSkewFudge code

Ensuring the new value is at least as high as 1 second higher
than the current value is sufficient.

The main code paths using this are checkAndSetTouched (for user group
changes) and saveSettings(), both of which use makeUpdateConditions() which
ensures we bail out if something else already wrote to it in the mean time.
As such, there is no longer a need to make sure our time is higher than
something another server may have written, given that is no longer something
we support.

This variable was introduced in 2005 (MW 1.4) with r9403 (1d12276bcb3),
and factored out as newTouchedTimestamp() in 2007 (MW 1.8)
with r16772 (c1094ba9876).

Change-Id: I940fb0dd125286a4a348c11e2c8d197f9288a75d

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index d3001f3..79b457c 100644 (file)
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -45,6 +45,12 @@ production.
   to configure a file in which to cache the SiteStore database table.
   This was never used. SiteStore already caches its information by default
   using BagOStuff (e.g. Memcached or APC).
+* $wgClockSkewFudge has been removed. It was used by User.php to minimize the
+  chances of a user.user_touched database update to the "current" timestamp
+  being before the value already there (e.g. due to clock skew between different
+  servers). This is no longer a problem because the code now ensures the
+  timestamp is always higher than the previous one. The writes are guarded with
+  CAS logic (check and set), which prevents updates that would overlap.
 
 === New features in 1.33 ===
 * (T96041) __EXPECTUNUSEDCATEGORY__ on a category page causes the category

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index a215af5..68d7846 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -2677,14 +2677,6 @@ $wgSidebarCacheExpiry = 86400;
  */
 $wgUseGzip = false;
 
-/**
- * Clock skew or the one-second resolution of time() can occasionally cause cache
- * problems when the user requests two pages within a short period of time. This
- * variable adds a given number of seconds to vulnerable timestamps, thereby giving
- * a grace period.
- */
-$wgClockSkewFudge = 5;
-
 /**
  * Invalidate various caches when LocalSettings.php changes. This is equivalent
  * to setting $wgCacheEpoch to the modification time of LocalSettings.php, as

diff --git a/includes/user/User.php b/includes/user/User.php
index 277731a..ea72bda 100644 (file)
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -2762,17 +2762,16 @@ class User implements IDBAccessObject, UserIdentity {
        /**
         * Generate a current or new-future timestamp to be stored in the
         * user_touched field when we update things.
+        *
         * @return string Timestamp in TS_MW format
         */
        private function newTouchedTimestamp() {
-               global $wgClockSkewFudge;
-
-               $time = wfTimestamp( TS_MW, time() + $wgClockSkewFudge );
-               if ( $this->mTouched && $time <= $this->mTouched ) {
-                       $time = wfTimestamp( TS_MW, wfTimestamp( TS_UNIX, $this->mTouched ) + 1 );
+               $time = time();
+               if ( $this->mTouched ) {
+                       $time = max( $time, wfTimestamp( TS_UNIX, $this->mTouched ) + 1 );
                }
 
-               return $time;
+               return wfTimestamp( TS_MW, $time );
        }
 
        /**