Solr
  1. Solr
  2. SOLR-4622

deprecate usage of DEFAULT_HOST_CONTEXT and DEFAULT_HOST_PORT

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 4.3, Trunk
    • Component/s: None
    • Labels:
      None

      Description

      Frequently, people who try to use solr cloud in a differnet servlet container (or in a jetty other then the preconfigured one supplied) they can be easily confused as to why/how/where solr is trying to hostContext=solr and hostPort=8983.

      these can be explicitly overridden in solr.xml, and the defaults are setup to read from system properties, but we should eliminate the hardcoded defaults from the java code (where most users can't even grep for them) and instead specify defaults for the sys properties in the example configs.

      1. SOLR-4622.patch
        0.4 kB
        Hoss Man
      2. SOLR-4622__phase3_trunk_only.patch
        4 kB
        Hoss Man
      3. SOLR-4622__phase3_trunk_only.patch
        4 kB
        Hoss Man

        Issue Links

          Activity

          Hide
          Hoss Man added a comment -

          Proposal...

          1) change the example solr.xml on 4x and trunk to use the current hardcoded defaults as defaults for ht sys properties...

          -  <cores adminPath="/admin/cores" defaultCoreName="collection1" host="${host:}" hostPort="${jetty.port:}" hostContext="${hostContext:}" zkClientTimeout="${zkClientTimeout:15000}">
          +  <cores adminPath="/admin/cores" defaultCoreName="collection1" host="${host:}" hostPort="${jetty.port:8983}" hostContext="${hostContext:solr}" zkClientTimeout="${zkClientTimeout:15000}">
          

          2) change the 4x code that uses DEFAULT_HOST_CONTEXT and DEFAULT_HOST_PORT to log a warning that they are deprecated if they are used, along with what their values are and how/where to explicitly set them

          3) change the trunk code that uses DEFAULT_HOST_CONTEXT and DEFAULT_HOST_PORT to fail fast if they aren't specified in solr.xml.

          Show
          Hoss Man added a comment - Proposal... 1) change the example solr.xml on 4x and trunk to use the current hardcoded defaults as defaults for ht sys properties... - <cores adminPath="/admin/cores" defaultCoreName="collection1" host="${host:}" hostPort="${jetty.port:}" hostContext="${hostContext:}" zkClientTimeout="${zkClientTimeout:15000}"> + <cores adminPath="/admin/cores" defaultCoreName="collection1" host="${host:}" hostPort="${jetty.port:8983}" hostContext="${hostContext:solr}" zkClientTimeout="${zkClientTimeout:15000}"> 2) change the 4x code that uses DEFAULT_HOST_CONTEXT and DEFAULT_HOST_PORT to log a warning that they are deprecated if they are used, along with what their values are and how/where to explicitly set them 3) change the trunk code that uses DEFAULT_HOST_CONTEXT and DEFAULT_HOST_PORT to fail fast if they aren't specified in solr.xml.
          Hide
          Hoss Man added a comment -

          Attached patch implements "Phase #1 & #2"

          Looking to commit this to trunk, merge to 4x, and then subsequently make another commit to trunk to remove the deprecated constants from trunk and replace the log.warn()ings with the commented out exceptions.

          comments?

          Show
          Hoss Man added a comment - Attached patch implements "Phase #1 & #2" Looking to commit this to trunk, merge to 4x, and then subsequently make another commit to trunk to remove the deprecated constants from trunk and replace the log.warn()ings with the commented out exceptions. comments?
          Hide
          Mark Miller added a comment -

          Sounds good.

          Show
          Mark Miller added a comment - Sounds good.
          Hide
          Hoss Man added a comment -

          Committed revision 1463242.
          Committed revision 1463251.

          Phase 1 & 2 committed to trunk and 4x.

          I'll deal with phrase #3 in a few days in case any dust gets stirred up.

          Show
          Hoss Man added a comment - Committed revision 1463242. Committed revision 1463251. Phase 1 & 2 committed to trunk and 4x. I'll deal with phrase #3 in a few days in case any dust gets stirred up.
          Hide
          Steve Rowe added a comment -

          test comment

          Show
          Steve Rowe added a comment - test comment
          Hide
          Hoss Man added a comment -

          NOTE: reviewing a recent question on solr-user makes me realize we should deal with "zkHost" in a similar fashion.

          there is already support to read it from solr.xml and if not set then check a system property with the same name. similar to the hostContext and hostPort changes already made above, using the zkHost system property directly should cause a warning in 4x encouraging people to add zkHost="${zkHost}" to their solr.xml if they want ot continue to use a system property, and should be removed from trunk so that it falls back on the normal config property substitution instead of an explicit System.getProperty() call.

          Show
          Hoss Man added a comment - NOTE: reviewing a recent question on solr-user makes me realize we should deal with "zkHost" in a similar fashion. there is already support to read it from solr.xml and if not set then check a system property with the same name. similar to the hostContext and hostPort changes already made above, using the zkHost system property directly should cause a warning in 4x encouraging people to add zkHost="${zkHost}" to their solr.xml if they want ot continue to use a system property, and should be removed from trunk so that it falls back on the normal config property substitution instead of an explicit System.getProperty() call.
          Hide
          Mark Miller added a comment -

          I think zkHost will likely fall out of solr.xml so that we can put those in zk. I think we really want to get system settings in the cloud just like core settings. It's fairly annoying now that to make many changes you need to update solr.xml on every node.

          Show
          Mark Miller added a comment - I think zkHost will likely fall out of solr.xml so that we can put those in zk. I think we really want to get system settings in the cloud just like core settings. It's fairly annoying now that to make many changes you need to update solr.xml on every node.
          Hide
          Hoss Man added a comment -

          I think zkHost will likely fall out of solr.xml so that we can put those in zk. I think we really want to get system settings in the cloud just like core settings.

          I've split this idea out into SOLR-4697 since evidently it's not a slam dunk that everyone agrees with, and i don't want it to distract from the work already done here that i plan on finishing up.

          Show
          Hoss Man added a comment - I think zkHost will likely fall out of solr.xml so that we can put those in zk. I think we really want to get system settings in the cloud just like core settings. I've split this idea out into SOLR-4697 since evidently it's not a slam dunk that everyone agrees with, and i don't want it to distract from the work already done here that i plan on finishing up.
          Hide
          Hoss Man added a comment -

          attached a trunk only patch for implementing phrase#3 of hte original proposal. (been holding off while a lot of CoreContainer/solr.xml stuff was up in the air)

          All tests pass, i'll commit once 4.3 is closer to fully baked so as to not make things hard for any last minute bug fix backports.

          Show
          Hoss Man added a comment - attached a trunk only patch for implementing phrase#3 of hte original proposal. (been holding off while a lot of CoreContainer/solr.xml stuff was up in the air) All tests pass, i'll commit once 4.3 is closer to fully baked so as to not make things hard for any last minute bug fix backports.
          Hide
          Hoss Man added a comment -

          revised trunk only patch since some things got refactored.

          now that 4.3 is out, i'll commit this to trunk as soon as i'm not longer on a train and have reliable net.

          Show
          Hoss Man added a comment - revised trunk only patch since some things got refactored. now that 4.3 is out, i'll commit this to trunk as soon as i'm not longer on a train and have reliable net.
          Hide
          Hoss Man added a comment -

          Committed revision 1480160.

          Show
          Hoss Man added a comment - Committed revision 1480160.
          Hide
          Uwe Schindler added a comment -

          Closed after release.

          Show
          Uwe Schindler added a comment - Closed after release.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development