From: <msc...@ao...> - 2003-08-04 05:00:08
Attachments:
signal.txt
027Watch3.t
|
Hey, just had an idea: How about if we add functionality to init_and_watch in Log::Log4perl to not only watch config files but to also be able to listen to (Unix) signals instead? Yeah, yeah, I know signals are unreliable and can cause all kinds of crazy stuff -- but how about if we have the signal handler just set a global variable, which is checked by the watcher code instead of calling time() all the time? Please take a look at the patch below, and let me know what you think ... the addtl. syntax is: init_and_watch($file, 'HUP'); # will listen to SIGHUP I've also attached a test file. -- Mike ############################ # Mike Schilli # # log...@pe... # # http://perlmeister.com # # log4perl.sourceforge.net # ############################ |
From: Kevin G. <ke...@go...> - 2003-08-05 18:07:02
|
That's a pretty neat idea, go for it, dude! msc...@ao... wrote: > Hey, > > just had an idea: How about if we add functionality to init_and_watch in Log::Log4perl to not only watch config files but to also be able to listen to (Unix) signals instead? Yeah, yeah, I know signals are unreliable and can cause all kinds of crazy stuff -- but how about if we have the signal handler just set a global variable, which is checked by the watcher code instead of calling time() all the time? > Please take a look at the patch below, and let me know what you think ... the addtl. syntax is: > > init_and_watch($file, 'HUP'); # will listen to SIGHUP > > I've also attached a test file. > > -- Mike > > ############################ > # Mike Schilli # > # log...@pe... # > # http://perlmeister.com # > # log4perl.sourceforge.net # > ############################ > > > ------------------------------------------------------------------------ > > Index: lib/Log/Log4perl/Config.pm > =================================================================== > RCS file: /cvsroot/log4perl/Log-Log4perl/lib/Log/Log4perl/Config.pm,v > retrieving revision 1.52 > diff -a -u -r1.52 Config.pm > --- lib/Log/Log4perl/Config.pm 20 Jul 2003 20:24:18 -0000 1.52 > +++ lib/Log/Log4perl/Config.pm 4 Aug 2003 04:47:28 -0000 > @@ -45,10 +45,16 @@ > sub init_and_watch { > ########################################### > my ($class, $config, $delay) = @_; > + # delay can be a signal name - in this case we're gonna > + # set up a signal handler. > > if(defined $WATCHER) { > $config = $WATCHER->file(); > - $delay = $WATCHER->check_interval(); > + if(defined $Log::Log4perl::Config::Watch::SIGNAL_CAUGHT) { > + $delay = $WATCHER->signal(); > + } else { > + $delay = $WATCHER->check_interval(); > + } > } > > print "init_and_watch ($config-$delay). Resetting.\n" if DEBUG; > @@ -57,8 +63,6 @@ > > defined ($delay) or $delay = $DEFAULT_WATCH_DELAY; > > - $delay =~ /\D/ && die "illegal non-numerical value for delay: $delay"; > - > if (ref $config) { > die "Log4perl can only watch a file, not a string of " . > "configuration information"; > @@ -66,10 +70,17 @@ > die "Log4perl can only watch a file, not a url like $config"; > } > > - $WATCHER = Log::Log4perl::Config::Watch->new( > - file => $config, > - check_interval => $delay, > - ); > + if($delay =~ /\D/) { > + $WATCHER = Log::Log4perl::Config::Watch->new( > + file => $config, > + signal => $delay, > + ); > + } else { > + $WATCHER = Log::Log4perl::Config::Watch->new( > + file => $config, > + check_interval => $delay, > + ); > + } > > _init($class, $config); > } > Index: lib/Log/Log4perl/Logger.pm > =================================================================== > RCS file: /cvsroot/log4perl/Log-Log4perl/lib/Log/Log4perl/Logger.pm,v > retrieving revision 1.55 > diff -a -u -r1.55 Logger.pm > --- lib/Log/Log4perl/Logger.pm 4 Aug 2003 02:37:35 -0000 1.55 > +++ lib/Log/Log4perl/Logger.pm 4 Aug 2003 04:47:30 -0000 > @@ -292,13 +292,14 @@ > > if (defined $Log::Log4perl::Config::WATCHER) { > > - $watch_code = <<'EOL'; > - my($logger, $subname) = @_; > - if(time() > $Log::Log4perl::Config::Watch::NEXT_CHECK_TIME and > - $Log::Log4perl::Config::WATCHER->change_detected()) { > + my $cond = generate_watch_conditional(); > + > + $watch_code = <<EOL; > + my(\$logger, \$subname) = \@_; > + if($cond) { > Log::Log4perl->init_and_watch(); > # Forward call to new configuration > - return $logger->$subname(); > + return \$logger->\$subname(); > } > EOL > } > @@ -317,20 +318,36 @@ > ################################################## > print "generate_watch_code:\n" if DEBUG; > > - return <<'EOL'; > + my $cond = generate_watch_conditional(); > + > + return <<EOL; > print "exe_watch_code:\n" if DEBUG; > > # more closures here > - if(time() > $Log::Log4perl::Config::Watch::NEXT_CHECK_TIME and > - $Log::Log4perl::Config::WATCHER->change_detected()) { > + if($cond) { > Log::Log4perl->init_and_watch(); > > - my $methodname = lc($level); > - $logger->$methodname(@_); # send the message > - # to the new configuration > + my \$methodname = lc(\$level); > + \$logger->\$methodname(\@_); # send the message > + # to the new configuration > return; #and return, we're done with this incarnation > } > EOL > +} > + > +################################################## > +sub generate_watch_conditional { > +################################################## > + > + if(defined $Log::Log4perl::Config::Watch::SIGNAL_CAUGHT) { > + # In this mode, we just check for the variable indicating > + # that the signal has been caught > + return q{$Log::Log4perl::Config::Watch::SIGNAL_CAUGHT}; > + } > + > + # In this mode, we check if the config file has been modified > + return q{time() > $Log::Log4perl::Config::Watch::NEXT_CHECK_TIME > + and $Log::Log4perl::Config::WATCHER->change_detected()}; > } > > ################################################## > Index: lib/Log/Log4perl/Config/Watch.pm > =================================================================== > RCS file: /cvsroot/log4perl/Log-Log4perl/lib/Log/Log4perl/Config/Watch.pm,v > retrieving revision 1.3 > diff -a -u -r1.3 Watch.pm > --- lib/Log/Log4perl/Config/Watch.pm 20 Jul 2003 20:24:01 -0000 1.3 > +++ lib/Log/Log4perl/Config/Watch.pm 4 Aug 2003 04:47:30 -0000 > @@ -8,6 +8,7 @@ > use constant DEBUG => 0; > > our $NEXT_CHECK_TIME; > +our $SIGNAL_CAUGHT; > > ########################################### > sub new { > @@ -16,6 +17,7 @@ > > my $self = { file => "", > check_interval => 30, > + signal => undef, > %options, > _last_checked_at => 0, > _last_timestamp => 0, > @@ -23,8 +25,15 @@ > > bless $self, $class; > > - # Just called to initialize > - $self->change_detected(); > + if($self->{signal}) { > + # We're in signal mode, set up the handler > + $SIG{$self->{signal}} = sub { $SIGNAL_CAUGHT = 1; }; > + # Reset the marker. The handler is going to modify it. > + $SIGNAL_CAUGHT = 0; > + } else { > + # Just called to initialize > + $self->change_detected(); > + } > > return $self; > } > @@ -35,6 +44,14 @@ > my($self) = @_; > > return $self->{file}; > +} > + > +########################################### > +sub signal { > +########################################### > + my($self) = @_; > + > + return $self->{signal}; > } > > ########################################### -- Happy Trails . . . Kevin M. Goess (and Anne and Frank) 904 Carmel Ave. Albany, CA 94706 (510) 525-5217 |
From: Jim C. <jc...@di...> - 2003-08-05 20:46:02
|
msc...@ao... wrote: >Hey, > >just had an idea: How about if we add functionality to init_and_watch in Log::Log4perl to not only watch config files but to also be able to listen to (Unix) signals instead? Yeah, yeah, I know signals are unreliable and can cause all kinds of crazy stuff -- but how about if we have the signal handler just set a global variable, which is checked by the watcher code instead of calling time() all the time? >Please take a look at the patch below, and let me know what you think ... the addtl. syntax is: > > init_and_watch($file, 'HUP'); # will listen to SIGHUP > >I've also attached a test file. > >-- Mike > > > Ive only glanced at patch, so this might be done already (or might be stupid - I havent thought it thru) does patch allow user to use ALRM to duplicate existing behavior. consistency would dictate that this is a good idea. in lib/Log/Log4perl/Config/Watch.pm new() sets the signal handler itself, its probly prudent to document this IN LARGE LETTERS. also, since the handler is set there, theres no opportunity to restart the alarm. any point to saving prior signal handler, and maybe even calling it after setting SIGNAL_CAUGHT ? also FWIW - 5.8.0 has safe signals by default, earlier doesnt. there may be some suprises in there. |
From: Mike S. <msc...@ao...> - 2003-08-06 04:06:14
|
Jim Cromie wrote: > does patch allow user to use ALRM to duplicate existing behavior. > consistency would dictate that this is a good idea. Interesting idea, I've just tested it with setting the signal to 'ALRM' and triggering it via alarm(1). Works. > new() sets the signal handler itself, its probly prudent to document > this IN LARGE LETTERS. I've added some documentation to both Log4perl.pm (about performance) and Watch.pm (about the new signal handler), thanks for the reminder :). > also, since the handler is set there, theres no opportunity to restart > the alarm. That should be up to the application. By calling alarm(0), the alarm will be reset. Should be independant of Watch.pm. > any point to saving prior signal handler, and maybe even calling it > after setting SIGNAL_CAUGHT ? Hmm, you mean what happens if there's already a signal handler defined? It'll be brutally overwritten, hopefully the new doc in Watch.pm illustrates that. I don't want to get into the business of calling other people's signal handlers :). > also FWIW - 5.8.0 has safe signals by default, earlier doesnt. there > may be some suprises in there. The design allows for all kinds of hickups -- all the handler does is set a global variable. It either does it or the signal gets lost. If it does, Log4perl gets re-initialized, if it gets lost, it doesn't. Can you think of anything more severe? Anyways, thanks a bunch for your comments! Oh, by the way, I found something in the watcher code: By delaying getting the $logger and $level parameters from the stack until they're actually needed, the 'SIG' watch enabled code now holds up to 90% of the full init() speed for suppressed logger calls! This also helped the traditional init_and_watch, which is now at about 60%. -- -- Mike Mike Schilli msc...@ao... AOL Unified Preferences Engineering |
From: Jim C. <jc...@di...> - 2003-08-06 20:33:32
|
Mike Schilli wrote: >Jim Cromie wrote: > > > does patch allow user to use ALRM to duplicate existing behavior. > > consistency would dictate that this is a good idea. > >Interesting idea, I've just tested it with setting the signal to 'ALRM' >and triggering it via alarm(1). Works. > cool - that was my hope > > also, since the handler is set there, theres no opportunity to restart > > the alarm. > >That should be up to the application. By calling alarm(0), the alarm >will be reset. Should be independant of Watch.pm. > > Ok - but it precludes use of alarm to (fully) emulate $delay = 60 behavior of existing timeout, as it cant reset itself. Of course, theres no 'real' reason to emulate - folks can do it the old way. > > any point to saving prior signal handler, and maybe even calling it > > after setting SIGNAL_CAUGHT ? > >Hmm, you mean what happens if there's already a signal handler defined? >It'll be brutally overwritten, hopefully the new doc in Watch.pm >illustrates that. I don't want to get into the business of calling other >people's signal handlers :). > > # you should add a warn "IM DESTROYING YOUR EXISTING SIGNAL HANDLER ON $sig" if $SIG{$sig} # cuz people are often unaware of the signals used deep in a library or a framework. # and perhaps/maybe give them some (obscure, undocumented) way to shoot themselves in the foot. $Log::Log4perl::Watcher::stack_handlers = 1; > > also FWIW - 5.8.0 has safe signals by default, earlier doesnt. there > > may be some suprises in there. > >The design allows for all kinds of hickups -- all the handler does is >set a global variable. It either does it or the signal gets lost. If it >does, Log4perl gets re-initialized, if it gets lost, it doesn't. Can you >think of anything more severe? > > heh - true true. your approach borders on paranoid :-) L<perldoc perlipc> >Anyways, thanks a bunch for your comments! > :-) even half-baked comments are better than silence. this list is low-traffic enough that a little noise is ok. > >Oh, by the way, I found something in the watcher code: By delaying >getting the $logger and $level parameters from the stack until they're >actually needed, the 'SIG' watch enabled code now holds up to 90% of the >full init() speed for suppressed logger calls! This also helped the >traditional init_and_watch, which is now at about 60%. > > > cool ! esp for traditional - where 99% of re-inits have no changes. btw - when re-init happens, does it just throw away the old init and rebuild a brand-new structure ? the reason I ask is that my AutoCategorize sub-class builds a lot of anonymous subs to delegate up to Log4perl, and it would be nice not to throw out the whole lot. If both the b4 and after configs are available at some (transient) point, and a hook is available so I can inspect them, I can find the diffs and delete only those subs whose config has been superceded. |
From: Wiggins d'A. <wi...@da...> - 2003-08-06 02:33:21
|
msc...@ao... wrote: > Hey, > > just had an idea: How about if we add functionality to init_and_watch in Log::Log4perl to not only watch config files but to also be able to listen to (Unix) signals instead? Yeah, yeah, I know signals are unreliable and can cause all kinds of crazy stuff -- but how about if we have the signal handler just set a global variable, which is checked by the watcher code instead of calling time() all the time? > Please take a look at the patch below, and let me know what you think ... the addtl. syntax is: > > init_and_watch($file, 'HUP'); # will listen to SIGHUP > > I've also attached a test file. > I like the idea but I think this goes back to the notion of init_and_watch calling die on failure. So two quick questions, 1) Is it possible to store the current config, load the new one, and rollback to the previous one if something goes wrong? 2) Is it possible to compartmentalize the config reading+updating, etc. code to the extent of rather than passing init_and_watch a signal to catch, that a particular class method could be called instead, then the user could install their own signal handler easy enough. (Or maybe both cause if you can do #2, then it should be trivial to have init_and_watch accept a signal and it would just set the call to the class method to that signal for the user). Currently I am using Log4perl extensively in a POE framework app (by the way it works beautifully, well with the exception of FileRotate but I think an upgrade will improve that situation) so I already have a framework for handling events and signals, but it would be nice if I could control the reloading of the config file on my own, rather than having init_and_watch do it for me either through a time based or a signal based mechanism. Because I have a 24/7 running app as well, I don't want to have the worry of sending a signal to the app or even after a certain amount of time, having it caught and having it 'die' on me when that isn't the type of signal I sent. This essentially gets somewhat back to being able to pass a callback to the init_and_watch so that when it fires, certain actions can be taken. I realize this is open source, and that I should put my keyboard where my mouth is but I am still a little intimidated by taking this on my own especially without being familar with the Log4perl internals (yet!), but these are just some ideas I have mulled over while writing our app. Given enough time I will probably get around to taking a stab at it myself, but if Mike or Kevin, et al. have a way to work it in while all of this is fresh I think it would be a nice addition. I also realize there is the log4j standards, etc. Just my $.02 for what their worth... http://danconia.org |
From: Mike S. <msc...@ao...> - 2003-08-06 22:22:10
|
Wiggins d'Anconia wrote: > I like the idea but I think this goes back to the notion of > init_and_watch calling die on failure. So two quick questions, You've got a lot of good points. There's a bunch open questions which we need to find answers for before we proceed (partially just summarizing your points): [1] Are we ok with the way Log::Log4perl handles re-initializing the configuration by throwing out everything and starting from scratch or should we rather go with the log4j model of a somewhat more complicated reload? [2] What's supposed to happen if a config reload fails? a) die b) keep the old one (and if: how can we notify the user? If it's a daemon?) [3] How does that fit in if we upgrade Log::Log4perl to handle logger repositories? Couple of comments on your questions: > 2) Is it possible to compartmentalize the config reading+updating, etc. > code to the extent of rather than passing init_and_watch a signal to > catch, that a particular class method could be called instead Sounds good, we should offer that. And, as you said, a callback to init_and_watch which calls back into the application when the re-init happens. > Currently I am using Log4perl extensively in a POE framework app (by the > way it works beautifully, well with the exception of FileRotate FileRotate has been updated a couple of times recently, as far as I'm aware (FileRotate isn't part of Log::Log4perl) it should be stable now. Grab the latest from CPAN. Lots of stuff ahead - and we're always welcoming helping hands :) -- -- Mike Mike Schilli m...@pe... |