rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

Hi,

Currently rotatelogs (at least on windows) holds the initial
log file by parent process (well it tries to rotate if something
is written from parent which is impossible for access logs).

I plan to use the apr_wait_for_io_or_timeout before calling
the apr_file_read(f_stdin, ...). This would give a chance
to parent to rotate his log as well and release the initial
log file descriptor.

Would that make any problems if executed in the parent?
Since apr_wait_for_io_or_timeout is posix only API, some
#ifdef _WIN32 would be required in rotatelogs


Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

William A. Rowe, Jr. wrote:
> Mladen Turk wrote:
>> Why? Timeout on reading from stdin is a nice and clean way for making the
>> rotation to happen even when there is no log entries fired that would
>> break the block on stdin. Rotation would always happen at midnight,
>> not at some random point of time in the future when something is written
>> to the log.
> 
> Yes!  And this is the point that I thought Jim was taking a look at, it
> has nothing to do with "Windows" :)
>

Right, but it happens to help the Windows not holding
the files by the parent as a side effect ;)
It's a four liner patch, and I'll propose a patch anyhow,
so others might care to give it a peek and test.


Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

[Rainer Jung <ra...@kippdata.de>]

On 09.06.2009 18:51, William A. Rowe, Jr. wrote:
> Mladen Turk wrote:
>> Why? Timeout on reading from stdin is a nice and clean way for making the
>> rotation to happen even when there is no log entries fired that would
>> break the block on stdin. Rotation would always happen at midnight,
>> not at some random point of time in the future when something is written
>> to the log.
> 
> Yes!  And this is the point that I thought Jim was taking a look at, it
> has nothing to do with "Windows" :)

If I got Mladen and you right, there are two topics:

a) rotating even when nothing is being logged in order to close the FDs.
This is independant of platform.

b) on Windows there is an additional rotatelogs process associated with
the parent.

Hope I got that right? Parts of the discussion seemed to jump between a)
and b).

Regards,

Rainer

Re: rotatelogs - Adding timeout for reading from stdin

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Mladen Turk wrote:
> 
> Why? Timeout on reading from stdin is a nice and clean way for making the
> rotation to happen even when there is no log entries fired that would
> break the block on stdin. Rotation would always happen at midnight,
> not at some random point of time in the future when something is written
> to the log.

Yes!  And this is the point that I thought Jim was taking a look at, it
has nothing to do with "Windows" :)

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

William A. Rowe, Jr. wrote:
> Mladen Turk wrote:
>>> Wouldn't it be better if we simply implement the mechanism to share
>>> the same handle between the parent and child?
>> Since the parent does not receive any access log hits how
>> would that prevent it from holding that handle forever?
> 
> The parent's handle to the process would be the child's handle to the
> process.  There would only be one rotatelogs process.
> 

And I suppose this will work only with httpd trunk.

However I'll have to see the actual implementation
before making any conclusions.

>>
>> And I would certainly like to hear from Jim what he proposes
>> for rotatelogs solution for holding the file handles in parent.
> 
> There should be no parent vs child question here.  The right fix is
> one process, just as on unix.  Windows bubblegum and bailing wire is
> the wrong fix.
> 

Why? Timeout on reading from stdin is a nice and clean way for making the
rotation to happen even when there is no log entries fired that would
break the block on stdin. Rotation would always happen at midnight,
not at some random point of time in the future when something is written
to the log.

Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Mladen Turk wrote:
>>
>> Wouldn't it be better if we simply implement the mechanism to share
>> the same handle between the parent and child?
> 
> Since the parent does not receive any access log hits how
> would that prevent it from holding that handle forever?

The parent's handle to the process would be the child's handle to the
process.  There would only be one rotatelogs process.

