Hello! On Wed, Sep 11, 2013 at 03:46:51PM +0300, Greg Vishnepolsky wrote:
> 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? I think right aproach would be to expose some kind of "default" upstream which can be used by modules / configured by users. Not sure how exactly this should be done from user point of view though. > > 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. Yes. The sockaddr contains information needed to identify a peer, and it's already used in multi-server upstream blocks for this. -- Maxim Dounin http://nginx.org/en/donation.html _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
