You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by William A Rowe Jr <wr...@rowe-clan.net> on 2017/10/12 17:42:30 UTC
Re: svn commit: r1808855 [2/2] - in /httpd/httpd/branches/2.4.x: ./
CHANGES STATUS docs/manual/ docs/manual/mod/mod_proxy.xml modules/http2/
modules/proxy/mod_proxy.c modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
On Sep 19, 2017 05:17, <ji...@apache.org> wrote:
Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/
modules/proxy/mod_proxy.c?rev=1808855&r1=1808854&r2=1808855&view=diff
============================================================
==================
--- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c (original)
+++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c Tue Sep 19
10:17:40 2017
@@ -103,7 +103,8 @@ static const char *set_worker_param(apr_
/* Normalized load factor. Used with BalancerMember,
* it is a number between 1 and 100.
*/
- ival = atoi(val);
+ double fval = atof(val);
+ ival = fval * 100.0;
if (ival < 1 || ival > 100)
return "LoadFactor must be a number between 1..100";
worker->s->lbfactor = ival;
As this patch was obviously never tested by a single reviewer, my
inclination is to revert this non-feature regression, in order to tag a
2.4.29 tomorrow a.m., Windows and OS/X 10.13 ready with the many small
fixes already committed. Then, let this feature be reintroduced when
working, with some testing, along with many other enhancements proposed
right now but all potentially disruptive, as a 2.4.30 to follow soon after
a .29 stability release.
Thoughts?
Re: svn commit: r1808855 [2/2] - in /httpd/httpd/branches/2.4.x: ./
CHANGES STATUS docs/manual/ docs/manual/mod/mod_proxy.xml modules/http2/
modules/proxy/mod_proxy.c modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
On Thu, Oct 12, 2017 at 2:30 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, Oct 12, 2017 at 9:18 PM, Yann Ylavic <yl...@gmail.com> wrote:
> > On Thu, Oct 12, 2017 at 7:42 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> >> On Sep 19, 2017 05:17, <ji...@apache.org> wrote:
> >>
> >>
> >> Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
> >> URL:
> >> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/
> modules/proxy/mod_proxy.c?rev=1808855&r1=1808854&r2=1808855&view=diff
> >> ============================================================
> ==================
> >> --- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c (original)
> >> +++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c Tue Sep 19
> 10:17:40
> >> 2017
> >> @@ -103,7 +103,8 @@ static const char *set_worker_param(apr_
> >> /* Normalized load factor. Used with BalancerMember,
> >> * it is a number between 1 and 100.
> >> */
> >> - ival = atoi(val);
> >> + double fval = atof(val);
> >> + ival = fval * 100.0;
> >> if (ival < 1 || ival > 100)
> >> return "LoadFactor must be a number between 1..100";
> >> worker->s->lbfactor = ival;
> >>
> >>
> >> As this patch was obviously never tested by a single reviewer, my
> >> inclination is to revert this non-feature regression, in order to tag a
> >> 2.4.29 tomorrow a.m., Windows and OS/X 10.13 ready with the many small
> fixes
> >> already committed. Then, let this feature be reintroduced when working,
> with
> >> some testing, along with many other enhancements proposed right now but
> all
> >> potentially disruptive, as a 2.4.30 to follow soon after a .29 stability
> >> release.
> >>
> >> Thoughts?
> >
> > Or apply the same thing as r1805206 for the balancer_manager case
> > (which I tested...).
> > Looks like a serious regression which shouldn't skip a release IMHO,
> > can't 2.4.29 wait a bit (if ever, such small fix could be voted quite
> > quickly)?
>
> Hmm, actually this backport was missing r1805195, I noticed for
> r1805190 but not this one...
>
Sigh... this is why I suggested rolling it away to get one good release out.
Then integrate. Thank you for pointing out the fix (is) on trunk/.
Since folks are provisioning and breaking their 2.4.27 servers as we are
chatting back and forth, it seems like sharing a 2.4.29 with them sooner
rather than later would be helpful. That's why I propose to tag tomorrow,
and kick off next week with a stable release.
Your effort to gather a vote on the missing patch by morning, that's fine
by me too. Simply expect we should offer something without regressions.
If anyone is building on the new OS/X, please pause to upvote Jim's
proposed maintainer-mode fix under the SHOWSTOPPERS section.
I believe he can simply commit that to 2.4.x as an OS-specific CTR
change. But since I can't validate except by observation, I'm not feeling
right about applying the change myself absent 3 votes. It would be good
to help both Windows and OS/X builders this cycle.
Re: svn commit: r1808855 [2/2] - in /httpd/httpd/branches/2.4.x: ./
CHANGES STATUS docs/manual/ docs/manual/mod/mod_proxy.xml modules/http2/
modules/proxy/mod_proxy.c modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
As with my comments to Jim on OS/X compatibility, is a new vote for a
previously approved commit even necessary? Unless this commit was not
profered for the backport wasn't originally offered, it seems you can
simply commit to right the backport submitted.
On Oct 12, 2017 14:30, "Yann Ylavic" <yl...@gmail.com> wrote:
> On Thu, Oct 12, 2017 at 9:18 PM, Yann Ylavic <yl...@gmail.com> wrote:
> > On Thu, Oct 12, 2017 at 7:42 PM, William A Rowe Jr <wr...@rowe-clan.net>
> wrote:
> >> On Sep 19, 2017 05:17, <ji...@apache.org> wrote:
> >>
> >>
> >> Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
> >> URL:
> >> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/
> modules/proxy/mod_proxy.c?rev=1808855&r1=1808854&r2=1808855&view=diff
> >> ============================================================
> ==================
> >> --- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c (original)
> >> +++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c Tue Sep 19
> 10:17:40
> >> 2017
> >> @@ -103,7 +103,8 @@ static const char *set_worker_param(apr_
> >> /* Normalized load factor. Used with BalancerMember,
> >> * it is a number between 1 and 100.
> >> */
> >> - ival = atoi(val);
> >> + double fval = atof(val);
> >> + ival = fval * 100.0;
> >> if (ival < 1 || ival > 100)
> >> return "LoadFactor must be a number between 1..100";
> >> worker->s->lbfactor = ival;
> >>
> >>
> >> As this patch was obviously never tested by a single reviewer, my
> >> inclination is to revert this non-feature regression, in order to tag a
> >> 2.4.29 tomorrow a.m., Windows and OS/X 10.13 ready with the many small
> fixes
> >> already committed. Then, let this feature be reintroduced when working,
> with
> >> some testing, along with many other enhancements proposed right now but
> all
> >> potentially disruptive, as a 2.4.30 to follow soon after a .29 stability
> >> release.
> >>
> >> Thoughts?
> >
> > Or apply the same thing as r1805206 for the balancer_manager case
> > (which I tested...).
> > Looks like a serious regression which shouldn't skip a release IMHO,
> > can't 2.4.29 wait a bit (if ever, such small fix could be voted quite
> > quickly)?
>
> Hmm, actually this backport was missing r1805195, I noticed for
> r1805190 but not this one...
>
Re: svn commit: r1808855 [2/2] - in /httpd/httpd/branches/2.4.x: ./
CHANGES STATUS docs/manual/ docs/manual/mod/mod_proxy.xml modules/http2/
modules/proxy/mod_proxy.c modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Oct 12, 2017 at 9:18 PM, Yann Ylavic <yl...@gmail.com> wrote:
> On Thu, Oct 12, 2017 at 7:42 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
>> On Sep 19, 2017 05:17, <ji...@apache.org> wrote:
>>
>>
>> Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
>> URL:
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c?rev=1808855&r1=1808854&r2=1808855&view=diff
>> ==============================================================================
>> --- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c (original)
>> +++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c Tue Sep 19 10:17:40
>> 2017
>> @@ -103,7 +103,8 @@ static const char *set_worker_param(apr_
>> /* Normalized load factor. Used with BalancerMember,
>> * it is a number between 1 and 100.
>> */
>> - ival = atoi(val);
>> + double fval = atof(val);
>> + ival = fval * 100.0;
>> if (ival < 1 || ival > 100)
>> return "LoadFactor must be a number between 1..100";
>> worker->s->lbfactor = ival;
>>
>>
>> As this patch was obviously never tested by a single reviewer, my
>> inclination is to revert this non-feature regression, in order to tag a
>> 2.4.29 tomorrow a.m., Windows and OS/X 10.13 ready with the many small fixes
>> already committed. Then, let this feature be reintroduced when working, with
>> some testing, along with many other enhancements proposed right now but all
>> potentially disruptive, as a 2.4.30 to follow soon after a .29 stability
>> release.
>>
>> Thoughts?
>
> Or apply the same thing as r1805206 for the balancer_manager case
> (which I tested...).
> Looks like a serious regression which shouldn't skip a release IMHO,
> can't 2.4.29 wait a bit (if ever, such small fix could be voted quite
> quickly)?
Hmm, actually this backport was missing r1805195, I noticed for
r1805190 but not this one...
Re: svn commit: r1808855 [2/2] - in /httpd/httpd/branches/2.4.x: ./
CHANGES STATUS docs/manual/ docs/manual/mod/mod_proxy.xml modules/http2/
modules/proxy/mod_proxy.c modules/proxy/mod_proxy_balancer.c modules/proxy/proxy_util.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Thu, Oct 12, 2017 at 7:42 PM, William A Rowe Jr <wr...@rowe-clan.net> wrote:
> On Sep 19, 2017 05:17, <ji...@apache.org> wrote:
>
>
> Modified: httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c?rev=1808855&r1=1808854&r2=1808855&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c (original)
> +++ httpd/httpd/branches/2.4.x/modules/proxy/mod_proxy.c Tue Sep 19 10:17:40
> 2017
> @@ -103,7 +103,8 @@ static const char *set_worker_param(apr_
> /* Normalized load factor. Used with BalancerMember,
> * it is a number between 1 and 100.
> */
> - ival = atoi(val);
> + double fval = atof(val);
> + ival = fval * 100.0;
> if (ival < 1 || ival > 100)
> return "LoadFactor must be a number between 1..100";
> worker->s->lbfactor = ival;
>
>
> As this patch was obviously never tested by a single reviewer, my
> inclination is to revert this non-feature regression, in order to tag a
> 2.4.29 tomorrow a.m., Windows and OS/X 10.13 ready with the many small fixes
> already committed. Then, let this feature be reintroduced when working, with
> some testing, along with many other enhancements proposed right now but all
> potentially disruptive, as a 2.4.30 to follow soon after a .29 stability
> release.
>
> Thoughts?
Or apply the same thing as r1805206 for the balancer_manager case
(which I tested...).
Looks like a serious regression which shouldn't skip a release IMHO,
can't 2.4.29 wait a bit (if ever, such small fix could be voted quite
quickly)?