>> I'd leave rotatelogs alone, please.  Its design is not flawed.  But
>> I think Jim was working on something which would release the file
>> just as soon as its time is up, and that is useful across all of
>> the architectures, not windows specific.
> 
> Using 1 second wait before reading from the stdin would give
> the required rotate resolution. In case of APR_TIMEUP it's
> just a continue in the for (;;) loop giving change to rotate
> logic to handle the files.
> 
> For Windows we can use apr_file_pipe_timeout_set(f_stdin, ...
> Dunno if there's a bug in APR/win32, but you can actually always
> set a timeout on a apr_file_t unlike on unix where this
> is possible only if apr_file_t is a pipe.
> 
> And I would certainly like to hear from Jim what he proposes
> for rotatelogs solution for holding the file handles in parent.

There should be no parent vs child question here.  The right fix is
one process, just as on unix.  Windows bubblegum and bailing wire is
the wrong fix.

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

William A. Rowe, Jr. wrote:
> Mladen Turk wrote:
>> Hi,
>>
>> Currently rotatelogs (at least on windows) holds the initial
>> log file by parent process (well it tries to rotate if something
>> is written from parent which is impossible for access logs).
>>
>> I plan to use the apr_wait_for_io_or_timeout before calling
>> the apr_file_read(f_stdin, ...). This would give a chance
>> to parent to rotate his log as well and release the initial
>> log file descriptor.
>>
>> Would that make any problems if executed in the parent?
>> Since apr_wait_for_io_or_timeout is posix only API, some
>> #ifdef _WIN32 would be required in rotatelogs
> 
> Wouldn't it be better if we simply implement the mechanism to share
> the same handle between the parent and child?

Since the parent does not receive any access log hits how
would that prevent it from holding that handle forever?

> 
> I'd leave rotatelogs alone, please.  Its design is not flawed.  But
> I think Jim was working on something which would release the file
> just as soon as its time is up, and that is useful across all of
> the architectures, not windows specific.
>

Using 1 second wait before reading from the stdin would give
the required rotate resolution. In case of APR_TIMEUP it's
just a continue in the for (;;) loop giving change to rotate
logic to handle the files.

For Windows we can use apr_file_pipe_timeout_set(f_stdin, ...
Dunno if there's a bug in APR/win32, but you can actually always
set a timeout on a apr_file_t unlike on unix where this
is possible only if apr_file_t is a pipe.

And I would certainly like to hear from Jim what he proposes
for rotatelogs solution for holding the file handles in parent.


Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

Plüm, Rüdiger, VF-Group wrote:
> Any particular reason why not using apr_wait_for_io_or_timeout
> at least on unix?
> 

See the answer I gave to Dan.
It uses file internal timeout which we cannot set, so no
it cannot be used.


Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

["Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>]

Any particular reason why not using apr_wait_for_io_or_timeout
at least on unix?

Regards

Rüdiger 

> -----Ursprüngliche Nachricht-----
> Von: Mladen Turk 
> Gesendet: Mittwoch, 10. Juni 2009 14:24
> An: dev@httpd.apache.org
> Betreff: Re: rotatelogs - Adding timeout for reading from stdin
> 
> William A. Rowe, Jr. wrote:
> > Mladen Turk wrote:
> > 
> > I'd leave rotatelogs alone, please.  Its design is not flawed.  But
> > I think Jim was working on something which would release the file
> > just as soon as its time is up, and that is useful across all of
> > the architectures, not windows specific.
> > 
> 
> Attached is the patch that uses 1 second timeout when reading
> from stdin (hard-coded because we have 1 second rotation resolution).
> 
> Unfortunately I had to make WIN32 specific peace of code,
> cause we don't have APR_FILES_AS_SOCKETS on that platform.
> 
> Code was testes both on windows and linux and it works like
> a charm (rotation is done at exact time regardless of log events).
> 
> 
> Regards
> -- 
> ^(TM)
> 

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

William A. Rowe, Jr. wrote:
> Mladen Turk wrote:
>> Code was testes both on windows and linux and it works like
>> a charm (rotation is done at exact time regardless of log events).
> 
> Foolish question, but rather than waking up every second, isn't it more
> rational to compute the expiry of the current file and timeout for that
> period of time?
> 

Might be a nice add-on.
However it would require reorganizing the rotate logic to calculate
early instead late. This would complicate the patch and it
would be hard to follow the difference.
I've choose a straight forward approach.

Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Mladen Turk wrote:
> 
> Code was testes both on windows and linux and it works like
> a charm (rotation is done at exact time regardless of log events).

Foolish question, but rather than waking up every second, isn't it more
rational to compute the expiry of the current file and timeout for that
period of time?

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

William A. Rowe, Jr. wrote:
> Mladen Turk wrote:
> 
> To clarify, it's no longer possible to pipe 'hugefile' through
> rotatelogs with 1MB granularity.  This should at least be noted
> in the docs if the patch is accepted.
> 
> Jim is traveling, it would be good to get his feedback Monday
> on your proposed patch, because I'm near certain it's a puzzle
> he had looked at.
> 

OK. I've created BZ enhancement ticket:
http://issues.apache.org/bugzilla/show_bug.cgi?id=47345

Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Mladen Turk wrote:
> Dan Poirier wrote:
>> Would wait_for_io_or_timeout() be a good candidate for apr?
>>
> 
> There is apr_wait_for_io_or_timeout but it uses the
> apr_file_t->timeout which can be set only for pipes
> and sockets.
> 
> Also the Win32 code will work only for stdhandles because
> they are pipes.

To clarify, it's no longer possible to pipe 'hugefile' through
rotatelogs with 1MB granularity.  This should at least be noted
in the docs if the patch is accepted.

Jim is traveling, it would be good to get his feedback Monday
on your proposed patch, because I'm near certain it's a puzzle
he had looked at.

FYI your patch didn't need platform specifics, if you used the
apr_poll API.

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

Dan Poirier wrote:
> Would wait_for_io_or_timeout() be a good candidate for apr?
> 

There is apr_wait_for_io_or_timeout but it uses the
apr_file_t->timeout which can be set only for pipes
and sockets.

Also the Win32 code will work only for stdhandles because
they are pipes.

In essence the answer is no.
However the full async file API is different thing...

Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

[Dan Poirier <po...@pobox.com>]

Would wait_for_io_or_timeout() be a good candidate for apr?

-- 
Dan Poirier <po...@pobox.com>

Re: rotatelogs - Adding timeout for reading from stdin

[Mladen Turk <mt...@apache.org>]

William A. Rowe, Jr. wrote:
> Mladen Turk wrote:
> 
> I'd leave rotatelogs alone, please.  Its design is not flawed.  But
> I think Jim was working on something which would release the file
> just as soon as its time is up, and that is useful across all of
> the architectures, not windows specific.
> 

Attached is the patch that uses 1 second timeout when reading
from stdin (hard-coded because we have 1 second rotation resolution).

Unfortunately I had to make WIN32 specific peace of code,
cause we don't have APR_FILES_AS_SOCKETS on that platform.

Code was testes both on windows and linux and it works like
a charm (rotation is done at exact time regardless of log events).


Regards
-- 
^(TM)

Re: rotatelogs - Adding timeout for reading from stdin

["William A. Rowe, Jr." <wr...@rowe-clan.net>]

Mladen Turk wrote:
> Hi,
> 
> Currently rotatelogs (at least on windows) holds the initial
> log file by parent process (well it tries to rotate if something
> is written from parent which is impossible for access logs).
> 
> I plan to use the apr_wait_for_io_or_timeout before calling
> the apr_file_read(f_stdin, ...). This would give a chance
> to parent to rotate his log as well and release the initial
> log file descriptor.
> 
> Would that make any problems if executed in the parent?
> Since apr_wait_for_io_or_timeout is posix only API, some
> #ifdef _WIN32 would be required in rotatelogs

Wouldn't it be better if we simply implement the mechanism to share
the same handle between the parent and child?  And reimplement the
locking for append I deployed in apr, which was removed some time
back?  Just as we do with listeners?  It seems long past time to
define the API contract to accomplish this, and there are sufficient
eyeballs to do it, finally.

I'd leave rotatelogs alone, please.  Its design is not flawed.  But
I think Jim was working on something which would release the file
just as soon as its time is up, and that is useful across all of
the architectures, not windows specific.