svn commit: r331880 - stable/11/etc

[Kyle Evans]

On Mon, Apr 9, 2018 at 10:52 AM, Rodney W. Grimes
<freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
>> On Mon, Apr 9, 2018 at 10:30 AM, Rodney W. Grimes
>> <freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
>> >> On Mon, Apr 9, 2018 at 10:14 AM, Rodney W. Grimes
>> >> <freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
>> >> >> On Mon, Apr 9, 2018 at 9:46 AM, Rodney W. Grimes
>> >> >> <freebsd at pdx.rh.cn85.dnsmgr.net> wrote:
>> >> >> >> On 04/02/18 17:39, Rodney W. Grimes wrote:
>> >> >> >> >> Author: kevans
>> >> >> >> >> Date: Mon Apr  2 15:28:48 2018
>> >> >> >> >> New Revision: 331880
>> >> >> >> >> URL: https://svnweb.freebsd.org/changeset/base/331880
>> >> >> >> >>
>> >> >> >> >> Log:
>> >> >> >> >>    MFC r328331: Support configuring arbitrary limits(1) for any rc.conf daemon
>> >> >> >> >>
>> >> >> >> >>    Usage is ${name}_limits, and the argument is any flags accepted by
>> >> >> >> >>    limits(1), such as `-n 100' (e.g. only allow 100 open files).
>> >> >> >> >>
>> >> >> >> >> Modified:
>> >> >> >> >>    stable/11/etc/rc.subr
>> >> >> >> >> Directory Properties:
>> >> >> >> >>    stable/11/   (props changed)
>> >> >> >> >>
>> >> >> >> >> Modified: stable/11/etc/rc.subr
>> >> >> >> >> ==============================================================================
>> >> >> >> >> --- stable/11/etc/rc.subr  Mon Apr  2 15:07:41 2018        (r331879)
>> >> >> >> >> +++ stable/11/etc/rc.subr  Mon Apr  2 15:28:48 2018        (r331880)
>> >> >> >> >> @@ -773,6 +773,8 @@ check_startmsgs()
>> >> >> >> >>   #
>> >> >> >> >>   #        ${name}_login_class n   Login class to use, else "daemon".
>> >> >> >> >>   #
>> >> >> >> >> +# ${name}_limits  n       limits(1) to apply to ${command}.
>> >> >> >> >> +#
>> >> >> >> >
>> >> >> >> > Caution, limits(1) is in /usr/bin, this code can fail if used before
>> >> >> >> > /usr is mounted.  (Ie, our rc.initdiskless) is probably broken by
>> >> >> >> > this change if a call is made to limits.
>> >> >> >> >
>> >> >> >> >
>> >> >> >>
>> >> >> >> Sorry for jumping on this so late.  This is also an issue in CURRENT,
>> >> >> >> and has been since at least 2016.
>> >> >> >
>> >> >> > I was aware that it was an issue and why I made a comment about it
>> >> >> > being MFC'ed.  Though I had forgot a bug report existed.
>> >> >>
>> >> >> I'm kind of surprised we haven't had more complaints about this- the
>> >> >> original commit for this stuff landed before stable/11 was even
>> >> >> branched, so it's been broken for all of 11.x's lifetime.
>> >> >
>> >> > History has taught me it takes a long time for this type of
>> >> > breakage to usually surface in a noticable way.  Also I think
>> >> > until you merged this last ${name}_limits thing it actually
>> >> > didn't cause an issue, except for the few like me running
>> >> > diskless systems and or seperate /usr.
>> >>
>> >> I don't see how this merge could possibly have been the cause of any
>> >> claimed issues- like I said before, it didn't add any limits
>> >> invocations, it added an arg to the limits invocation that already
>> >> existed. You can see this pretty clearly from the diff, we didn't even
>> >> change any conditions for limits to be invoked.
>> >
>> > limits_mysql="NO" is defined by the startup script for mysql,
>> > that now causes /etc/rc to try and use that variable in a
>> > different way.
>> >
>> > You added a variable, one that was already in use by some other
>> > /etc/rc* related component.  Collision of differening uses is
>> > causing errors.
>> >
>>
>> Ah, apologies, I misread your previous e-mail and had interpreted it
>> as you claiming again that this broke things for those of you "running
>> diskless systems and or seperate /usr." -- this other breakage, these
>> are things we can fix and aren't really large hurdles to climb over.
>
> Mostly true, other than the hurdle of that 0mp mentions in his
> https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=227205
>         We need to remember that we cannot simply switch to
>         the new mechanism as it is only available in 12-CURRENT
>         and soon in 11-STABLE (and 11.2).
>
> I am not sure how to handle that with the users, it is a operational
> interface change in how limits are done for these ports and probably
> is going to break a lot of peoples systems if they try to update
> from 11.1 to 11.2 because there /etc/rc.conf file is full of old
> stuff that this new stuff is incompatible with.
>
> IMHO, it would be best to post pone this change to 12, as people
> are more willing to suffer painful upgrades when going between
> major versions.
>

Right- so, back out this MFC (and the subsequent FreeBSD_version bump)
and fix the ports to do the right thing for 12.x while that's still
not a technically supported branch?

>>
>> We just need people like 0mp that are actually inclined to address it
>> in ports, and we need to actually communicate changes like this with
>> ports people and assess what's going to break and make a plan to get
>> it fixed.
>
> Problem was/is no one had the foresight to see the ports breakage
> coming and avoid it in some way.  That happens, its engineering,
> lets find a fix and move on.
>
>> IMO this in particular wasn't a major change, and it shouldn't have
>> been too big of a deal (unlike the commit that it built upon). I don't
>> think it should've been broken in head for two months in the various
>> ports that 0mp has identified- even if people don't run these
>> databases on head, we should've assessed the fallout and fixed it
>> somewhere in the two month's time. We're not talking half the ports
>> tree, we're talking < 30 ports. =(
>
> Its usually the tiny, minor, itty bit little nit change that bites
> the hardest :-)
>