Solr
  1. Solr
  2. SOLR-3032

Deprecate logOnce from SolrException

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.0-ALPHA
    • Component/s: None

      Description

      There seems to be a growing consensus (well, Muir and Hoss agree at least) that having this logOnce concept in SolrException is more trouble than it's worth. Point in case is that trunk (4x) fails to report anything useful in the log file when you define a custom component and don't have any <lib> statements going to the right place.

      So the proposal is to remove the whole logOnce process, supporting variables etc. The first step here will be deprecating the various bits of code in SolrException and starting to remove their usages.

      I'm opening this up for discussion, error reporting seems to be one of those things that generates endless discussion and I'd like them aired before putting too much work into this. My goal will be to have this in the code base by next Tuesday, so speak up.

      1. SOLR-3032.patch
        33 kB
        Erick Erickson
      2. SOLR-3032-3x.patch
        2 kB
        Erick Erickson

        Issue Links

          Activity

          Hide
          Erick Erickson added a comment -

          This should be tackled after the other two JIRAs

          Show
          Erick Erickson added a comment - This should be tackled after the other two JIRAs
          Hide
          Erick Erickson added a comment -

          Should the deprecations be back-ported to 3.x? If we did, we'd be able to remove the actual code that much sooner.

          Show
          Erick Erickson added a comment - Should the deprecations be back-ported to 3.x? If we did, we'd be able to remove the actual code that much sooner.
          Hide
          Mark Miller added a comment -

          For 4, I don't think we should deprecate this - we should simply rip it out.

          Show
          Mark Miller added a comment - For 4, I don't think we should deprecate this - we should simply rip it out.
          Hide
          Yonik Seeley added a comment -

          For 4, I don't think we should deprecate this - we should simply rip it out.

          Yep, for Java API's everything is pretty much fair game when going to a new major version.

          Show
          Yonik Seeley added a comment - For 4, I don't think we should deprecate this - we should simply rip it out. Yep, for Java API's everything is pretty much fair game when going to a new major version.
          Hide
          Erick Erickson added a comment -

          OK, here's a first cut. The rule I tried to follow (and I need to go over it again with fresh eyes) was that if an exception was re-thrown, logging was unnecessary so I took it out.

          As a bonus, SolrConfig.severeErrors is gone as is all the stuff around CoreContainer.abortOnConfigurationError.

          Most of this is unutterably boring, but take a look at SolrDispatchFilter, the real changes are there.

          I'll add deprecation notices to the 3x code, but won't change anything else there.

          I'm putting this out for comments. All tests pass, but I'm not sure tests do much to deal with logging so that probably only proves that things compile.

          I'll look this over again tomorrow, then I expcet I'll commit on Sunday/Monday unless there are howls of protest.

          And I just want to add that modern IDEs make this far too easy. "Back in MY day", real programmers used real editors. See: http://xkcd.com/378/

          Show
          Erick Erickson added a comment - OK, here's a first cut. The rule I tried to follow (and I need to go over it again with fresh eyes) was that if an exception was re-thrown, logging was unnecessary so I took it out. As a bonus, SolrConfig.severeErrors is gone as is all the stuff around CoreContainer.abortOnConfigurationError. Most of this is unutterably boring, but take a look at SolrDispatchFilter, the real changes are there. I'll add deprecation notices to the 3x code, but won't change anything else there. I'm putting this out for comments. All tests pass, but I'm not sure tests do much to deal with logging so that probably only proves that things compile. I'll look this over again tomorrow, then I expcet I'll commit on Sunday/Monday unless there are howls of protest. And I just want to add that modern IDEs make this far too easy. "Back in MY day", real programmers used real editors. See: http://xkcd.com/378/
          Hide
          Erick Erickson added a comment -

          Just deprecates the various c'tors etc that are removed in the trunk patch.

          Show
          Erick Erickson added a comment - Just deprecates the various c'tors etc that are removed in the trunk patch.
          Hide
          Erick Erickson added a comment -

          NOTE: methods etc removed are deprecated in the associated 3x patch.

          If log messages are disappearing, the correct fix should be to find out either where they're being swallowed and report there.

          3x (deprecations only): 1232193
          trunk (code removed): 1232192

          Show
          Erick Erickson added a comment - NOTE: methods etc removed are deprecated in the associated 3x patch. If log messages are disappearing, the correct fix should be to find out either where they're being swallowed and report there. 3x (deprecations only): 1232193 trunk (code removed): 1232192

            People

            • Assignee:
              Erick Erickson
              Reporter:
              Erick Erickson
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development