Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Include type in hashKeyIntoSlots()

Browse source 
Otherwise, all pool types that use slots will collide
due to the slotted keys not using a type prefix.

Bug: T134144
Change-Id: Ib367fedf2cfb7fecc290206e69e0d105276e96e6
This commit is contained in:
Aaron Schulz 2016-05-03 03:53:06 -07:00
parent 773cdb9471
commit 39ed32922e
4 changed files with 28 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
includes/poolcounter/PoolCounter.php
View file

@ -81,7 +81,7 @@ abstract class PoolCounter {
/**
* @param array $conf
* @param string $type
* @param string $type The class of actions to limit concurrency for (task type)
* @param string $key
*/
protected function __construct( $conf, $type, $key ) {
@ -93,8 +93,9 @@ abstract class PoolCounter {
}
if ( $this->slots ) {
$key = $this->hashKeyIntoSlots( $key, $this->slots );
$key = $this->hashKeyIntoSlots( $type, $key, $this->slots );
}
$this->key = $key;
$this->isMightWaitKey = !preg_match( '/^nowait:/', $this->key );
}
@ -102,7 +103,7 @@ abstract class PoolCounter {
/**
* Create a Pool counter. This should only be called from the PoolWorks.
*
* @param string $type
* @param string $type The class of actions to limit concurrency for (task type)
* @param string $key
*
* @return PoolCounter
@ -192,18 +193,19 @@ abstract class PoolCounter {
}
/**
* Given a key (any string) and the number of lots, returns a slot number (an integer from
* the [0..($slots-1)] range). This is used for a global limit on the number of instances of
* a given type that can acquire a lock. The hashing is deterministic so that
* Given a key (any string) and the number of lots, returns a slot key (a prefix with a suffix
* integer from the [0..($slots-1)] range). This is used for a global limit on the number of
* instances of a given type that can acquire a lock. The hashing is deterministic so that
* PoolCounter::$workers is always an upper limit of how many instances with the same key
* can acquire a lock.
*
* @param string $type The class of actions to limit concurrency for (task type)
* @param string $key PoolCounter instance key (any string)
* @param int $slots The number of slots (max allowed value is 65536)
* @return int
* @return string Slot key with the type and slot number
*/
protected function hashKeyIntoSlots( $key, $slots ) {
return hexdec( substr( sha1( $key ), 0, 4 ) ) % $slots;
protected function hashKeyIntoSlots( $type, $key, $slots ) {
return $type . ':' . ( hexdec( substr( sha1( $key ), 0, 4 ) ) % $slots );
}
}

2
includes/poolcounter/PoolCounterWork.php
View file

@ -31,7 +31,7 @@ abstract class PoolCounterWork {
protected $cacheable = false; // does this override getCachedWork() ?
/**
* @param string $type The type of PoolCounter to use
* @param string $type The class of actions to limit concurrency for (task type)
* @param string $key Key that identifies the queue this work is placed on
*/
public function __construct( $type, $key ) {

2
includes/poolcounter/PoolCounterWorkViaCallback.php
View file

@ -44,7 +44,7 @@ class PoolCounterWorkViaCallback extends PoolCounterWork {
* If a 'doCachedWork' callback is provided, then execute() may wait for any prior
* process in the pool to finish and reuse its cached result.
*
* @param string $type
* @param string $type The class of actions to limit concurrency for
* @param string $key
* @param array $callbacks Map of callbacks
* @throws MWException

21
tests/phpunit/includes/poolcounter/PoolCounterTest.php
View file

@ -56,17 +56,26 @@ class PoolCounterTest extends MediaWikiTestCase {
$keysWithTwoSlots = $keysWithFiveSlots = [];
foreach ( range( 1, 100 ) as $i ) {
$keysWithTwoSlots[] = $hashKeyIntoSlots->invoke( $poolCounter, 'key ' . $i, 2 );
$keysWithFiveSlots[] = $hashKeyIntoSlots->invoke( $poolCounter, 'key ' . $i, 5 );
$keysWithTwoSlots[] = $hashKeyIntoSlots->invoke( $poolCounter, 'test', 'key ' . $i, 2 );
$keysWithFiveSlots[] = $hashKeyIntoSlots->invoke( $poolCounter, 'test', 'key ' . $i, 5 );
}
$this->assertArrayEquals( range( 0, 1 ), array_unique( $keysWithTwoSlots ) );
$this->assertArrayEquals( range( 0, 4 ), array_unique( $keysWithFiveSlots ) );
$twoSlotKeys = [];
for ( $i = 0; $i <= 1; $i++ ) {
$twoSlotKeys[] = "test:$i";
}
$fiveSlotKeys = [];
for ( $i = 0; $i <= 4; $i++ ) {
$fiveSlotKeys[] = "test:$i";
}
$this->assertArrayEquals( $twoSlotKeys, array_unique( $keysWithTwoSlots ) );
$this->assertArrayEquals( $fiveSlotKeys, array_unique( $keysWithFiveSlots ) );
// make sure it is deterministic
$this->assertEquals(
$hashKeyIntoSlots->invoke( $poolCounter, 'asdfgh', 1000 ),
$hashKeyIntoSlots->invoke( $poolCounter, 'asdfgh', 1000 )
$hashKeyIntoSlots->invoke( $poolCounter, 'test', 'asdfgh', 1000 ),
$hashKeyIntoSlots->invoke( $poolCounter, 'test', 'asdfgh', 1000 )
);
}
}