[logback-user] how to graceful shutdown logback with AsyncAppenders and without immediateFlush

[Michael Reinhold]

Hi Gena,

The intent of the shutdown hook functionality is not to be a general
application shutdown hook, but to provide reasonably ways of having logback
wait for a some condition before shutting down (not just calling
LoggerContext.stop immediately). Logback can provide one or more general
mechanisms for performing this action, however if the user's needs differ
they could implement their own.

The benefit of utilizing a logback based shutdown hook for cleaning up
logback resources is that logback can ensure that shutdown hook is properly
removed when the context is reset (which reduces the complexity of using
the Runtime hook methods directly). Additionally, the user can utilize
existing, tested implementations if one of the Logback provided shutdown
hook mechanisms is appropriate for the user's application or scenario.

Just because a shutdown hook implementation is simple (such as a one-liner)
does not make the logback managed mechanism invalid or irrelevant. The user
still has the choice to implement a hook however they choose - via a
predefined, managed hook or a completely custom hook.

I'm not sure how this design is premature optimization. It is a very simple
design for flexibility - there is no impact to speed, debuggability, or
maintenance. Though some may qualify it as "you aren't going to need it",
YAGNI more accurately refers to features which significantly take away from
the development of core functionality, testing, documentation, etc or add
significant complexity, limit future flexibility, etc. The
ShutdownHookAction and associated classes are compact, well-defined, do not
detract from the maintainability of logback as a whole, do not add
significant complexity, nor limit future flexibility.

To be completely honest, Ceki first suggested using a Joran action about a
week ago. Since then I have only spent about 4 hours researching,
developing, and testing the ShutdownActionHook solution - which is now 100%
complete from a functionality standpoint and 95% complete overall (just
pending documentation and some unit testing). The other changes to
AsyncAppender had been built into a proof of concept and re-implemented
with suggestions from the pull request over a week ago. I have probably
spent more time debating the merits of your proposal vs mine than actually
researching, developing, testing, and documenting!   :)

Not that I regret the debate by any means - it has been a healthy design
discussion and has helped to clarify other use-cases and mechanisms that I
may not have thought of otherwise. That being said, with buy-in from
maintainers of Logback on this design (at least to this point), I don't see
much point in debating further. It still seems to me that the best solution
is the blending of our two proposals - using my structure to provide
flexibility and your solution to transition the logback mode. Since my
solution is nearly complete and yours may be a ways off, this seems like a
natural fit both technically and logistically. That being said, if anyone
from Logback team wants to weigh in on how they feel would be best to
proceed in light of the proposals and discussion, I am open to proceeding
with the result of the group decision.

At the very least, regardless of what proposal or blending of proposals is
approved, I believe that the changes already made to the AsyncAppender in
the open pull request are still valid since that is what ensures that
LoggerContext.stop does not return until AsyncAppender has flushed its
queue or reached its configured timeout. From my perspective, it is just
the discussion around the shutdown hook implementation components that is
in contention, agreed?

Thanks!

Regards,

Mike Reinhold




On Thu, Mar 20, 2014 at 3:03 PM, Gena Makhomed <gmm at csdoc.com> wrote:

> On 20.03.2014 17:17, Michael Reinhold wrote:
>
>  All that being said, I'm not quite sure why you are resistant to a
>> framework for implementing shutdown hooks. Is there a particular reason
>> you object to the user being able to select which shutdown mechanism
>> should be employed by Logback? I understand that you feel your solution
>> covers most or all cases, but is it not possible that the end user of
>> Logback may disagree and prefer to apply their own shutdown procedure or
>> policy?
>>
>
> Right now what user can do inside logback shutdown hook - is just one
> action: calling LoggerContext.stop after stopping other app subsystems.
>
> And, if user do graceful shutdown of other subsystems in shutdown hook
> called "logback shutdown hook" - in fact they write own java code for
> application shutdown hook, but falsely name it "logback shutdown hook"
>
> This mismatch between name of logback shutdown hook and its role
> of application shutdown hook is the main reason why I am resistant
> to a creating yet one more framework for implementing shutdown hooks.
>
> Second reason: framework for implementing shutdown hooks already exists.
> it is Runtime.addShutdownHook() and Runtime.removeShutdownHook() methods
>
> Third reason: if implement my proposed solution in logback,
> the code of logback shutdown hook will look like this:
>
> public void run() { loggerContext.switchToFailSafeMode(); }
>
> If user want to call LoggerContext.stop in own application shutdown hook
> it still can do it in own APPLICATION shutdown hook without any problems
>
> Fourth reason:
>
> I suppose additional attribute <configuration shutdownHook="false">
> (with default value "true") can be added to logback, if some user need
> completely turn off build-in logback shutdown hook and want to write
> own application shutdown hook, using already existing framework for
> implementing shutdown hooks from JVM. But right now this is looks
> like premature optimization:
>
> ========================================================================
>
> http://c2.com/cgi/wiki?PrematureOptimization
>
> "Programmers waste enormous amounts of time thinking about, or worrying
> about, the speed of noncritical parts of their programs, and these attempts
> at efficiency actually have a strong negative impact when debugging and
> maintenance are considered. We should forget about small efficiencies, say
> about 97% of the time: premature optimization is the root of all evil. Yet
> we should not pass up our opportunities in that critical 3%."
>
> ========================================================================
>
> Fifth reason:
>
> see arguments from page
> https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it
>
> "Always implement things when you actually need them, never when you just
> foresee that you need them".
>
> "Until the feature is actually needed, it is difficult to fully define
> what it should do and to test it. If the new feature is not properly
> defined and tested, it may not work correctly, even if it eventually is
> needed".
>
> "Adding the new feature may suggest other new features. If these new
> features are implemented as well, this may result in a snowball effect
> towards feature creep".
>
>
>  As a side question, how long would you estimate it would take to modify
>> logback-core (and/or the other logback components) to implement your
>> proposed solution?
>>
>
> I am not learn all details of code, but suppose that this is
> not very difficult task. May be few weeks, if work in spare time
> (need to read all sources of logback and understand internal logic).
>
>
>  Are you currently working on implementing
>> this switchToFailSafe functionality
>>
>
> Right now - not, because I don't know if this code will be accepted
> or rejected, my first try was to ask about such idea - if idea accepted,
> as the next step I can try to write patch with this idea implementation.
>
>
>  or are you proposing that someone else from the logback community develop
>> it?
>>
>
> I just proposing idea how to make logback graceful shutdown solution.
> If this idea will be accepted and approved, then next step can be done.
>
> If someone else from the logback community want
> to code this idea - I have nothing against this.
>
>
> --
> Best regards,
>  Gena
> _______________________________________________
> Logback-user mailing list
> Logback-user at qos.ch
> http://mailman.qos.ch/mailman/listinfo/logback-user
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.qos.ch/pipermail/logback-user/attachments/20140320/14dd0e99/attachment-0001.html>