Solr
  1. Solr
  2. SOLR-1846

Remove support for (broken) abortOnConfigurationError

    Details

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

      Description

      Setting abortOnConfigurationError==false has not worked for some time, and based on a POLL of existing users, no one seems to need/want it, so we should remove support for it completely to make error handling and reporting work more cleanly.

      http://n3.nabble.com/POLL-Users-of-abortOnConfigurationError-tt484030.html#a484030

      1. SOLR-1846.patch
        22 kB
        Hoss Man
      2. SOLR-1846.patch
        12 kB
        Hoss Man

        Issue Links

          Activity

          Hide
          Hoss Man added a comment -

          Attached patch should get us to a good point to tackle some of the related issues. It updates all code paths (unless i missed one) that put something into SolrConfig.severeErrors so that that code path also explicilty throws the corrisponding exception.

          This seems to be working well and is a good base for building up better per-core error reporting in SolrDispatchFilter (because now all the exceptions can be propogated up to CoreContainer and tracked per core)

          As is, this patch breaks BadIndexSchemaTest ... and i'm not really sure what the 'right' fix is ... the test explicitly expects a bad schema.xml to be loaded properly, and then looks for 3 errors in SOlrCOnfig.severeErrors – errors are still added to severeErrors befor getting thrown, btu the test still errors out during setUp because the SolrCore can't be inited (because the IndexSchema doesn't finishing initing)

          my best suggestion: split the test into three test, each using a differnet config (one per type of error tested) and assert that we get an exception during setUp.

          Show
          Hoss Man added a comment - Attached patch should get us to a good point to tackle some of the related issues. It updates all code paths (unless i missed one) that put something into SolrConfig.severeErrors so that that code path also explicilty throws the corrisponding exception. This seems to be working well and is a good base for building up better per-core error reporting in SolrDispatchFilter (because now all the exceptions can be propogated up to CoreContainer and tracked per core) As is, this patch breaks BadIndexSchemaTest ... and i'm not really sure what the 'right' fix is ... the test explicitly expects a bad schema.xml to be loaded properly, and then looks for 3 errors in SOlrCOnfig.severeErrors – errors are still added to severeErrors befor getting thrown, btu the test still errors out during setUp because the SolrCore can't be inited (because the IndexSchema doesn't finishing initing) my best suggestion: split the test into three test, each using a differnet config (one per type of error tested) and assert that we get an exception during setUp.
          Hide
          Hoss Man added a comment -

          updated patch to included the above mentioned suggestions for BadIndexSchemaTest ... it's actually a lot easier now that we have SolrTestCaseJ4

          As part of this, i also enhanced AbstractPluginLoader to include the getMessage() from the exception it wraps in it's own message – this made the test easier to write, but also seems like a good idea in general (prior to this patch the exception messages just told you what type of plugin had a problem being initialized, and you had to look at the nested exception to see why it had a problem)

          Show
          Hoss Man added a comment - updated patch to included the above mentioned suggestions for BadIndexSchemaTest ... it's actually a lot easier now that we have SolrTestCaseJ4 As part of this, i also enhanced AbstractPluginLoader to include the getMessage() from the exception it wraps in it's own message – this made the test easier to write, but also seems like a good idea in general (prior to this patch the exception messages just told you what type of plugin had a problem being initialized, and you had to look at the nested exception to see why it had a problem)
          Hide
          Hoss Man added a comment -

          Committed revision 945886.

          Show
          Hoss Man added a comment - Committed revision 945886.

            People

            • Assignee:
              Hoss Man
              Reporter:
              Hoss Man
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development