Loadbalancer failover with curl
It seems failover doesnt work if curl (maybe and other adapters) get any response (with any http code) from server.
Loadbalancer expect HttpException from $adapter->execute method https://github.com/basdenooijer/solarium/blob/master/library/Solarium/Plugin/Loadbalancer/Loadbalancer.php#L501
But it looks like in Curl adapter there is only one place where HttpException can be throwed: https://github.com/basdenooijer/solarium/blob/master/library/Solarium/Core/Client/Adapter/Curl.php#L244
But what if i get response with 502 error and some html?
I can fix it if i call inside Curl adapter:
curl_setopt($handler, CURLOPT_FAILONERROR, true);
but i dont know about negative impact on other parts
The current implementation was designed like this, it only fails when there is no connection to the server at all. I do agree this could be improved, some error responses should also be treated as a failure an cause a failover. However, I think only some errors should cause a failover. It would be bad to execute a malformed query (or another input error) on all servers.
I'm not really sure yet which errors should cause a failover and which not. Maybe it should be configurable in the plugin, with a very safe default set?
Solarium 6.0 ~will deprecate the Curl adapter in favour of~ added a PSR-18 compatible implementation as described in #550. PSR-18 states:
A Client MUST NOT treat a well-formed HTTP request or HTTP response as an error condition. For example, response status codes in the 400 and 500 range MUST NOT cause an exception and MUST be returned to the Calling Library as normal.
A Client MUST throw an instance of
Psr\Http\Client\ClientExceptionInterfaceif and only if it is unable to send the HTTP request at all or if the HTTP response could not be parsed into a PSR-7 response object.
It's up to Loadbalancer to detect a 502 response.
EDIT: Support for PSR-18 was added, but the Curl adapter wasn't deprecated.
@thomascorthals which action needs to be taken now for solarium 6?
I can fix it if i call inside Curl adapter:
curl_setopt($handler, CURLOPT_FAILONERROR, true);but i dont know about negative impact on other parts
It will have a negative impact on REST calls where a 4xx status can be a valid response.
For instance https://solr.apache.org/guide/8_11/managed-resources.html:
You can test to see if a specific word exists by sending a GET request for that word as a child resource of the set, such as:
curl "http://localhost:8983/solr/techproducts/schema/analysis/stopwords/english/foo"This request will return a status code of 200 if the child resource (foo) exists or 404 if it does not exist the managed list.
You don't want a failover for a 404 like this.
Optimistic Concurrency is another case where you don't want a failover on a 409 status.
Solarium 6.3.1 was released with optional failover on HTTP error statuses. You have to set/add status codes for which you want a failover yourself, the list is empty by default.
$loadbalancer->setFailoverEnabled(true);
$loadbalancer->setFailoverMaxRetries(3);
$loadbalancer->addFailoverStatusCode(502);