Revert "objectcache: fix race conditions in RedisBagOStuff::incr()"

This commit reverts most of 7e647d2f0f5b, but keeps unrelated code
clean ups from it, as well as the conflicting changes from d8b952ae47.

From WMF production nutcracker:

> nc_redis.c parsed unsupported command 'WATCH'

The use of WATCH, in addition to failing the commands that use it,
also appears to also have caused a chain reaction making nutcracker
intermittently unavailable to other web requests.

Bug: T228303
Change-Id: Ic37efc2963b147e461837571ae0b65acf3f60cb4

diff --git a/includes/libs/objectcache/RedisBagOStuff.php b/includes/libs/objectcache/RedisBagOStuff.php
index 2d1ed05..21b14f7 100644 (file)
--- a/includes/libs/objectcache/RedisBagOStuff.php
+++ b/includes/libs/objectcache/RedisBagOStuff.php
@@ -368,54 +368,11 @@ class RedisBagOStuff extends BagOStuff {
                }
 
                try {
-                       $conn->watch( $key );
-                       if ( $conn->exists( $key ) ) {
-                               $conn->multi( Redis::MULTI );
-                               $conn->incrBy( $key, $value );
-                               $batchResult = $conn->exec();
-                               if ( $batchResult === false ) {
-                                       $result = false;
-                               } else {
-                                       $result = end( $batchResult );
-                               }
-                       } else {
-                               $result = false;
-                               $conn->unwatch();
-                       }
-               } catch ( RedisException $e ) {
-                       try {
-                               $conn->unwatch(); // sanity
-                       } catch ( RedisException $ex ) {
-                               // already errored
-                       }
-                       $result = false;
-                       $this->handleException( $conn, $e );
-               }
-
-               $this->logRequest( 'incr', $key, $conn->getServer(), $result );
-
-               return $result;
-       }
-
-       public function incrWithInit( $key, $exptime, $value = 1, $init = 1 ) {
-               $conn = $this->getConnection( $key );
-               if ( !$conn ) {
-                       return false;
-               }
-
-               $ttl = $this->convertToRelative( $exptime );
-               $preIncrInit = $init - $value;
-               try {
-                       $conn->multi( Redis::MULTI );
-                       $conn->set( $key, $preIncrInit, $ttl ? [ 'nx', 'ex' => $ttl ] : [ 'nx' ] );
-                       $conn->incrBy( $key, $value );
-                       $batchResult = $conn->exec();
-                       if ( $batchResult === false ) {
-                               $result = false;
-                               $this->debug( "incrWithInit request to {$conn->getServer()} failed" );
-                       } else {
-                               $result = end( $batchResult );
+                       if ( !$conn->exists( $key ) ) {
+                               return false;
                        }
+                       // @FIXME: on races, the key may have a 0 TTL
+                       $result = $conn->incrBy( $key, $value );
                } catch ( RedisException $e ) {
                        $result = false;
                        $this->handleException( $conn, $e );