From 46bcd7b1b67d38f8993a548b52183e6ed686afd6 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Sun, 15 Aug 2004 07:23:39 +0000 Subject: [PATCH] Added FOR UPDATE mode to Block.php, to fix memcached concurrency problem in BlockCache.php. This should fix the regular reports from en of blocks not taking effect. --- includes/Block.php | 67 +++++++++++++++++++++++++++-------------- includes/BlockCache.php | 61 +++++++++++++++++-------------------- 2 files changed, 71 insertions(+), 57 deletions(-) diff --git a/includes/Block.php b/includes/Block.php index 427fda22ee..3be78ede0c 100644 --- a/includes/Block.php +++ b/includes/Block.php @@ -10,10 +10,13 @@ # Globals used: $wgBlockCache, $wgAutoblockExpiry +define ( 'EB_KEEP_EXPIRED', 1 ); +define ( 'EB_FOR_UPDATE', 2 ); + class Block { /* public*/ var $mAddress, $mUser, $mBy, $mReason, $mTimestamp, $mAuto, $mId, $mExpiry; - /* private */ var $mNetworkBits, $mIntegerAddr; + /* private */ var $mNetworkBits, $mIntegerAddr, $mForUpdate; function Block( $address = '', $user = '', $by = 0, $reason = '', $timestamp = '' , $auto = 0, $expiry = '' ) @@ -25,7 +28,8 @@ class Block $this->mTimestamp = $timestamp; $this->mAuto = $auto; $this->mExpiry = $expiry; - + + $this->mForUpdate = false; $this->initialiseRange(); } @@ -49,27 +53,33 @@ class Block $ret = false; $killed = false; - $dbr =& wfGetDB( DB_SLAVE ); - $ipblocks = $dbr->tableName( 'ipblocks' ); + if ( $this->forUpdate() ) { + $db =& wfGetDB( DB_MASTER ); + $options = 'FOR UPDATE'; + } else { + $db =& wfGetDB( DB_SLAVE ); + $options = ''; + } + $ipblocks = $db->tableName( 'ipblocks' ); if ( 0 == $user && $address=="" ) { - $sql = "SELECT * from $ipblocks"; + $sql = "SELECT * from $ipblocks $options"; } elseif ($address=="") { - $sql = "SELECT * FROM $ipblocks WHERE ipb_user={$user}"; + $sql = "SELECT * FROM $ipblocks WHERE ipb_user={$user} $options"; } elseif ($user=="") { - $sql = "SELECT * FROM $ipblocks WHERE ipb_address='" . $dbr->strencode( $address ) . "'"; + $sql = "SELECT * FROM $ipblocks WHERE ipb_address='" . $db->strencode( $address ) . "' $options"; } else { - $sql = "SELECT * FROM $ipblocks WHERE (ipb_address='" . $dbr->strencode( $address ) . - "' OR ipb_user={$user})"; + $sql = "SELECT * FROM $ipblocks WHERE (ipb_address='" . $db->strencode( $address ) . + "' OR ipb_user={$user}) $options"; } - $res = $dbr->query( $sql, $fname ); - if ( 0 == $dbr->numRows( $res ) ) { + $res = $db->query( $sql, $fname ); + if ( 0 == $db->numRows( $res ) ) { # User is not blocked $this->clear(); } else { # Get first block - $row = $dbr->fetchObject( $res ); + $row = $db->fetchObject( $res ); $this->initFromRow( $row ); if ( $killExpired ) { @@ -77,7 +87,7 @@ class Block do { $killed = $this->deleteIfExpired(); if ( $killed ) { - $row = $dbr->fetchObject( $res ); + $row = $db->fetchObject( $res ); if ( $row ) { $this->initFromRow( $row ); } @@ -95,7 +105,7 @@ class Block $ret = true; } } - $dbr->freeResult( $res ); + $db->freeResult( $res ); return $ret; } @@ -130,18 +140,25 @@ class Block } # Callback with a Block object for every block - /*static*/ function enumBlocks( $callback, $tag, $killExpired = true ) + /*static*/ function enumBlocks( $callback, $tag, $flags = 0 ) { - $dbr =& wfGetDB( DB_SLAVE ); - $ipblocks = $dbr->tableName( 'ipblocks' ); - - $sql = "SELECT * FROM $ipblocks ORDER BY ipb_timestamp DESC"; - $res = $dbr->query( $sql, 'Block::enumBans' ); $block = new Block(); + if ( $flags & EB_FOR_UPDATE ) { + $db =& wfGetDB( DB_MASTER ); + $options = 'FOR UPDATE'; + $block->forUpdate( true ); + } else { + $db =& wfGetDB( DB_SLAVE ); + $options = ''; + } + $ipblocks = $db->tableName( 'ipblocks' ); + + $sql = "SELECT * FROM $ipblocks ORDER BY ipb_timestamp DESC $options"; + $res = $db->query( $sql, 'Block::enumBans' ); - while ( $row = $dbr->fetchObject( $res ) ) { + while ( $row = $db->fetchObject( $res ) ) { $block->initFromRow( $row ); - if ( $killExpired ) { + if ( !( $flags & EB_KEEP_EXPIRED ) ) { if ( !$block->deleteIfExpired() ) { $callback( $block, $tag ); } @@ -232,7 +249,7 @@ class Block { global $wgBlockCache; if ( is_object( $wgBlockCache ) ) { - $wgBlockCache->clear(); + $wgBlockCache->loadFromDB(); } } @@ -246,6 +263,10 @@ class Block return $this->mNetworkBits; } + function forUpdate( $x = NULL ) { + return wfSetVar( $this->mForUpdate, $x ); + } + /* static */ function getAutoblockExpiry( $timestamp ) { global $wgAutoblockExpiry; diff --git a/includes/BlockCache.php b/includes/BlockCache.php index 8b49a0142c..9f099b2846 100644 --- a/includes/BlockCache.php +++ b/includes/BlockCache.php @@ -7,8 +7,7 @@ class BlockCache { var $mData = false, $mMemcKey; - function BlockCache( $deferLoad = false, $dbName = '' ) - { + function BlockCache( $deferLoad = false, $dbName = '' ) { global $wgDBname; if ( $dbName == '' ) { @@ -22,34 +21,38 @@ class BlockCache } } - function load() - { + # Load the blocks from the database and save them to memcached + function loadFromDB() { + global $wgUseMemCached, $wgMemc; + $this->mData = array(); + # Selecting FOR UPDATE is a convenient way to serialise the memcached and DB operations, + # which is necessary even though we don't update the DB + if ( $wgUseMemCached ) { + Block::enumBlocks( 'wfBlockCacheInsert', '', EB_FOR_UPDATE ); + $wgMemc->set( $this->mMemcKey, $this->mData, 0 ); + } else { + Block::enumBlocks( 'wfBlockCacheInsert', '' ); + } + } + + # Load the cache from memcached or, if that's not possible, from the DB + function load() { global $wgUseMemCached, $wgMemc; if ( $this->mData === false) { - $saveMemc = false; # Try memcached if ( $wgUseMemCached ) { $this->mData = $wgMemc->get( $this->mMemcKey ); - if ( !$this->mData ) { - $saveMemc = true; - } } if ( !is_array( $this->mData ) ) { - # Load from DB - $this->mData = array(); - Block::enumBlocks( 'wfBlockCacheInsert', '' ); # Calls $this->insert() - } - - if ( $saveMemc ) { - $wgMemc->set( $this->mMemcKey, $this->mData, 0 ); + $this->loadFromDB(); } } } - function insert( &$block ) - { + # Add a block to the cache + function insert( &$block ) { if ( $block->mUser == 0 ) { $nb = $block->getNetworkBits(); $ipint = $block->getIntegerAddr(); @@ -62,9 +65,9 @@ class BlockCache $this->mData[$nb][$index] = 1; } } - - function get( $ip ) - { + + # Find out if a given IP address is blocked + function get( $ip ) { $this->load(); $ipint = ip2long( $ip ); $blocked = false; @@ -90,24 +93,14 @@ class BlockCache return $block; } - function clear() - { - global $wgUseMemCached, $wgMemc; - - $this->mData = false; - if ( $wgUseMemCached ) { - $wgMemc->delete( $this->mMemcKey ); - } - } - - function clearLocal() - { + # Clear the local cache + # There was once a clear() to clear memcached too, but I deleted it + function clearLocal() { $this->mData = false; } } -function wfBlockCacheInsert( $block, $tag ) -{ +function wfBlockCacheInsert( $block, $tag ) { global $wgBlockCache; $wgBlockCache->insert( $block ); } -- 2.20.1