r97687 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97686‎ | r97687 | r97688 >
Date:01:08, 21 September 2011
Author:asher
Status:ok (Comments)
Tags:live 
Comment:
bug 31052 : live hack to support reading from old non-slave external store servers
Modified paths:
  • /branches/wmf/1.17wmf1/includes/ExternalStoreDB.php (modified) (history)

Diff [purge]

Index: branches/wmf/1.17wmf1/includes/ExternalStoreDB.php
——@@ -29,8 +29,18 @@
3030 * @return DatabaseBase object
3131 */
3232 function &getSlave( $cluster ) {
 33+ global $wgDefaultExternalStore;
 34+
3335 $wiki = isset($this->mParams['wiki']) ? $this->mParams['wiki'] : false;
3436 $lb =& $this->getLoadBalancer( $cluster );
 37+
 38+ if ( !in_array( "DB://".$cluster, $wgDefaultExternalStore ) ) {
 39+ wfDebug( "read only external store" );
 40+ $lb->allowLagged(true);
 41+ } else {
 42+ wfDebug( "writable external store" );
 43+ }
 44+
3545 return $lb->getConnection( DB_SLAVE, array(), $wiki );
3646 }
3747

Follow-up revisions

RevisionCommit summaryAuthorDate
r981521.18wmf1: MFT r93912, r95318, r97403, r97650, r97657, r97661, r97687, r97777catrope18:13, 26 September 2011
r98756Merge r97687reedy13:19, 3 October 2011
r99989REL1_18:...reedy22:24, 16 October 2011

Comments

#Comment by MarkAHershberger (talk | contribs)   13:14, 21 September 2011 
+		if ( !in_array( "DB://".$cluster, $wgDefaultExternalStore ) ) {

in_array() is slow, use isset() (see #5 here: http://ilia.ws/archives/12-PHP-Optimization-Tricks.html)

#Comment by 😂 (talk | contribs)   14:11, 21 September 2011

You're thinking array_key_exists(), not in_array(). They're two different things.

#Comment by 😂 (talk | contribs)   14:18, 21 September 2011

I would add that some of those "optimization tricks" are micro-optimization hacks while sacrificing code readability. The one about using isset() vs. strlen() is rather humorous.

#Comment by MarkAHershberger (talk | contribs)   17:42, 21 September 2011

Agreed, Tried to find a better one, but couldn't in 5s.

#Comment by Afeldman (talk | contribs)   18:31, 21 September 2011

I think I actually need to use in_array() in this case but don't worry, $wgDefaultExternalStore is generally only going to contain 0 or 1 elements.

The WMF db.php contains:

$wgDefaultExternalStore = array( 

       'DB://cluster22',

);

isset will return false since the key isn't assigned to a non-null value, but in a test with php 5.3.6, array_key_exists also returns false, unless the key is explicitly assigned to null. Seems like a php bug to me, but only in_array() works as I'd expect in this case.

$ php -r '$test = array("abc123"); if ( array_key_exists("abc123", $test) ) { echo "true\n"; } else { echo "false\n"; }' false

$ php -r '$test = array("abc123"=>NULL); if ( array_key_exists("abc123", $test) ) { echo "true\n"; } else { echo "false\n"; }' true

#Comment by 😂 (talk | contribs)   18:48, 21 September 2011

The two examples you give are exactly what is supposedto happen

Status & tagging log