Lucene - Core
  1. Lucene - Core
  2. LUCENE-3598

Improve InfoStream class in trunk to be more consistent with logging-frameworks like slf4j/log4j/commons-logging

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 4.0-ALPHA
    • Component/s: core/index
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      Followup on a thread by Shai Erea on java-dev@lao: I already discussed with Robert about that, that there is one thing missing. Currently the IW only checks if the infoStream!=null and then passes the message to the method, and that may ignore it. For your requirement it is the case that this is enabled or disabled dynamically. Unfortunately if the construction of the message is heavy, then this wastes resources.

      I would like to add another method to this class: abstract boolean isEnabled() that can also be implemented. I would then replace all null checks in IW by this method. The default config in IW would be changed to use a NoOutputInfoStream that returns false here and ignores the message.

      A simple logger wrapper for e.g. log4j / slf4j then could look like (ignoring component, could be enabled):

      Loger log = YourLoggingFramework.getLogger(IndexWriter.class);
      
      public void message(String component, String message) {
        log.debug(component + ": " + message);
      }
      
      public boolean isEnabled(String component) {
        return log.isDebugEnabled();
      }
      

      Using this you could enable/disable logging live by e.g. the log4j management console of your app server by enabling/disabling IndexWriter.class logging.

      The changes are really simple:

      • PrintStreamInfoStream returns true, always, mabye make it dynamically enable/disable to allow Shai's request
      • infoStream.getDefault() is never null and can never be set to null. Instead the default is a singleton NoOutputInfoStream that returns false of isEnabled(component).
      • All null checks on infoStream should be replaced by infoStream.isEanbled(component), this is possible as always != null. There are no slowdowns by this - it's like Collections.emptyList() instead stupid null checks.
      1. LUCENE-3598.patch
        37 kB
        Michael McCandless
      2. LUCENE-3598.patch
        63 kB
        Uwe Schindler
      3. LUCENE-3598.patch
        61 kB
        Uwe Schindler
      4. LUCENE-3598.patch
        8 kB
        Uwe Schindler
      5. LUCENE-3598.patch
        7 kB
        Uwe Schindler

        Activity

        Uwe Schindler created issue -
        Uwe Schindler made changes -
        Field Original Value New Value
        Description Followup on a [thread by Shai Erea on java-dev@lao|http://lucene.472066.n3.nabble.com/IndexWriter-infoStream-is-final-td3537485.html]: I already discussed with Robert about that, that there is one thing missing. Currently the IW only checks if the infoStream!=null and then passes the message to the method, and that *may* ignore it. For your requirement it is the case that this is enabled or disabled dynamically. Unfortunately if the construction of the message is heavy, then this wastes resources.

        I would like to add another method to this class: abstract boolean isMessageEnabled() that can also be implemented. I would then replace all null checks in IW by this method. The default config in IW would be changed to use a NoOutputInfoStream that returns false here and ignores the message.

        A simple logger wrapper for e.g. log4j / slf4j then could look like (ignoring component, could be enabled):

        Loger log = YourLoggingFramework.getLogger(IndexWriter.class);

        {code:java}
        public void message(String component, String message) {
          log.debug(component + ": " + message);
        }

        public boolean isMessageEnabled(String component) {
          return log.isDebugEnabled();
        }
        {code}

        Using this you could enable/disable logging live by e.g. the log4j management console of your app server by enabling/disabling IndexWriter.class logging.

        The changes are really simple:
        - PrintStreamInfoStream returns true, always, mabye make it dynamically enable/disable to allow Shai's request
        - infoStream.getDefault() is never null and can never be set to null. Instead the default is a singleton NoOutputInfoStream that returns false of isMessageEnabled().
        - All null checks on infoStream should be replaced by infoStream.isMessageEanbled(component), this is possible as always != null. There are no slowdowns by this - it's like Collections.emptyList() instead stupid null checks.
        Followup on a [thread by Shai Erea on java-dev@lao|http://lucene.472066.n3.nabble.com/IndexWriter-infoStream-is-final-td3537485.html]: I already discussed with Robert about that, that there is one thing missing. Currently the IW only checks if the infoStream!=null and then passes the message to the method, and that *may* ignore it. For your requirement it is the case that this is enabled or disabled dynamically. Unfortunately if the construction of the message is heavy, then this wastes resources.

        I would like to add another method to this class: abstract boolean isMessageEnabled() that can also be implemented. I would then replace all null checks in IW by this method. The default config in IW would be changed to use a NoOutputInfoStream that returns false here and ignores the message.

        A simple logger wrapper for e.g. log4j / slf4j then could look like (ignoring component, could be enabled):

        {code:java}
        Loger log = YourLoggingFramework.getLogger(IndexWriter.class);

        public void message(String component, String message) {
          log.debug(component + ": " + message);
        }

        public boolean isMessageEnabled(String component) {
          return log.isDebugEnabled();
        }
        {code}

        Using this you could enable/disable logging live by e.g. the log4j management console of your app server by enabling/disabling IndexWriter.class logging.

        The changes are really simple:
        - PrintStreamInfoStream returns true, always, mabye make it dynamically enable/disable to allow Shai's request
        - infoStream.getDefault() is never null and can never be set to null. Instead the default is a singleton NoOutputInfoStream that returns false of isMessageEnabled().
        - All null checks on infoStream should be replaced by infoStream.isMessageEanbled(component), this is possible as always != null. There are no slowdowns by this - it's like Collections.emptyList() instead stupid null checks.
        Uwe Schindler made changes -
        Attachment LUCENE-3598.patch [ 12505192 ]
        Uwe Schindler made changes -
        Description Followup on a [thread by Shai Erea on java-dev@lao|http://lucene.472066.n3.nabble.com/IndexWriter-infoStream-is-final-td3537485.html]: I already discussed with Robert about that, that there is one thing missing. Currently the IW only checks if the infoStream!=null and then passes the message to the method, and that *may* ignore it. For your requirement it is the case that this is enabled or disabled dynamically. Unfortunately if the construction of the message is heavy, then this wastes resources.

        I would like to add another method to this class: abstract boolean isMessageEnabled() that can also be implemented. I would then replace all null checks in IW by this method. The default config in IW would be changed to use a NoOutputInfoStream that returns false here and ignores the message.

        A simple logger wrapper for e.g. log4j / slf4j then could look like (ignoring component, could be enabled):

        {code:java}
        Loger log = YourLoggingFramework.getLogger(IndexWriter.class);

        public void message(String component, String message) {
          log.debug(component + ": " + message);
        }

        public boolean isMessageEnabled(String component) {
          return log.isDebugEnabled();
        }
        {code}

        Using this you could enable/disable logging live by e.g. the log4j management console of your app server by enabling/disabling IndexWriter.class logging.

        The changes are really simple:
        - PrintStreamInfoStream returns true, always, mabye make it dynamically enable/disable to allow Shai's request
        - infoStream.getDefault() is never null and can never be set to null. Instead the default is a singleton NoOutputInfoStream that returns false of isMessageEnabled().
        - All null checks on infoStream should be replaced by infoStream.isMessageEanbled(component), this is possible as always != null. There are no slowdowns by this - it's like Collections.emptyList() instead stupid null checks.
        Followup on a [thread by Shai Erea on java-dev@lao|http://lucene.472066.n3.nabble.com/IndexWriter-infoStream-is-final-td3537485.html]: I already discussed with Robert about that, that there is one thing missing. Currently the IW only checks if the infoStream!=null and then passes the message to the method, and that *may* ignore it. For your requirement it is the case that this is enabled or disabled dynamically. Unfortunately if the construction of the message is heavy, then this wastes resources.

        I would like to add another method to this class: abstract boolean isEnabled() that can also be implemented. I would then replace all null checks in IW by this method. The default config in IW would be changed to use a NoOutputInfoStream that returns false here and ignores the message.

        A simple logger wrapper for e.g. log4j / slf4j then could look like (ignoring component, could be enabled):

        {code:java}
        Loger log = YourLoggingFramework.getLogger(IndexWriter.class);

        public void message(String component, String message) {
          log.debug(component + ": " + message);
        }

        public boolean isEnabled(String component) {
          return log.isDebugEnabled();
        }
        {code}

        Using this you could enable/disable logging live by e.g. the log4j management console of your app server by enabling/disabling IndexWriter.class logging.

        The changes are really simple:
        - PrintStreamInfoStream returns true, always, mabye make it dynamically enable/disable to allow Shai's request
        - infoStream.getDefault() is never null and can never be set to null. Instead the default is a singleton NoOutputInfoStream that returns false of isEnabled(component).
        - All null checks on infoStream should be replaced by infoStream.isEanbled(component), this is possible as always != null. There are no slowdowns by this - it's like Collections.emptyList() instead stupid null checks.
        Uwe Schindler made changes -
        Attachment LUCENE-3598.patch [ 12505195 ]
        Uwe Schindler made changes -
        Attachment LUCENE-3598.patch [ 12505257 ]
        Uwe Schindler made changes -
        Attachment LUCENE-3598.patch [ 12505258 ]
        Uwe Schindler made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Assignee Uwe Schindler [ thetaphi ]
        Resolution Fixed [ 1 ]
        Michael McCandless made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Michael McCandless made changes -
        Attachment LUCENE-3598.patch [ 12506860 ]
        Michael McCandless made changes -
        Status Reopened [ 4 ] Resolved [ 5 ]
        Fix Version/s 4.0 [ 12314025 ]
        Resolution Fixed [ 1 ]
        Uwe Schindler made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

          • Assignee:
            Uwe Schindler
            Reporter:
            Uwe Schindler
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development