Add http client pool #25
Conversation
|
|
||
| return $response; | ||
| }, function ($exception) { | ||
| $this->disable(); |
There was a problem hiding this comment.
not sure if this is a good idea. it could mean that flaky backends would lead to random client objects being disabled over time.
There was a problem hiding this comment.
ah, found the reenable. but still, is your assumption that one client stops working while others continue to work? as this is not multithread or multiprocess code, i don't expect this to be a relevant use case.
or is the idea to use a bunch of clients to do round robin on the client to different instances of the same server application rather than on a load balancer in front of the servers?
There was a problem hiding this comment.
or is the idea to use a bunch of clients to do round robin on the client to different instances of the same server application rather than on a load balancer in front of the servers?
Yes
Having this can remove SPOF from application, as every request / frontend / command act as load balancer, and not only one process
In fact this is something which his already existing in the elasticsearch-php, as my goal is to propose a solution to integrate httplug into their lib this will allow them to completly remove their code about http client and only relying on us.
Also i think this make sense as elasticsearch is not a exclusive solution using cluster mecanism.
|
the implementation looks sane enough to me. i am not sure if one is supposed to do this - i thought with ES the idea is you talk to the cluster and ES does heal itself and make sure one of its nodes is responding. if you are writing, this could even create a huge mess if the ES node runs into a split brain - you would then write to both halves... |
| */ | ||
| class HttpClientPoolItem extends FlexibleHttpClient | ||
| { | ||
| /** @var int Number of request this client is actually sending */ |
It depends on your configuration, in our projects we mainly set all nodes to be master with data to have a real cluster and also always make sure that if a node fail the system is still responding. Having one master fail to this principle, as if the master node is down the entire application is down, it's the same if we put a load balancer in front of elasticsearch as the SPOF will be on the loadbalancer. By directly load balancing into the code we are sure that they will never be a SPOF between our application and the elasticsearch cluster. |
joelwurtz
force-pushed
the
http-client-pool
branch
4 times, most recently
from
e709366 to
af96966
Compare
joelwurtz
force-pushed
the
http-client-pool
branch
from
af96966 to
df40da7
Compare
| }); | ||
|
|
||
| if (0 === count($clientPool)) { | ||
| throw new HttpClientNotFoundException('Cannot choose a http client as there is no one present in the pool'); |
There was a problem hiding this comment.
should we just force-use a disabled client in this case rather than do a hard failure? it would reduce the time we have problems if the backend went away for a short time.
There was a problem hiding this comment.
This can be done by setting 0 on the http client pool item
…tance in client pool item
| } catch (Exception $e) { | ||
| $this->disable(); | ||
| --$this->sendingRequestCount; | ||
| $this->decrementRequestCount(); |
There was a problem hiding this comment.
if we where using php 5.5 only, we could use finally for this. but we are on 5.4 it seems :-(
joelwurtz
force-pushed
the
http-client-pool
branch
from
fb2d8eb to
2914928
Compare
|
can you add an entry to the changelog please? apart from that looks ready to me. |
|
Sorry for not being active in this one. I am happy to merge when there is a change log entry. |
Add a Http Client Pool as discussed in #8