Proxy address reuse and expiry #367
Conversation
ylavic
force-pushed
the
proxy_address_reuse_and_expiry
branch
2 times, most recently
from
ebf11e3
to
e474096
Compare
…ine/initialize workers.
ylavic
force-pushed
the
proxy_address_reuse_and_expiry
branch
from
e474096
to
0d3792b
Compare
| s, APLOGNO(00960) "%s: an error occurred creating a " | ||
| "new connection to %pI (%s)", proxy_function, | ||
| backend_addr, conn->hostname); | ||
| /* XXX: Will be closed when proxy_conn is closed */ | ||
| socket_cleanup(conn); |
There was a problem hiding this comment.
Why isn't this needed any longer?
There was a problem hiding this comment.
Every caller of ap_proxy_connection_create[_ex]() should release/invalidate the connection on failure already, like with any other ap_proxy_ function taking a proxy_conn_rec which fails? We do this upstream already afaict.
There was a problem hiding this comment.
Good point, but in order to be save shouldn't we do at least a conn->close = 1 here in case the caller only releases the connection but forgets to update the close flag before? But I agree that the current consumers in our code do it properly.
There was a problem hiding this comment.
Added 97ce1e9 to be consistent on how we close the socket on reuse failure.
modules/proxy/proxy_util.c
Outdated
|
|
||
| /* Update worker->cp->addr if it's not set (or reset) */ | ||
| if (!AP_VOLATILIZE_T(apr_sockaddr_t *, worker->cp->addr)) { | ||
| worker->cp->addr = address->addr; |
There was a problem hiding this comment.
How do we ensure it never expires even if it got reset in the meantime?
There was a problem hiding this comment.
The first address/DNS lookup will never expire because it has a refcount of 2, but if some (third party) module plays by resetting worker->cp->addr arbitrarily after the startup it will be set to the current address, which will expire when a new one is needed and the worker or last connection using it releases it.
I don't think it's ever been safe to reset worker->cp->addr, so we should be good?
There was a problem hiding this comment.
I agree that using setting worker->cp->addr to NULL as an indicator was not a good idea and given that this possibility is probably only available in one patch release of 2.4. I am personally happy to drop this. I only feel unsure if we could do it for formal reasons. But probably as long as no one complains we get away with it :-p.
There was a problem hiding this comment.
I don't see where we do this in 2.4, it's trunk changes only for now and addressed by this PR so we should be good ;)
There was a problem hiding this comment.
My bad. Missed that it was never part of 2.4.x.
…dress_reuse_and_expiry
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1911167 13f79535-47bb-0310-9956-ffa450edef68
…dress_reuse_and_expiry
|
Let's go for it. |
|
@ylavic : Do you care to commit to trunk? |
1 similar comment
|
@ylavic : Do you care to commit to trunk? |
Yeah, sorry about the delay (quite busy times..). |
to check if the address changed only, and if not return APR_EEXIST. If it changed update the worker address as usual for every one to know/update. This is used by ap_proxy_connect_backend() to check if it should try with some new address(es) on failure to connect() first with the current one(s), and avoid expiring the worker address globally/spuriously for connect() errors potentially not related to expired address(es).
|
The last commit (176de10) is to avoid expiring the current backend address forcibly and maybe spuriously if ap_proxy_connect_backend() fails on the existing address(es) for something else than some address(es) change. WDYT? |
Define a new proxy_address struct holding the current/latest sockaddr in use
by each proxy worker and conn. Since backend addresses can be updated when
their TTL expires and while connections are being processed, each address is
refcounted and freed only when the last worker (or conn) using it grabs the
new one.
The lifetime of the addresses is handled at a single place by the new
ap_proxy_determine_address() function. It guarantees to bind the current/latest
backend address to the passed in conn (or do nothing if it's up to date already).
The function is called indirectly by ap_proxy_determine_connection() for the
proxy modules that use it, or directly by mod_proxy_ftp and mod_proxy_hcheck.
It also is called eventually by ap_proxy_connect_backend() when connect()ing all
the current addresses fails, to check (PROXY_DETERMINE_ADDRESS_CHECK) if some
new addrs are available.
This commit is also a rework of the lifetime of conn->addr, conn->hostname
and conn->forward, using the conn->uds_pool and conn->fwd_pool for the cases
where the backend is connected through a UDS socket and a remote CONNECT proxy
respectively.
* include/ap_mmn.h:
Minor bump for new function/fields.
* modules/proxy/mod_proxy.h (struct proxy_address,
ap_proxy_determine_addresss()):
Declare ap_proxy_determine_addresss() and opaque struct proxy_address,
new fields to structs proxy_conn_rec/proxy_worker_shared/proxy_worker.
* modules/proxy/mod_proxy.c (set_worker_param):
Parse/set the new worker->address_ttl parameter.
* modules/proxy/proxy_util.c (proxy_util_register_hooks(),
ap_proxy_initialize_worker(),
ap_proxy_connection_reusable(),
ap_proxyerror(), proxyerror_core(),
init_conn_pool(), make_conn_subpool(),
connection_make(), connection_cleanup(),
connection_constructor()):
Initialize *proxy_start_time in proxy_util_register_hooks() as the epoch
from which expiration times are relative (i.e. seconds stored in an uint32_t
for atomic changes).
Make sure worker->s->is_address_reusable and worker->s->disablereuse are
consistant in ap_proxy_initialize_worker(), thus no need to check for both
in ap_proxy_connection_reusable().
New proxyerror_core() helper taking an apr_status_t to log, wrap in
ap_proxyerror().
New make_conn_subpool() to create worker->cp->{pool,dns} with their own
allocator.
New connection_make() helper to factorize code in connection_cleanup() and
connection_constructor().
* modules/proxy/proxy_util.c (proxy_address_inc(), proxy_address_dec(),
proxy_address_cleanup(), proxy_address_set_expired(),
worker_address_get(), worker_address_set(),
worker_address_resolve(), proxy_addrs_equal(),
ap_proxy_determine_address(),
ap_proxy_determine_connection(),
ap_proxy_connect_backend()):
Implement ap_proxy_determine_address() using the above helpers for atomic changes,
and call it from ap_proxy_determine_connection() and ap_proxy_connect_backend().
* modules/proxy/mod_proxy_ftp.c (proxy_ftp_handler):
Use ap_proxy_determine_address() and use the returned backend->addr.
* modules/proxy/mod_proxy_hcheck.c (hc_determine_connection, hc_get_backend,
hc_init_worker, hc_watchdog_callback):
Use ap_proxy_determine_address() in hc_determine_connection() and call the
latter from hc_get_backend(), replace hc_init_worker() by hc_init_baton()
which now calls hc_get_hcworker() and hc_get_backend() to resolve the first
address at init time.
* modules/proxy/mod_proxy_http.c (proxy_http_handler):
Use backend->addr and ->hostname instead of worker->cp->addr and
worker->s->hostname_ex respectively.
* modules/proxy/mod_proxy_ajp.c (ap_proxy_ajp_request):
Use backend->addr and ->hostname instead of worker->cp->addr and
worker->s->hostname_ex respectively.
Closes apache#367
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1912459 13f79535-47bb-0310-9956-ffa450edef68
(cherry picked from commit 3c7f67f)
Define a new proxy_address struct holding the current/latest sockaddr in use
by each proxy worker and conn. Since backend addresses can be updated when
their TTL expires and while connections are being processed, each address is
refcounted and freed only when the last worker (or conn) using it grabs the
new one.
The lifetime of the addresses is handled at a single place by the new
ap_proxy_determine_address() function. It guarantees to bind the current/latest
backend address to the passed in conn (or do nothing if it's up to date already).
The function is called indirectly by ap_proxy_determine_connection() for the
proxy modules that use it, or directly by mod_proxy_ftp and mod_proxy_hcheck.
It also is called eventually by ap_proxy_connect_backend() when connect()ing all
the current addresses fails, to check (PROXY_DETERMINE_ADDRESS_CHECK) if some
new addrs are available.
This commit is also a rework of the lifetime of conn->addr, conn->hostname
and conn->forward, using the conn->uds_pool and conn->fwd_pool for the cases
where the backend is connected through a UDS socket and a remote CONNECT proxy
respectively.
* include/ap_mmn.h:
Minor bump for new function/fields.
* modules/proxy/mod_proxy.h (struct proxy_address,
ap_proxy_determine_addresss()):
Declare ap_proxy_determine_addresss() and opaque struct proxy_address,
new fields to structs proxy_conn_rec/proxy_worker_shared/proxy_worker.
* modules/proxy/mod_proxy.c (set_worker_param):
Parse/set the new worker->address_ttl parameter.
* modules/proxy/proxy_util.c (proxy_util_register_hooks(),
ap_proxy_initialize_worker(),
ap_proxy_connection_reusable(),
ap_proxyerror(), proxyerror_core(),
init_conn_pool(), make_conn_subpool(),
connection_make(), connection_cleanup(),
connection_constructor()):
Initialize *proxy_start_time in proxy_util_register_hooks() as the epoch
from which expiration times are relative (i.e. seconds stored in an uint32_t
for atomic changes).
Make sure worker->s->is_address_reusable and worker->s->disablereuse are
consistant in ap_proxy_initialize_worker(), thus no need to check for both
in ap_proxy_connection_reusable().
New proxyerror_core() helper taking an apr_status_t to log, wrap in
ap_proxyerror().
New make_conn_subpool() to create worker->cp->{pool,dns} with their own
allocator.
New connection_make() helper to factorize code in connection_cleanup() and
connection_constructor().
* modules/proxy/proxy_util.c (proxy_address_inc(), proxy_address_dec(),
proxy_address_cleanup(), proxy_address_set_expired(),
worker_address_get(), worker_address_set(),
worker_address_resolve(), proxy_addrs_equal(),
ap_proxy_determine_address(),
ap_proxy_determine_connection(),
ap_proxy_connect_backend()):
Implement ap_proxy_determine_address() using the above helpers for atomic changes,
and call it from ap_proxy_determine_connection() and ap_proxy_connect_backend().
* modules/proxy/mod_proxy_ftp.c (proxy_ftp_handler):
Use ap_proxy_determine_address() and use the returned backend->addr.
* modules/proxy/mod_proxy_hcheck.c (hc_determine_connection, hc_get_backend,
hc_init_worker, hc_watchdog_callback):
Use ap_proxy_determine_address() in hc_determine_connection() and call the
latter from hc_get_backend(), replace hc_init_worker() by hc_init_baton()
which now calls hc_get_hcworker() and hc_get_backend() to resolve the first
address at init time.
* modules/proxy/mod_proxy_http.c (proxy_http_handler):
Use backend->addr and ->hostname instead of worker->cp->addr and
worker->s->hostname_ex respectively.
* modules/proxy/mod_proxy_ajp.c (ap_proxy_ajp_request):
Use backend->addr and ->hostname instead of worker->cp->addr and
worker->s->hostname_ex respectively.
Closes apache#367
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1912459 13f79535-47bb-0310-9956-ffa450edef68
(cherry picked from commit 3c7f67f)
Define a new proxy_address struct holding the current/latest sockaddr in use
by each proxy worker and conn. Since backend addresses can be updated when
their TTL expires and while connections are being processed, each address is
refcounted and freed only when the last worker (or conn) using it grabs the
new one.
The lifetime of the addresses is handled at a single place by the new
ap_proxy_determine_address() function. It guarantees to bind the current/latest
backend address to the passed in conn (or do nothing if it's up to date already).
The function is called indirectly by ap_proxy_determine_connection() for the
proxy modules that use it, or directly by mod_proxy_ftp and mod_proxy_hcheck.
It also is called eventually by ap_proxy_connect_backend() when connect()ing all
the current addresses fails, to check (PROXY_DETERMINE_ADDRESS_CHECK) if some
new addrs are available.
This commit is also a rework of the lifetime of conn->addr, conn->hostname
and conn->forward, using the conn->uds_pool and conn->fwd_pool for the cases
where the backend is connected through a UDS socket and a remote CONNECT proxy
respectively.
* include/ap_mmn.h:
Minor bump for new function/fields.
* modules/proxy/mod_proxy.h (struct proxy_address,
ap_proxy_determine_addresss()):
Declare ap_proxy_determine_addresss() and opaque struct proxy_address,
new fields to structs proxy_conn_rec/proxy_worker_shared/proxy_worker.
* modules/proxy/mod_proxy.c (set_worker_param):
Parse/set the new worker->address_ttl parameter.
* modules/proxy/proxy_util.c (proxy_util_register_hooks(),
ap_proxy_initialize_worker(),
ap_proxy_connection_reusable(),
ap_proxyerror(), proxyerror_core(),
init_conn_pool(), make_conn_subpool(),
connection_make(), connection_cleanup(),
connection_constructor()):
Initialize *proxy_start_time in proxy_util_register_hooks() as the epoch
from which expiration times are relative (i.e. seconds stored in an uint32_t
for atomic changes).
Make sure worker->s->is_address_reusable and worker->s->disablereuse are
consistant in ap_proxy_initialize_worker(), thus no need to check for both
in ap_proxy_connection_reusable().
New proxyerror_core() helper taking an apr_status_t to log, wrap in
ap_proxyerror().
New make_conn_subpool() to create worker->cp->{pool,dns} with their own
allocator.
New connection_make() helper to factorize code in connection_cleanup() and
connection_constructor().
* modules/proxy/proxy_util.c (proxy_address_inc(), proxy_address_dec(),
proxy_address_cleanup(), proxy_address_set_expired(),
worker_address_get(), worker_address_set(),
worker_address_resolve(), proxy_addrs_equal(),
ap_proxy_determine_address(),
ap_proxy_determine_connection(),
ap_proxy_connect_backend()):
Implement ap_proxy_determine_address() using the above helpers for atomic changes,
and call it from ap_proxy_determine_connection() and ap_proxy_connect_backend().
* modules/proxy/mod_proxy_ftp.c (proxy_ftp_handler):
Use ap_proxy_determine_address() and use the returned backend->addr.
* modules/proxy/mod_proxy_hcheck.c (hc_determine_connection, hc_get_backend,
hc_init_worker, hc_watchdog_callback):
Use ap_proxy_determine_address() in hc_determine_connection() and call the
latter from hc_get_backend(), replace hc_init_worker() by hc_init_baton()
which now calls hc_get_hcworker() and hc_get_backend() to resolve the first
address at init time.
* modules/proxy/mod_proxy_http.c (proxy_http_handler):
Use backend->addr and ->hostname instead of worker->cp->addr and
worker->s->hostname_ex respectively.
* modules/proxy/mod_proxy_ajp.c (ap_proxy_ajp_request):
Use backend->addr and ->hostname instead of worker->cp->addr and
worker->s->hostname_ex respectively.
Closes apache#367
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1912459 13f79535-47bb-0310-9956-ffa450edef68
(cherry picked from commit 3c7f67f)
|
Is there an easy way to figure out what version this patch landed in? I see the docs for it are already advertised, but the docs don't say when it became a valid option. I don't see it mentioned in the changelog. Also, if I'm understanding this feature correctly, it will be a better solution to a problem folks have where load balancers rotate IPs: Does this section of the workers doc need to be updated with this feature? Specifically: ** DNS resolution happens when the socket to the origin domain is created for the first time. When connection reuse is enabled, each backend domain is resolved only once per child process, and cached for all further connections until the child is recycled. This information should to be considered while planning DNS maintenance tasks involving backend domains. Please also check ProxyPass parameters for more details about connection reuse. How does the above interact with this new feature? Will it remove connections when at age address_ttl? Or, will it take a failure on a connection, and then resolve dns if address_ttl is expired before creating a new connection? |
It will be part of the next 2.4.x release.
This would need some update.
It will close connections when the TTL of the DNS entry is expired or if the the address changed in the meantime as another connection triggered a new lookup which resulted in a different address and recreate them. |
No description provided.