You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Yann Ylavic <yl...@gmail.com> on 2018/02/06 07:24:45 UTC
Which server id for mod_proxy_lb? (was: New ServerUID directive)
On Mon, Feb 5, 2018 at 8:09 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>
> On Feb 5, 2018, at 7:38 AM, Stefan Eissing <st...@greenbytes.de>
> wrote:
>
> 2. Does httpd core expose a VirtualHost Identifier in its API and
> what would the semantic properties of such an identifier be?
>
> Yes, it does. It's the server_rec. That contains all the info required
> to determine any and all fashion of "unique" Vhost IDs.
So, for balancers (slotmems) needs, how about:
- All IP:port from server_addr_rec list +
- ServerName +
- ServerAlias(es)
(i.e. hash/MD5 thereof).
?
The rationale is that this is solely what determines the "election" of
a server_rec for each request.
All duplicates, since nothing is enforced on this in httpd (should
it?), would never be elected at runtime (i.e. ignored).
We may want to detect the case though, or do we blindly reuse slotmems
for them (w/o any consistency checks)?
Regards,
Yann.
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Eric Covener <co...@gmail.com>.
On Tue, Feb 6, 2018 at 9:48 AM, Yann Ylavic <yl...@gmail.com> wrote:
> On Tue, Feb 6, 2018 at 3:33 PM, Eric Covener <co...@gmail.com> wrote:
>> On Tue, Feb 6, 2018 at 9:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>> With my proposal, this is lost only if IP:port(s) or
>>> ServerName/Alias(s) change, which is already a win and shouldn't
>>> change the way each balancer is bound to its vhost (i.e. a request on
>>> a vhost wouldn't be accounted/handled by a lb on another vhost).
>>
>> I am thinking no ServerAlias in the hash.
>
> Looks reasonable because taking ServerAlias into account only serves
> corner/weird cases (mis)configurations where two vhosts would have the
> same everything else...
> But if a request asks for the ServerAlias which is only in one of the
> two, all bets would be off on the lb side (the SHMs would be reused
> but not necessarily with the same size).
> This is probably a case which we should error/warn about at
> (re)startup anyway, or maybe include ServerAlias in lb only if it
> happens?
I see what you mean now, I was only thinking "find the wrong
vhost-keyed thing" but not that
offsets/etc could be living in real vhost server config and not line
up correctly. Not so sure now.
--
Eric Covener
covener@gmail.com
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Feb 6, 2018 at 3:33 PM, Eric Covener <co...@gmail.com> wrote:
> On Tue, Feb 6, 2018 at 9:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
>> With my proposal, this is lost only if IP:port(s) or
>> ServerName/Alias(s) change, which is already a win and shouldn't
>> change the way each balancer is bound to its vhost (i.e. a request on
>> a vhost wouldn't be accounted/handled by a lb on another vhost).
>
> I am thinking no ServerAlias in the hash.
Looks reasonable because taking ServerAlias into account only serves
corner/weird cases (mis)configurations where two vhosts would have the
same everything else...
But if a request asks for the ServerAlias which is only in one of the
two, all bets would be off on the lb side (the SHMs would be reused
but not necessarily with the same size).
This is probably a case which we should error/warn about at
(re)startup anyway, or maybe include ServerAlias in lb only if it
happens?
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Eric Covener <co...@gmail.com>.
On Tue, Feb 6, 2018 at 9:23 AM, Yann Ylavic <yl...@gmail.com> wrote:
> With my proposal, this is lost only if IP:port(s) or
> ServerName/Alias(s) change, which is already a win and shouldn't
> change the way each balancer is bound to its vhost (i.e. a request on
> a vhost wouldn't be accounted/handled by a lb on another vhost).
I am thinking no ServerAlias in the hash.
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Yann Ylavic <yl...@gmail.com>.
With my proposal, this is lost only if IP:port(s) or
ServerName/Alias(s) change, which is already a win and shouldn't
change the way each balancer is bound to its vhost (i.e. a request on
a vhost wouldn't be accounted/handled by a lb on another vhost).
On Tue, Feb 6, 2018 at 3:18 PM, Yann Ylavic <yl...@gmail.com> wrote:
> It depends on what you do now, but if it's "reload the server with any
> change before the vhost" than there is an issue.
> You'd lose SHMs, persisted files, all the vhost's balancers states...
>
> On Tue, Feb 6, 2018 at 2:29 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> IMO, unless there are issues/problems with what we do *now*,
>> we shouldn't be changing things...
>>
>>> On Feb 6, 2018, at 2:24 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> On Mon, Feb 5, 2018 at 8:09 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>
>>>> On Feb 5, 2018, at 7:38 AM, Stefan Eissing <st...@greenbytes.de>
>>>> wrote:
>>>>
>>>> 2. Does httpd core expose a VirtualHost Identifier in its API and
>>>> what would the semantic properties of such an identifier be?
>>>>
>>>> Yes, it does. It's the server_rec. That contains all the info required
>>>> to determine any and all fashion of "unique" Vhost IDs.
>>>
>>> So, for balancers (slotmems) needs, how about:
>>> - All IP:port from server_addr_rec list +
>>> - ServerName +
>>> - ServerAlias(es)
>>> (i.e. hash/MD5 thereof).
>>> ?
>>>
>>> The rationale is that this is solely what determines the "election" of
>>> a server_rec for each request.
>>> All duplicates, since nothing is enforced on this in httpd (should
>>> it?), would never be elected at runtime (i.e. ignored).
>>> We may want to detect the case though, or do we blindly reuse slotmems
>>> for them (w/o any consistency checks)?
>>>
>>>
>>> Regards,
>>> Yann.
>>
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Jim Jagielski <ji...@jaguNET.com>.
At this point, I no longer have a horse in this race... From a philosophical
point of view, adding 10kgs of fluff to fix 1kg of error seems over-engineering
to me, but that is just me.
+1 for whatever you think is best... and thx for your
work on this.
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Feb 7, 2018 at 1:47 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> Just for fun, what is the functional difference, if any, between this
> very large patch, that adds lots of code, and this extremely simple
> diff which, from what I can tell, handles the exact defined
> "problem" with the original code???
This oneliner is what I proposed to Mark in the PR to unblock his
situation (IPC-SysV leaks on main process crash)...
But unless one configures a different ServerName (or ServerAdmin??)
for each vhost, multiple servers may have the same ID.
AFAICT, this is neither a requirement nor an enforcement in httpd, and
there may even be multiple vhost configured on different ports with
the same ServerName (legitimately).
If this happens, mod_proxy_lb will possibly reuse slotmems with
incompatible sizes (two vhosts with the same ID but for example a
different number of balancer members), which is controlled by
mod_slotmem but leads to the previous SHM to be overwritten (i.e. all
bets are off).
We *really* need an unique ID for mod_proxy_lb to work correctly.
>
> Just curious if our current policy is to use a sledgehammer now
> to fix what can be handled with a pair of tweezers.
You just ignored anything related the ID (hence the issue) so far, at
least there is a proposal this time.
Please go ahead with a simpler fix if the sledgehammer hurts you.
> On Feb 6, 2018, at 12:59 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> <mod_proxy_lb.diff>
This patch was bogus, I have fixed it but I'd better wait for yours now.
Regards,
Yann.
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Feb 7, 2018 at 1:47 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>
> Just curious if our current policy is to use a sledgehammer now
> to fix what can be handled with a pair of tweezers.
Btw, I also think this is a sledgehammer for the mod_proxy_lb case,
while it wouldn't for s->server_id (IMHO).
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Jim Jagielski <ji...@jaguNET.com>.
Just for fun, what is the functional difference, if any, between this
very large patch, that adds lots of code, and this extremely simple
diff which, from what I can tell, handles the exact defined
"problem" with the original code???
Just curious if our current policy is to use a sledgehammer now
to fix what can be handled with a pair of tweezers.
diff --git a/modules/proxy/mod_proxy_balancer.c b/modules/proxy/mod_proxy_balancer.c
index bdefc8f54..21a65910d 100644
--- a/modules/proxy/mod_proxy_balancer.c
+++ b/modules/proxy/mod_proxy_balancer.c
@@ -784,13 +784,12 @@ static int balancer_post_config(apr_pool_t *pconf, apr_pool_t *plog,
* During create_proxy_config() we created a dummy id. Now that
* we have identifying info, we can create the real id
*/
- id = apr_psprintf(pconf, "%s.%s.%d.%s.%s.%u.%s",
+ id = apr_psprintf(pconf, "%s.%s.%d.%s.%s.%s",
(s->server_scheme ? s->server_scheme : "????"),
(s->server_hostname ? s->server_hostname : "???"),
(int)s->port,
(s->server_admin ? s->server_admin : "??"),
(s->defn_name ? s->defn_name : "?"),
- s->defn_line_number,
(s->error_fname ? s->error_fname : DEFAULT_ERRORLOG));
conf->id = apr_psprintf(pconf, "p%x",
ap_proxy_hashfunc(id, PROXY_HASHFUNC_DEFAULT));
> On Feb 6, 2018, at 12:59 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> <mod_proxy_lb.diff>
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Feb 6, 2018 at 6:17 PM, Stefan Eissing
<st...@greenbytes.de> wrote:
>
>
>> Am 06.02.2018 um 16:14 schrieb Jim Jagielski <ji...@jaguNET.com>:
>>
>> I don't think it does.
>
> I do not understand. I feel that I am missing something here.
>
> You're saying that the scenario does not exist or that it does not trigger the described effect?
FWIW, patch and logs attached, for a test with and without the changes
proposed for the ID.
With the patch SHMs are reused when a blank line is added before the
vhost, without they are not...
This is trunk, after r1822509, while before this commit (i.e.
mod_slotmem_shm of 2.4.x) SHMs are not reused even with no
configuration change (tested too, no logs attached here).
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Stefan Eissing <st...@greenbytes.de>.
> Am 06.02.2018 um 16:14 schrieb Jim Jagielski <ji...@jaguNET.com>:
>
> I don't think it does.
I do not understand. I feel that I am missing something here.
You're saying that the scenario does not exist or that it does not trigger the described effect?
>> On Feb 6, 2018, at 9:18 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> It depends on what you do now, but if it's "reload the server with any
>> change before the vhost" than there is an issue.
>> You'd lose SHMs, persisted files, all the vhost's balancers states...
>>
>> On Tue, Feb 6, 2018 at 2:29 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>> IMO, unless there are issues/problems with what we do *now*,
>>> we shouldn't be changing things...
>>>
>>>> On Feb 6, 2018, at 2:24 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>
>>>> On Mon, Feb 5, 2018 at 8:09 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>
>>>>> On Feb 5, 2018, at 7:38 AM, Stefan Eissing <st...@greenbytes.de>
>>>>> wrote:
>>>>>
>>>>> 2. Does httpd core expose a VirtualHost Identifier in its API and
>>>>> what would the semantic properties of such an identifier be?
>>>>>
>>>>> Yes, it does. It's the server_rec. That contains all the info required
>>>>> to determine any and all fashion of "unique" Vhost IDs.
>>>>
>>>> So, for balancers (slotmems) needs, how about:
>>>> - All IP:port from server_addr_rec list +
>>>> - ServerName +
>>>> - ServerAlias(es)
>>>> (i.e. hash/MD5 thereof).
>>>> ?
>>>>
>>>> The rationale is that this is solely what determines the "election" of
>>>> a server_rec for each request.
>>>> All duplicates, since nothing is enforced on this in httpd (should
>>>> it?), would never be elected at runtime (i.e. ignored).
>>>> We may want to detect the case though, or do we blindly reuse slotmems
>>>> for them (w/o any consistency checks)?
>>>>
>>>>
>>>> Regards,
>>>> Yann.
>>>
>
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Jim Jagielski <ji...@jaguNET.com>.
I don't think it does.
> On Feb 6, 2018, at 9:18 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> It depends on what you do now, but if it's "reload the server with any
> change before the vhost" than there is an issue.
> You'd lose SHMs, persisted files, all the vhost's balancers states...
>
> On Tue, Feb 6, 2018 at 2:29 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> IMO, unless there are issues/problems with what we do *now*,
>> we shouldn't be changing things...
>>
>>> On Feb 6, 2018, at 2:24 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> On Mon, Feb 5, 2018 at 8:09 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>
>>>> On Feb 5, 2018, at 7:38 AM, Stefan Eissing <st...@greenbytes.de>
>>>> wrote:
>>>>
>>>> 2. Does httpd core expose a VirtualHost Identifier in its API and
>>>> what would the semantic properties of such an identifier be?
>>>>
>>>> Yes, it does. It's the server_rec. That contains all the info required
>>>> to determine any and all fashion of "unique" Vhost IDs.
>>>
>>> So, for balancers (slotmems) needs, how about:
>>> - All IP:port from server_addr_rec list +
>>> - ServerName +
>>> - ServerAlias(es)
>>> (i.e. hash/MD5 thereof).
>>> ?
>>>
>>> The rationale is that this is solely what determines the "election" of
>>> a server_rec for each request.
>>> All duplicates, since nothing is enforced on this in httpd (should
>>> it?), would never be elected at runtime (i.e. ignored).
>>> We may want to detect the case though, or do we blindly reuse slotmems
>>> for them (w/o any consistency checks)?
>>>
>>>
>>> Regards,
>>> Yann.
>>
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Yann Ylavic <yl...@gmail.com>.
It depends on what you do now, but if it's "reload the server with any
change before the vhost" than there is an issue.
You'd lose SHMs, persisted files, all the vhost's balancers states...
On Tue, Feb 6, 2018 at 2:29 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> IMO, unless there are issues/problems with what we do *now*,
> we shouldn't be changing things...
>
>> On Feb 6, 2018, at 2:24 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> On Mon, Feb 5, 2018 at 8:09 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>
>>> On Feb 5, 2018, at 7:38 AM, Stefan Eissing <st...@greenbytes.de>
>>> wrote:
>>>
>>> 2. Does httpd core expose a VirtualHost Identifier in its API and
>>> what would the semantic properties of such an identifier be?
>>>
>>> Yes, it does. It's the server_rec. That contains all the info required
>>> to determine any and all fashion of "unique" Vhost IDs.
>>
>> So, for balancers (slotmems) needs, how about:
>> - All IP:port from server_addr_rec list +
>> - ServerName +
>> - ServerAlias(es)
>> (i.e. hash/MD5 thereof).
>> ?
>>
>> The rationale is that this is solely what determines the "election" of
>> a server_rec for each request.
>> All duplicates, since nothing is enforced on this in httpd (should
>> it?), would never be elected at runtime (i.e. ignored).
>> We may want to detect the case though, or do we blindly reuse slotmems
>> for them (w/o any consistency checks)?
>>
>>
>> Regards,
>> Yann.
>
Re: Which server id for mod_proxy_lb? (was: New ServerUID directive)
Posted by Jim Jagielski <ji...@jaguNET.com>.
IMO, unless there are issues/problems with what we do *now*,
we shouldn't be changing things...
> On Feb 6, 2018, at 2:24 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> On Mon, Feb 5, 2018 at 8:09 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>
>> On Feb 5, 2018, at 7:38 AM, Stefan Eissing <st...@greenbytes.de>
>> wrote:
>>
>> 2. Does httpd core expose a VirtualHost Identifier in its API and
>> what would the semantic properties of such an identifier be?
>>
>> Yes, it does. It's the server_rec. That contains all the info required
>> to determine any and all fashion of "unique" Vhost IDs.
>
> So, for balancers (slotmems) needs, how about:
> - All IP:port from server_addr_rec list +
> - ServerName +
> - ServerAlias(es)
> (i.e. hash/MD5 thereof).
> ?
>
> The rationale is that this is solely what determines the "election" of
> a server_rec for each request.
> All duplicates, since nothing is enforced on this in httpd (should
> it?), would never be elected at runtime (i.e. ignored).
> We may want to detect the case though, or do we blindly reuse slotmems
> for them (w/o any consistency checks)?
>
>
> Regards,
> Yann.