You are viewing a plain text version of this content. The canonical link for it is here.
Posted to bugs@httpd.apache.org by bu...@apache.org on 2011/05/28 19:34:20 UTC
DO NOT REPLY [Bug 51285] New: [PATCH] rotatelogs: Add -p option to
call arbitrary post-rotate program
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
Bug #: 51285
Summary: [PATCH] rotatelogs: Add -p option to call arbitrary
post-rotate program
Product: Apache httpd-2
Version: 2.3-HEAD
Platform: PC
OS/Version: Linux
Status: NEW
Severity: normal
Priority: P2
Component: support
AssignedTo: bugs@httpd.apache.org
ReportedBy: sveniu@ifi.uio.no
Classification: Unclassified
rotatelogs: Add -p option to call arbitrary post-rotate program.
Post rotation, call the optional post-rotation program in a separate
thread. apr_proc_create() is called with cmdtype APR_PROGRAM, so it
has to be an executable file. Scripts are supported as long as the
shebang line uses a working interpreter.
The post-rotation program is run with equivalent arguments and
environment variables to let users choose:
argv[1]: Full path to current file
argv[2]: Full path to previous file
Env var ROTATELOGS_PATH_CUR: Full path to current file
Env var ROTATELOGS_PATH_PREV: Full path to previous file
Notes:
The original motivation for this was to have a flexible callback
to be able to maintain a current access.log symlink, and also
do event-based (vs cron-based) compression and archival/upload
to central systems.
Thread support is required and assumed. It's not clear to me how
important it would be to also support non-threaded systems. Adding
a fork-based solution (APR_HAS_THREADS) can be done rather quickly,
if needed.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #6 from Joe Orton <jo...@redhat.com> 2011-06-18 15:20:39 UTC ---
Created attachment 27172
--> https://issues.apache.org/bugzilla/attachment.cgi?id=27172
simplified patch
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #9 from sveniu@ifi.uio.no 2011-06-19 19:43:24 UTC ---
Thanks, this looks good! I've done a bit of testing, and have some notes:
1. The process management works well. I've tried:
a) postrotate program taking longer (sleep) than the specified rotation
interval. This of course builds up children, but works exactly as
expected.
b) Apache/rotatelogs exiting before postrotate program finishes: Child
is adopted by init, and continues to run until it completes.
2. Some might be surprised by or dislike the defunct child processes in the
process list. I don't mind them at all. A fix could be select/poll, which
would also handle exit codes, etc, but I wouldn't say that's a blocker for
committing.
3. The usage() output, the manpage and the xml program documentation should
not refer to the env variables any more.
5. There's some stray trailing whitespace in your latest diff, lines 108 and
122.
Would it be at all possible to also commit this to 2.2? My ultimate goal
is to have this included in Debian, hopefully in a Squeeze update (as opposed
to Wheezy, which could be years in the future).
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #12 from Joe Orton <jo...@redhat.com> 2011-06-27 12:07:56 UTC ---
OK, that makes sense - the docs didn't mention this. I've updated docs + code
to work that way in r1140099. Thanks!
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
sveniu@ifi.uio.no changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |jorton@redhat.com
--- Comment #5 from sveniu@ifi.uio.no 2011-06-02 06:39:31 UTC ---
Hi, Joe.
a) This started out as a separate thread (to avoid many processes in the pid
list) that calls the external program so that the thread would wait for the
program to finish, and catch any problems. I have no issues with changing this,
though. Would it make sense to simply fork the external program without looking
back, and let it handle its own errors? Its stderr is still directed to the
error log, presumably, so it's easy to catch problems anyway.
b) As far as I can see in the source, apr_tokenize_to_argv() does the right
thing with quoting and escaping. It requires users to put escaped spaces or
escaped quotes in the apache config, though. Some examples in the docs would
probably do the trick.
c) APR_PROGRAM was chosen since the env arg to apr_proc_create() is ignored if
the type is APR_PROGRAM_ENV. If we'd rather use apr_env_set() to set the vars,
then use APR_PROGRAM_ENV, that should take care of it.
Let me know what you think, and I'll make the changes.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
sveniu@ifi.uio.no changed:
What |Removed |Added
----------------------------------------------------------------------------
Keywords| |PatchAvailable
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #8 from Joe Orton <jo...@redhat.com> 2011-06-18 15:59:16 UTC ---
Sorry that I never followed up before.
I've attached a patch which has some simplification in:
1) it is necessary to wait() for the forked children at some point, otherwise
we'd collect an ever-increasing horde of zombies (defunct processes). What
I've tried here is:
a) not using detached processes
b) do just "fire and forget"
c) collect any zombies before each new post-rotate
We could still log the exitcode/reason when collecting the zombies I guess, for
diagnostics... not sure.
2) using the tokenize does seem more work than necessary, e.g. if the log
filenames had spaces in we'd need to quote to get tokenize to work. have
switched to set up argv[] directly
3) seems simpler to drop the env vars completely; if the spawned process wants
to use env vars it can do that based on argv
What do you think? It seems to work from some brief testing, particularly with
a post-rotate program which just does "sleep". I'd be happy to commit as-is;
more review/testing would be great.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #11 from sveniu@ifi.uio.no 2011-06-20 11:52:24 UTC ---
It was actually intended to have the program execute on the initial open,
with the goal of not necessarily only doing post-rotate work, but also
do stuff with the newly created file. The main idea was to make a symlink
from the new file to access-current.log or similar (like the -L arg does,
although with symlinks). It would be up to the program to verify that
the second arg (previous file) is present.
Real name: Sven Ulland
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #14 from sveniu@ifi.uio.no 2012-01-02 12:48:59 UTC ---
Any chance of still getting this backported to 2.2?
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
Joe Orton <jo...@redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #27172|0 |1
is obsolete| |
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #4 from Joe Orton <jo...@redhat.com> 2011-06-01 13:21:56 UTC ---
Thanks for sending in the patch! I like the idea; I've had a requests for
similar "do X post-rotate" functionality so there is some kind of general need
here. A few comments:
a) What's the purpose of the double-forking and thread support?
apr_proc_create() will fork/exec internally, so why fork (/spawn thread) then
call apr_proc_create()?
b) Isn't the apr_tokenize_to_argv() usage going to split up any paths with
spaces in? it should be simple enough to set up argv[] correctly without
needing to flatten and reparse the string anyway.
c) probably should set the cmdtype to APR_PROGRAM_ENV to pass through any env
vars
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #7 from Joe Orton <jo...@redhat.com> 2011-06-18 15:52:20 UTC ---
Created attachment 27173
--> https://issues.apache.org/bugzilla/attachment.cgi?id=27173
second attempt at simplified patch
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
Joe Orton <jo...@redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
--- Comment #10 from Joe Orton <jo...@redhat.com> 2011-06-20 10:32:30 UTC ---
Thanks a lot! I made the whitespace and docs changes as suggested, and also
fixed the logic so that the program only gets executed after a rotation, not
after the initial open() of the file or after a failed attempt to rotate; I
presume that was not the intend? (there were cases where the program would
omit the second filename arg)
(BTW, If you let me know your real name I can put that in CHANGES!)
I can propose this for backport to 2.2.x.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #1 from sveniu@ifi.uio.no 2011-05-28 17:38:14 UTC ---
Created attachment 27085
--> https://issues.apache.org/bugzilla/attachment.cgi?id=27085
Patch for rotatelogs.c and its documentation
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
sveniu@ifi.uio.no changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |minfrin@apache.org,
| |poirier@pobox.com,
| |rainer.jung@kippdata.de,
| |sveniu@ifi.uio.no
--- Comment #3 from sveniu@ifi.uio.no 2011-05-30 09:29:18 UTC ---
Adding minfrin, poirier, rjung to CC list, as you have been most
recently active in the rotatelogs.c commit history. Is this patch
something you could consider merging? Possibly also back-porting
to 2.2, so it's easier to get pushed out to distros (I'm thinking
of Debian).
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
sveniu@ifi.uio.no changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #27085|0 |1
is obsolete| |
--- Comment #2 from sveniu@ifi.uio.no 2011-05-30 09:25:03 UTC ---
Created attachment 27090
--> https://issues.apache.org/bugzilla/attachment.cgi?id=27090
Full updated patch for rotatelogs.c and its documentation
Updated patch (full patch attached, applies to svn 1129082), supports
forking and threading, depending on apr build support.
Tested on Debian 6, Linux 2.6.32-amd64, building APR with and without
thread support, then building httpd/support/. Successful results with
both.
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org
DO NOT REPLY [Bug 51285] [PATCH] rotatelogs: Add -p option to call
arbitrary post-rotate program
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=51285
--- Comment #13 from svensven@gmail.com 2011-09-26 12:46:29 UTC ---
Joe, how did the proposal for backporting to 2.2 go?
--
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: bugs-unsubscribe@httpd.apache.org
For additional commands, e-mail: bugs-help@httpd.apache.org