<div dir="ltr"><div><div><div><div>Hi Maxim, thanks for the prompt reply!<br><br>> While the patch may work, it looks bad from architectural point of<br>> 
view.  It essentially makes upstream keepalive module an integral<br>> 
part of the upstream module, which isn't a good thing (and also<br>> 
will break --without-http_upstream_<div class="gmail_quote">> keepalive_module).  The<br>> 
upstream module should provide an interface to do things instead.<br><br></div><div class="gmail_quote">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?<br>
</div><div class="gmail_quote">
<br>> 
Also, it looks like the patch adds lots of code duplication.<br>> 
The code to check peer address and lookup a connection in the<br>> 
cache is already present in the upstream keepalive module, and it<br>> 
should be used instead of adding another structures/code to do the<br>> 
same task.</div><br></div>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?<br>
</div>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?<br>
</div>If you believe that this should work, I agree that this is a better way to do the patch.<br><br></div>Thanks,<br>Greg<br></div>