Skip to content

Commit

Permalink
Fix bug where retries were not being properly set
Browse files Browse the repository at this point in the history
Stung by the infamous isset() vs null issue in PHP.  Need to check if
the `retries` param is set OR if it is null, since the default set
of config options set `retries` to null.

This bug prevented retries from taking place, which meant that under
normal operating scenarios a bad host could take down the client with
an exception, when it normally should have retried on a different host.
  • Loading branch information
polyfractal committed Mar 4, 2014
1 parent 3af3188 commit cb2a6e8
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/Elasticsearch/Transport.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public function __construct($hosts, $params, LoggerInterface $log)
$this->seeds = $hosts;
$this->setConnections($hosts);

if (isset($this->params['retries']) !== true) {
if (isset($this->params['retries']) !== true || $this->params['retries'] === null) {
$this->params['retries'] = count($hosts);
}

Expand Down
27 changes: 26 additions & 1 deletion tests/Elasticsearch/Tests/ClientTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,35 @@ public function testConstructorStringHost()
public function testOneGoodOneBadHostNoException()
{
$params = array('hosts' => array (
'127.0.0.1:9201',
'127.0.0.1:9200',
));
$client = new Elasticsearch\Client($params);

// Perform three requests to make sure the bad host is tried at least once
$client->exists(array("index" => 'test', 'type' => 'test', 'id' => 'test'));
$client->exists(array("index" => 'test', 'type' => 'test', 'id' => 'test'));
$client->exists(array("index" => 'test', 'type' => 'test', 'id' => 'test'));

}


/**
* @expectedException Elasticsearch\Common\Exceptions\Curl\CouldNotConnectToHost
*/
public function testOneGoodOneBadHostNoRetryException()
{
$params = array('hosts' => array (
'127.0.0.1:9201',
'127.0.0.1:9200',
));
$this->client = new Elasticsearch\Client($params);
$params['retries'] = 0;
$client = new Elasticsearch\Client($params);

// Perform three requests to make sure the bad host is tried at least once
$client->exists(array("index" => 'test', 'type' => 'test', 'id' => 'test'));
$client->exists(array("index" => 'test', 'type' => 'test', 'id' => 'test'));
$client->exists(array("index" => 'test', 'type' => 'test', 'id' => 'test'));

}

Expand Down

0 comments on commit cb2a6e8

Please sign in to comment.