Details

    • Type: Wish
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.2.15
    • Fix Version/s: 2.0.6
    • Component/s: Appenders, Core
    • Labels:
      None
    • Environment:
      NA

      Description

      I would like to see an API that flushes any appenders that have buffered data. E.g. a method LogManager.Flush(). An application might call such a method at regular intervals, e.g. on a Timer.

      A naive implementation with the current log4net would iterate through appenders, looking for those that support flushing, and call the appender's flush method, e.g.:

      foreach (IAppender appender in
      LogManager.GetRepository().GetAppenders())

      { BufferingAppenderSkeleton bas = appender as BufferingAppenderSkeleton; if (bas != null) bas.Flush(); }

      But (a) I'm not sure this is thread-safe and (b) there are potentially other appenders that may want to be able to flush data (e.g. a TextWriterAppender with ImmediateFlush = false).

      The request consists of:

      • Add an interface, IFlushableAppender or equivalent, with a single method Flush().
      • Implement this interface in all relevant appenders (BufferingAppenderSkeleton, TextWriterAppender, ...)
      • Add a thread-safe static Flush() method to LogManager.

        Issue Links

          Activity

          Hide
          bodewig Stefan Bodewig added a comment -

          you are right, done.

          Show
          bodewig Stefan Bodewig added a comment - you are right, done.
          Hide
          joe JocularJoe added a comment -

          Thanks for doing this, but just one point.

          AppenderSkeleton.Flush should return true.

          Only a buffering or asynchronous appender would ever return false, generally because the timeout has expired before all buffered events have been processed. Other appenders will want to inherit an implementation that returns true.

          Show
          joe JocularJoe added a comment - Thanks for doing this, but just one point. AppenderSkeleton.Flush should return true. Only a buffering or asynchronous appender would ever return false, generally because the timeout has expired before all buffered events have been processed. Other appenders will want to inherit an implementation that returns true.
          Hide
          bodewig Stefan Bodewig added a comment -

          has been trivial enough for me to do

          svn revision 1765389

          Show
          bodewig Stefan Bodewig added a comment - has been trivial enough for me to do svn revision 1765389
          Hide
          joe JocularJoe added a comment -

          I'm happy to create a pull request for this, or maybe you prefer to do it yourself as it's so trivial?

          Show
          joe JocularJoe added a comment - I'm happy to create a pull request for this, or maybe you prefer to do it yourself as it's so trivial?
          Hide
          bodewig Stefan Bodewig added a comment -

          fine with me

          Show
          bodewig Stefan Bodewig added a comment - fine with me
          Hide
          joe JocularJoe added a comment -

          Yes. I thought we'd keep the IFlushable interface, but implement it in AppenderSkeleton as a virtual method that just returns true.

          Appenders that derive from AppenderSkeleton would then not need to implement IFlushable, instead would override AppenderSkeleton.Flush. Appenders that implement IAppender but don't derive from AppenderSkeleton would decide whether to opt in to IFlushable.

          My thought was that this would help avoid appender developers (most of whom will derive from AppenderSkeleton) implementing IFlushable with a non-virtual method, and therefore making inheritance more difficult.

          It's a minor point, but if it's going to be done, now's the time to do it.

          Show
          joe JocularJoe added a comment - Yes. I thought we'd keep the IFlushable interface, but implement it in AppenderSkeleton as a virtual method that just returns true. Appenders that derive from AppenderSkeleton would then not need to implement IFlushable, instead would override AppenderSkeleton.Flush. Appenders that implement IAppender but don't derive from AppenderSkeleton would decide whether to opt in to IFlushable. My thought was that this would help avoid appender developers (most of whom will derive from AppenderSkeleton) implementing IFlushable with a non-virtual method, and therefore making inheritance more difficult. It's a minor point, but if it's going to be done, now's the time to do it.
          Hide
          bodewig Stefan Bodewig added a comment -

          wouldn't you still need IFlushable for ILoggerRepository's interaction with LogManager?

          Show
          bodewig Stefan Bodewig added a comment - wouldn't you still need IFlushable for ILoggerRepository 's interaction with LogManager ?
          Hide
          JoeJoe Joe added a comment -

          Stefan Bodewig - it occurs to me that it would have been better to implement IFlushable as an empty virtual method in AppenderSkeleton.

          A trivial change now, but a breaking change once it's released.

          What do you think?

          Show
          JoeJoe Joe added a comment - Stefan Bodewig - it occurs to me that it would have been better to implement IFlushable as an empty virtual method in AppenderSkeleton. A trivial change now, but a breaking change once it's released. What do you think?
          Hide
          bodewig Stefan Bodewig added a comment -

          fixed with svn revision 1765137

          Many thanks JocularJoe

          Show
          bodewig Stefan Bodewig added a comment - fixed with svn revision 1765137 Many thanks JocularJoe
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user bodewig commented on the issue:

          https://github.com/apache/log4net/pull/37

          Many thanks @JJoe2 and sorry for the hassle you had to go through.

          tracked as LOG4NET-511 - https://issues.apache.org/jira/browse/LOG4NET-511

          Show
          githubbot ASF GitHub Bot added a comment - Github user bodewig commented on the issue: https://github.com/apache/log4net/pull/37 Many thanks @JJoe2 and sorry for the hassle you had to go through. tracked as LOG4NET-511 - https://issues.apache.org/jira/browse/LOG4NET-511
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user JJoe2 closed the pull request at:

          https://github.com/apache/log4net/pull/25

          Show
          githubbot ASF GitHub Bot added a comment - Github user JJoe2 closed the pull request at: https://github.com/apache/log4net/pull/25
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user JJoe2 opened a pull request:

          https://github.com/apache/log4net/pull/25

          API to flush appenders that buffer logging data

          Implementation of https://issues.apache.org/jira/browse/LOG4NET-511

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/JJoe2/log4net wip/Flush

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/log4net/pull/25.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #25


          commit a66d2fc61c4a1cfd8317aee41385ef6c43373ffc
          Author: JoeJoe <jocularjoe@hotmail.com>
          Date: 2016-04-22T19:21:07Z

          Use UTC internally to avoid ambiguous times

          commit 9e9f1759d6a80594cd2d12bd54a9e3af9e7cfaa0
          Author: JoeJoe <jocularjoe@hotmail.com>
          Date: 2016-04-23T19:58:36Z

          Implement flushing of appenders that buffer data


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user JJoe2 opened a pull request: https://github.com/apache/log4net/pull/25 API to flush appenders that buffer logging data Implementation of https://issues.apache.org/jira/browse/LOG4NET-511 You can merge this pull request into a Git repository by running: $ git pull https://github.com/JJoe2/log4net wip/Flush Alternatively you can review and apply these changes as the patch at: https://github.com/apache/log4net/pull/25.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #25 commit a66d2fc61c4a1cfd8317aee41385ef6c43373ffc Author: JoeJoe <jocularjoe@hotmail.com> Date: 2016-04-22T19:21:07Z Use UTC internally to avoid ambiguous times commit 9e9f1759d6a80594cd2d12bd54a9e3af9e7cfaa0 Author: JoeJoe <jocularjoe@hotmail.com> Date: 2016-04-23T19:58:36Z Implement flushing of appenders that buffer data

            People

            • Assignee:
              Unassigned
              Reporter:
              JoeJoe Joe
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development