Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: domain of trawick@gmail.com designates
 209.85.211.184 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:in-reply-to:references:date:message-id:subject:from:to
         :content-type:content-transfer-encoding;
        b=F9yyVqLE+YvIEf3ES2xKdM/VBH9XzXL5F6+tGQEsD3Cw7y5xWqU9JJQqH2xks4LVfK
         JpFJikj9JF9BiB6x20efrloYCNgnZIJZ1AGxhggPUp9MFsLgw9gaE8Y9JZWqXACfVdK1
         V+TSdoAjrwTZ+HrcnNsPmk2Mw/urOwdChU/OQ=
MIME-Version: 1.0
In-Reply-To: <989D4E55-BCAD-4332-B54F-53D7E4A5D0F7@sharp.fm>
References: <20100216214204.271482388900@eris.apache.org>
	 <cc67648e1002161411n2feb692an1ce0a3573782684e@mail.gmail.com>
	 <989D4E55-BCAD-4332-B54F-53D7E4A5D0F7@sharp.fm>
Date: Wed, 17 Feb 2010 13:54:14 -0500
Message-ID: <cc67648e1002171054t799c74bfg4f55380bf28c4760@mail.gmail.com>
Subject: Re: svn commit: r910705 - in /httpd/httpd/trunk: CHANGES
	docs/manual/programs/htcacheclean.xml support/htcacheclean.c
From: Jeff Trawick <trawick@gmail.com>
To: dev@httpd.apache.org
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 16, 2010 at 6:08 PM, Graham Leggett <minfrin@sharp.fm> wrote:
> On 17 Feb 2010, at 12:11 AM, Jeff Trawick wrote:
>
>>> + =A0*) support/htcacheclean: Teach it how to write a pid file (modelle=
d on
>>> + =A0 =A0 httpd's writing of a pid file) so that it becomes possible to=
 run
>>> + =A0 =A0 more than one instance of htcacheclean on the same machine.
>>> + =A0 =A0 [Graham Leggett]
>>
>> Based on my own initial confusion of the "becomes possible" language
>> (which isn't strictly true), I suspect that this feature description
>> would be more readily understood if this stressed the ease of
>> termination by pid file, since the admin/script no longer has to check
>> ps output (which certainly is a bigger benefit when there are multiple
>> instances, which would require looking at the path parameter).
>> Mileage varies, of course.
>
> s/possible/practical/ I think makes more sense.
>
> "killall <processname>" is a really awful way to kill a process :)
>
>>> + =A0 =A0" =A0-P =A0 Specify PIDFILE as the file to write the pid file =
to."
>>> =A0 =A0 NL
>>
>> s/write the pid file to/write the pid to/
>
> Just fixed this in r910735.
>
>>> + =A0 =A0 =A0 =A0 =A0 =A0apr_file_printf(file, "%ld" APR_EOL_STR, (long=
) mypid);
>>
>> APR_PID_T_FMT and no (long) cast
>
> server/log.c is guilty too, fixed it in r910747.
>
>>> + =A0 =A0 =A0 =A0 =A0 =A0apr_file_close(file);
>>> + =A0 =A0 =A0 =A0}
>>> + =A0 =A0 =A0 =A0else if (!isdaemon) {
>>> + =A0 =A0 =A0 =A0 =A0 =A0apr_file_printf(errfile,
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0"Could not write the pid file =
'%s': %s" APR_EOL_STR,
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pidfile, apr_strerror(status, =
errmsg, sizeof
>>> errmsg));
>>> + =A0 =A0 =A0 =A0}
>>
>> why not just let this be a fatal error? =A0the user specified an invalid
>> path in all likelihood and needs to fix it
>
> We've daemonised at this point, so we can't fail, as the end user won't k=
now
> it's happened. There is also an established pattern in here that when
> daemonised, it stays silent, which probably isn't a good idea, but would
> need to be fixed separately.
>
> One option is to use syslog, will need to see how httpd does it so the
> Windows side works too.

The daemon-report-pidfile-error situation seems to be easily handled
just by reversing the order of the apr_proc_detach block with the
if-pidfile block.  Then the unusable pidfile path error could be
reported to the console in conjunction with -d just like a bogus cache
path or such errors (which are already fatal).

>
>> Also, I suspect that cleaning up the pid file on exit would be
>> anticipated.
>
> Also fixed, svn is off and on and so will commit when it's back up.
>
> server/log.c is also guilty of the same thing, would it make sense to fix
> both at the same time?

Fix both?  Although httpd has left dangling pid files in weird failure
scenarios for a long time, I wouldn't consider it a feature, so yeah.

At the same time?  irrelevant as far as I'm concerned