Fwd: Automatic pooling of upstream keepalive connections (patch proposal)

[Greg Vishnepolsky]

Hi Maxim, thanks for the prompt reply!

> While the patch may work, it looks bad from architectural point of
> view.  It essentially makes upstream keepalive module an integral
> part of the upstream module, which isn't a good thing (and also
> will break --without-http_upstream_
> keepalive_module).  The
> upstream module should provide an interface to do things instead.

You're definitely right about this, I haven't thought about that configure
option. How do you suggest to decouple the code? Perhaps add some kind of
callback to the proxy configuration and expose a setter interface?

> Also, it looks like the patch adds lots of code duplication.
> The code to check peer address and lookup a connection in the
> cache is already present in the upstream keepalive module, and it
> should be used instead of adding another structures/code to do the
> same task.

When you're saying "is already present", are you referring to the code in
"ngx_http_upstream_get_keepalive_peer", where "item->sockaddr" is being
compared, as the key to the connection cache?
If so, I'll try to see if it works in the described case. Perhaps a
hostname should be added as another "uniqueness" identifier to this cache
in addition to "sockaddr"? Then a single
"ngx_http_upstream_keepalive_srv_conf_t" can be used for many hosts?
If you believe that this should work, I agree that this is a better way to
do the patch.

Thanks,
Greg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20130911/36f568db/attachment.html>