r4797 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r4796‎ \| r4797 \| r4798 >
Date:	07:23, 15 August 2004
Author:	timstarling
Status:	old
Tags:
Comment:	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.
Modified paths:	/trunk/phase3/includes/Block.php (modified) (history) /trunk/phase3/includes/BlockCache.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/BlockCache.php
—	—	@@ -7,8 +7,7 @@
8	8	{
9	9	var $mData = false, $mMemcKey;
10	10
11		~~- function BlockCache( $deferLoad = false, $dbName = '' )~~
12		~~- {~~
	11	+ function BlockCache( $deferLoad = false, $dbName = '' ) {
13	12	global $wgDBname;
14	13
15	14	if ( $dbName == '' ) {
—	—	@@ -22,34 +21,38 @@
23	22	}
24	23	}
25	24
26		~~- function load()~~
27		~~- {~~
	25	+ # Load the blocks from the database and save them to memcached
	26	+ function loadFromDB() {
28	27	global $wgUseMemCached, $wgMemc;
	28	+ $this->mData = array();
	29	+ # Selecting FOR UPDATE is a convenient way to serialise the memcached and DB operations,
	30	+ # which is necessary even though we don't update the DB
	31	+ if ( $wgUseMemCached ) {
	32	+ Block::enumBlocks( 'wfBlockCacheInsert', '', EB_FOR_UPDATE );
	33	+ $wgMemc->set( $this->mMemcKey, $this->mData, 0 );
	34	+ } else {
	35	+ Block::enumBlocks( 'wfBlockCacheInsert', '' );
	36	+ }
	37	+ }
	38	+
	39	+ # Load the cache from memcached or, if that's not possible, from the DB
	40	+ function load() {
	41	+ global $wgUseMemCached, $wgMemc;
29	42
30	43	if ( $this->mData === false) {
31		~~- $saveMemc = false;~~
32	44	# Try memcached
33	45	if ( $wgUseMemCached ) {
34	46	$this->mData = $wgMemc->get( $this->mMemcKey );
35		~~- if ( !$this->mData ) {~~
36		~~- $saveMemc = true;~~
37		~~- }~~
38	47	}
39	48
40	49	if ( !is_array( $this->mData ) ) {
41		~~- # Load from DB~~
42		~~- $this->mData = array();~~
43		~~- Block::enumBlocks( 'wfBlockCacheInsert', '' ); # Calls $this->insert()~~
	50	+ $this->loadFromDB();
44	51	}
45		-
46		~~- if ( $saveMemc ) {~~
47		~~- $wgMemc->set( $this->mMemcKey, $this->mData, 0 );~~
48		~~- }~~
49	52	}
50	53	}
51	54
52		~~- function insert( &$block )~~
53		~~- {~~
	55	+ # Add a block to the cache
	56	+ function insert( &$block ) {
54	57	if ( $block->mUser == 0 ) {
55	58	$nb = $block->getNetworkBits();
56	59	$ipint = $block->getIntegerAddr();
—	—	@@ -62,9 +65,9 @@
63	66	$this->mData[$nb][$index] = 1;
64	67	}
65	68	}
66		-
67		~~- function get( $ip )~~
68		~~- {~~
	69	+
	70	+ # Find out if a given IP address is blocked
	71	+ function get( $ip ) {
69	72	$this->load();
70	73	$ipint = ip2long( $ip );
71	74	$blocked = false;
—	—	@@ -90,24 +93,14 @@
91	94	return $block;
92	95	}
93	96
94		~~- function clear()~~
95		~~- {~~
96		~~- global $wgUseMemCached, $wgMemc;~~
97		-
	97	+ # Clear the local cache
	98	+ # There was once a clear() to clear memcached too, but I deleted it
	99	+ function clearLocal() {
98	100	$this->mData = false;
99		~~- if ( $wgUseMemCached ) {~~
100		~~- $wgMemc->delete( $this->mMemcKey );~~
101		~~- }~~
102	101	}
103		-
104		~~- function clearLocal()~~
105		~~- {~~
106		~~- $this->mData = false;~~
107		~~- }~~
108	102	}
109	103
110		~~-function wfBlockCacheInsert( $block, $tag )~~
111		-{
	104	+function wfBlockCacheInsert( $block, $tag ) {
112	105	global $wgBlockCache;
113	106	$wgBlockCache->insert( $block );
114	107	}
Index: trunk/phase3/includes/Block.php
—	—	@@ -10,10 +10,13 @@
11	11
12	12	# Globals used: $wgBlockCache, $wgAutoblockExpiry
13	13
	14	+define ( 'EB_KEEP_EXPIRED', 1 );
	15	+define ( 'EB_FOR_UPDATE', 2 );
	16	+
14	17	class Block
15	18	{
16	19	/* public*/ var $mAddress, $mUser, $mBy, $mReason, $mTimestamp, $mAuto, $mId, $mExpiry;
17		~~- /* private */ var $mNetworkBits, $mIntegerAddr;~~
	20	+ /* private */ var $mNetworkBits, $mIntegerAddr, $mForUpdate;
18	21
19	22	function Block( $address = '', $user = '', $by = 0, $reason = '',
20	23	$timestamp = '' , $auto = 0, $expiry = '' )
—	—	@@ -25,7 +28,8 @@
26	29	$this->mTimestamp = $timestamp;
27	30	$this->mAuto = $auto;
28	31	$this->mExpiry = $expiry;
29		-
	32	+
	33	+ $this->mForUpdate = false;
30	34	$this->initialiseRange();
31	35	}
32	36
—	—	@@ -49,27 +53,33 @@
50	54
51	55	$ret = false;
52	56	$killed = false;
53		~~- $dbr =& wfGetDB( DB_SLAVE );~~
54		~~- $ipblocks = $dbr->tableName( 'ipblocks' );~~
	57	+ if ( $this->forUpdate() ) {
	58	+ $db =& wfGetDB( DB_MASTER );
	59	+ $options = 'FOR UPDATE';
	60	+ } else {
	61	+ $db =& wfGetDB( DB_SLAVE );
	62	+ $options = '';
	63	+ }
	64	+ $ipblocks = $db->tableName( 'ipblocks' );
55	65
56	66	if ( 0 == $user && $address=="" ) {
57		~~- $sql = "SELECT * from $ipblocks";~~
	67	+ $sql = "SELECT * from $ipblocks $options";
58	68	} elseif ($address=="") {
59		~~- $sql = "SELECT * FROM $ipblocks WHERE ipb_user={$user}";~~
	69	+ $sql = "SELECT * FROM $ipblocks WHERE ipb_user={$user} $options";
60	70	} elseif ($user=="") {
61		~~- $sql = "SELECT * FROM $ipblocks WHERE ipb_address='" . $dbr->strencode( $address ) . "'";~~
	71	+ $sql = "SELECT * FROM $ipblocks WHERE ipb_address='" . $db->strencode( $address ) . "' $options";
62	72	} else {
63		~~- $sql = "SELECT * FROM $ipblocks WHERE (ipb_address='" . $dbr->strencode( $address ) .~~
64		~~- "' OR ipb_user={$user})";~~
	73	+ $sql = "SELECT * FROM $ipblocks WHERE (ipb_address='" . $db->strencode( $address ) .
	74	+ "' OR ipb_user={$user}) $options";
65	75	}
66	76
67		~~- $res = $dbr->query( $sql, $fname );~~
68		~~- if ( 0 == $dbr->numRows( $res ) ) {~~
	77	+ $res = $db->query( $sql, $fname );
	78	+ if ( 0 == $db->numRows( $res ) ) {
69	79	# User is not blocked
70	80	$this->clear();
71	81	} else {
72	82	# Get first block
73		~~- $row = $dbr->fetchObject( $res );~~
	83	+ $row = $db->fetchObject( $res );
74	84	$this->initFromRow( $row );
75	85
76	86	if ( $killExpired ) {
—	—	@@ -77,7 +87,7 @@
78	88	do {
79	89	$killed = $this->deleteIfExpired();
80	90	if ( $killed ) {
81		~~- $row = $dbr->fetchObject( $res );~~
	91	+ $row = $db->fetchObject( $res );
82	92	if ( $row ) {
83	93	$this->initFromRow( $row );
84	94	}
—	—	@@ -95,7 +105,7 @@
96	106	$ret = true;
97	107	}
98	108	}
99		~~- $dbr->freeResult( $res );~~
	109	+ $db->freeResult( $res );
100	110	return $ret;
101	111	}
102	112
—	—	@@ -130,18 +140,25 @@
131	141	}
132	142
133	143	# Callback with a Block object for every block
134		~~- /static/ function enumBlocks( $callback, $tag, $killExpired = true )~~
	144	+ /static/ function enumBlocks( $callback, $tag, $flags = 0 )
135	145	{
136		~~- $dbr =& wfGetDB( DB_SLAVE );~~
137		~~- $ipblocks = $dbr->tableName( 'ipblocks' );~~
	146	+ $block = new Block();
	147	+ if ( $flags & EB_FOR_UPDATE ) {
	148	+ $db =& wfGetDB( DB_MASTER );
	149	+ $options = 'FOR UPDATE';
	150	+ $block->forUpdate( true );
	151	+ } else {
	152	+ $db =& wfGetDB( DB_SLAVE );
	153	+ $options = '';
	154	+ }
	155	+ $ipblocks = $db->tableName( 'ipblocks' );
138	156
139		~~- $sql = "SELECT * FROM $ipblocks ORDER BY ipb_timestamp DESC";~~
140		~~- $res = $dbr->query( $sql, 'Block::enumBans' );~~
141		~~- $block = new Block();~~
	157	+ $sql = "SELECT * FROM $ipblocks ORDER BY ipb_timestamp DESC $options";
	158	+ $res = $db->query( $sql, 'Block::enumBans' );
142	159
143		~~- while ( $row = $dbr->fetchObject( $res ) ) {~~
	160	+ while ( $row = $db->fetchObject( $res ) ) {
144	161	$block->initFromRow( $row );
145		~~- if ( $killExpired ) {~~
	162	+ if ( !( $flags & EB_KEEP_EXPIRED ) ) {
146	163	if ( !$block->deleteIfExpired() ) {
147	164	$callback( $block, $tag );
148	165	}
—	—	@@ -232,7 +249,7 @@
233	250	{
234	251	global $wgBlockCache;
235	252	if ( is_object( $wgBlockCache ) ) {
236		~~- $wgBlockCache->clear();~~
	253	+ $wgBlockCache->loadFromDB();
237	254	}
238	255	}
239	256
—	—	@@ -246,6 +263,10 @@
247	264	return $this->mNetworkBits;
248	265	}
249	266
	267	+ function forUpdate( $x = NULL ) {
	268	+ return wfSetVar( $this->mForUpdate, $x );
	269	+ }
	270	+
250	271	/* static */ function getAutoblockExpiry( $timestamp )
251	272	{
252	273	global $wgAutoblockExpiry;

Follow-up revisions

Revision	Commit summary	Author	Date
r84332	Remove some insanely ancient (~r4797) unused globals and variables, and make ...	happy-melon	16:41, 19 March 2011

Status & tagging log

01:56, 13 October 2010 😂 (talk | contribs) changed the status of r4797 [removed: new added: old]