r66275 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r66274‎ | r66275 | r66276 >
Date:15:03, 12 May 2010
Author:platonides
Status:ok (Comments)
Tags:
Comment:
Store the Tor list update status in a new memcache key, to avoid
missing the items during the update.
This way, loadExitNodes can be run every 25-28 minutes and
MediaWiki will always have a list to work with.

Probably solves bug 23321
Modified paths:
  • /trunk/extensions/TorBlock/TorBlock.class.php (modified) (history)

Diff [purge]

Index: trunk/extensions/TorBlock/TorBlock.class.php
——@@ -102,9 +102,21 @@
103103 wfDebug( "Loading Tor exit node list from memcached.\n" );
104104 // Lucky.
105105 return self::$mExitNodes = $nodes;
106 - } elseif ($nodes == 'loading') {
107 - // Somebody else is loading it.
108 - return array();
 106+ } else {
 107+ $liststatus = $wgMemc->get( 'mw-tor-list-status' );
 108+ if ( $liststatus == 'loading' ) {
 109+ // Somebody else is loading it.
 110+ wfDebug( "Old Tor list expired and we are still loading the new one.\n" );
 111+ return array();
 112+ } else if ( $liststatus == 'loaded' ) {
 113+ $nodes = $wgMemc->get( 'mw-tor-exit-nodes' );
 114+ if (is_array($nodes)) {
 115+ return self::$mExitNodes = $nodes;
 116+ } else {
 117+ wfDebug( "Tried very hard to get the Tor list since mw-tor-list-status says it is loaded, to no avail.\n" );
 118+ return array();
 119+ }
 120+ }
109121 }
110122
111123 // We have to actually load from the server.
——@@ -128,7 +140,7 @@
129141
130142 // Set loading key, to prevent DoS of server.
131143
132 - $wgMemc->set( 'mw-tor-exit-nodes', 'loading', 300 );
 144+ $wgMemc->set( 'mw-tor-list-status', 'loading', 300 );
133145
134146 $nodes = array();
135147 foreach( $wgTorIPs as $ip ) {
——@@ -137,6 +149,7 @@
138150
139151 // Save to cache.
140152 $wgMemc->set( 'mw-tor-exit-nodes', $nodes, 1800 ); // Store for half an hour.
 153+ $wgMemc->set( 'mw-tor-list-status', 'loaded', 1800 );
141154
142155 wfProfileOut( __METHOD__ );
143156

Comments

#Comment by Werdna (talk | contribs)   13:01, 16 May 2010

This doubles the number of memcached gets done by TorBlock. Not clear on the cost/benefit.

#Comment by Werdna (talk | contribs)   13:04, 16 May 2010

Alternative solution: Don't set the loading key if a list already exists.

#Comment by Platonides (talk | contribs)   19:51, 16 May 2010

On the normal case, you still have one memcache get per hit. Only if the master key expired you get two hits or -if you hit a race condition- even a third.

>Don't set the loading key if a list already exists. Then the key could expire while you are loading it. On a second thought, the difference may not be that big, since it would perform two queries instead of one for sites that for sites that use the external script and doesn't have $wgTorLoadNodes set to false.

Status & tagging log