Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-7871

Platform independent config file instead of solr.in.sh and solr.in.cmd

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 5.2.1
    • Fix Version/s: None
    • Component/s: scripts and tools
    • Labels:

      Description

      Spinoff from SOLR-7043

      The config files solr.in.sh and solr.in.cmd are currently executable batch files, but all they do is to set environment variables for the start scripts on the format key=value

      Suggest to instead have one central platform independent config file e.g. bin/solr.yml or bin/solrstart.properties which is parsed by SolrCLI.java.

      1. SOLR-7871.patch
        22 kB
        Jan Høydahl
      2. SOLR-7871.patch
        60 kB
        Jan Høydahl
      3. SOLR-7871.patch
        62 kB
        Jason Gerlowski
      4. SOLR-7871.patch
        45 kB
        Jason Gerlowski
      5. SOLR-7871.patch
        46 kB
        Jason Gerlowski
      6. SOLR-7871.patch
        50 kB
        Jason Gerlowski
      7. SOLR-7871.patch
        60 kB
        Jason Gerlowski
      8. SOLR-7871.patch
        73 kB
        Jason Gerlowski
      9. SOLR-7871.patch
        76 kB
        Jason Gerlowski
      10. SOLR-7871.patch
        136 kB
        Jason Gerlowski
      11. SOLR-7871.patch
        303 kB
        Jason Gerlowski
      12. SOLR-7871.patch
        136 kB
        Jason Gerlowski
      13. SOLR-7871.patch
        137 kB
        Jason Gerlowski
      14. SOLR-7871.patch
        138 kB
        Jason Gerlowski

        Issue Links

          Activity

          Hide
          janhoy Jan Høydahl added a comment -

          Questions that need to be solved

          • Some of the properties such as JVM options need to be injected when Java starts
          • What config format to choose, where it should live and what to name the file
          • Upgradability - as suggested before, perhaps Solr should ship with e.g. bin/solr.yml.template and never overwrite user's copy

          Benefits of .yml is that it is future proof, supporting lists and other structures, but .properties is probably good enough for our needs.

          Show
          janhoy Jan Høydahl added a comment - Questions that need to be solved Some of the properties such as JVM options need to be injected when Java starts What config format to choose, where it should live and what to name the file Upgradability - as suggested before, perhaps Solr should ship with e.g. bin/solr.yml.template and never overwrite user's copy Benefits of .yml is that it is future proof, supporting lists and other structures, but .properties is probably good enough for our needs.
          Hide
          janhoy Jan Høydahl added a comment -

          I have continued a bit on this one.

          My plan is to move the following functionality from solr.sh|cmd into a new SolrCliConfig.java:

          • Locate correct config file and location
          • Parse the config file
          • Resolve SOLR_PID_DIR, SOLR_TIP, DEFAULT_SERVER_DIR etc
          • Configure defaults for SOLR_URL_SCHEME, SOLR_SSL_OPTS, SOLR_JETTY_CONFIG etc

          The shell script will then call SolrCliConfig and get all variables in return.
          In the first phase, we'll just return a string which the script can evaluate to set all variables.

          I propose that the new config file will be of a simple "properties" format similar to solr.in.sh and be called solr.conf. The new SolrCliConfig will use solr.conf if found, else fallback to and parse solr.in.* as well, for a smooth transition.

          Show
          janhoy Jan Høydahl added a comment - I have continued a bit on this one. My plan is to move the following functionality from solr.sh|cmd into a new SolrCliConfig.java : Locate correct config file and location Parse the config file Resolve SOLR_PID_DIR, SOLR_TIP, DEFAULT_SERVER_DIR etc Configure defaults for SOLR_URL_SCHEME, SOLR_SSL_OPTS, SOLR_JETTY_CONFIG etc The shell script will then call SolrCliConfig and get all variables in return. In the first phase, we'll just return a string which the script can evaluate to set all variables. I propose that the new config file will be of a simple "properties" format similar to solr.in.sh and be called solr.conf . The new SolrCliConfig will use solr.conf if found, else fallback to and parse solr.in.* as well, for a smooth transition.
          Hide
          janhoy Jan Høydahl added a comment -

          Attaching a work in progress patch which is able to resolve correct config file from the usual folders and parse both .sh and .cmd correctly (tested with current default files). A bunch of unit tests already written. The parser throws an exception if complex solr.in.* script is detected, then user needs to edit it first.

          Next step would be to add all the defaults to a central place, and then find a way to replace large chunks of variable magic from bin/solr.*

          I won't be able to continue on this during the summer, but will pick it up again in August...

          Show
          janhoy Jan Høydahl added a comment - Attaching a work in progress patch which is able to resolve correct config file from the usual folders and parse both .sh and .cmd correctly (tested with current default files). A bunch of unit tests already written. The parser throws an exception if complex solr.in.* script is detected, then user needs to edit it first. Next step would be to add all the defaults to a central place, and then find a way to replace large chunks of variable magic from bin/solr.* I won't be able to continue on this during the summer, but will pick it up again in August...
          Hide
          erickerickson Erick Erickson added a comment -

          Jan:

          I took a quick glance and it appears that SOLR-9194 still should be checked in, right?

          Show
          erickerickson Erick Erickson added a comment - Jan: I took a quick glance and it appears that SOLR-9194 still should be checked in, right?
          Hide
          upayavira Upayavira added a comment -

          If you make this look both in your config file, and also in environment variables, you will be able to take the functionality out of bin/solr, but maintain backwards compatibility. Also, I suspect that more and more applications will be expecting to be configured with environment variables given that is how Docker apps expect to be configured, so having both capabilities is valuable.

          Show
          upayavira Upayavira added a comment - If you make this look both in your config file, and also in environment variables, you will be able to take the functionality out of bin/solr, but maintain backwards compatibility. Also, I suspect that more and more applications will be expecting to be configured with environment variables given that is how Docker apps expect to be configured, so having both capabilities is valuable.
          Hide
          janhoy Jan Høydahl added a comment -

          Sure!

          Show
          janhoy Jan Høydahl added a comment - Sure!
          Hide
          janhoy Jan Høydahl added a comment -

          Absolutely, that is what the current patch does. If a certain value is NOT explicitly configured in solr.in.sh, it will attempt to load from System Env variables, if it does not exist there, it will load from the application defaults map (which is going to be initialized from a solrcliconfig.properties inside of WEB-INF/classes.

          Initially I wanted to let environment variables override what was in solr.in.sh, but to keep back-compat, I currently do it the other way around. Anyways, I think we should ship future releases with a bin/solr.conf.template or bin/solr.yml.template where all is commented out and thus falls back to app defaults, unless people either configure environment variabele and/or rename the file to bin/solr.conf.

          Show
          janhoy Jan Høydahl added a comment - Absolutely, that is what the current patch does. If a certain value is NOT explicitly configured in solr.in.sh, it will attempt to load from System Env variables, if it does not exist there, it will load from the application defaults map (which is going to be initialized from a solrcliconfig.properties inside of WEB-INF/classes . Initially I wanted to let environment variables override what was in solr.in.sh, but to keep back-compat, I currently do it the other way around. Anyways, I think we should ship future releases with a bin/solr.conf.template or bin/solr.yml.template where all is commented out and thus falls back to app defaults, unless people either configure environment variabele and/or rename the file to bin/solr.conf .
          Hide
          dsmiley David Smiley added a comment -

          I think the key is, environment variables should override a config file.

          Show
          dsmiley David Smiley added a comment - I think the key is, environment variables should override a config file.
          Hide
          janhoy Jan Høydahl added a comment -

          Agree that env must override config file.

          Show
          janhoy Jan Høydahl added a comment - Agree that env must override config file.
          Hide
          noble.paul Noble Paul added a comment -

          finally, it this going to be a properties file or yaml file?

          Show
          noble.paul Noble Paul added a comment - finally, it this going to be a properties file or yaml file?
          Hide
          janhoy Jan Høydahl added a comment -

          finally, it this going to be a properties file or yaml file?

          In my latest patch I landed on properties format config named bin/solr.conf, which looks exactly like a simple solr.in.sh file:

          SOLR_PORT = 8080
          

          At the end of the day, the more interesting discussion is whether we should move to a new format for 7.x, I can live with properties, yml, json, whatever

          Show
          janhoy Jan Høydahl added a comment - finally, it this going to be a properties file or yaml file? In my latest patch I landed on properties format config named bin/solr.conf , which looks exactly like a simple solr.in.sh file: SOLR_PORT = 8080 At the end of the day, the more interesting discussion is whether we should move to a new format for 7.x, I can live with properties, yml, json, whatever
          Hide
          noble.paul Noble Paul added a comment -

          +1 for properties file format

          What is stopping us from rolling this out in 7.0 ?

          Show
          noble.paul Noble Paul added a comment - +1 for properties file format What is stopping us from rolling this out in 7.0 ?
          Hide
          janhoy Jan Høydahl added a comment -

          I think it would be a great improvement for 7.x. The patch is very much WIP and there will of course be a ton of documentation fixes etc. But the current patch does its best to parse solr.in.sh|cmd so we have a good chance of maintaining back-compat for all of 7.x and then accept only one format in 8.x

          Show
          janhoy Jan Høydahl added a comment - I think it would be a great improvement for 7.x. The patch is very much WIP and there will of course be a ton of documentation fixes etc. But the current patch does its best to parse solr.in.sh|cmd so we have a good chance of maintaining back-compat for all of 7.x and then accept only one format in 8.x
          Hide
          ichattopadhyaya Ishan Chattopadhyaya added a comment - - edited

          +1 to doing this in 7.0 (and removing solr.in.sh/solr.in.cmd), and not bothering about backcompat for solr.in.sh/solr.in.cmd.

          I wonder, though, how will users retain their configurations when they upgrade from 7.x to 7.y, esp. when 7.y introduces newer config defaults? Zookeeper has the concept of zoo_sample.cfg that users then copy over to zoo.cfg. Is that model worth consideration?

          Edit:

          Anyways, I think we should ship future releases with a bin/solr.conf.template or bin/solr.yml.template where all is commented out and thus falls back to app defaults, unless people either configure environment variabele and/or rename the file to bin/solr.conf.

          This ^ makes sense, where users copy over the template file to solr.conf.

          Show
          ichattopadhyaya Ishan Chattopadhyaya added a comment - - edited +1 to doing this in 7.0 (and removing solr.in.sh/solr.in.cmd), and not bothering about backcompat for solr.in.sh/solr.in.cmd. I wonder, though, how will users retain their configurations when they upgrade from 7.x to 7.y, esp. when 7.y introduces newer config defaults? Zookeeper has the concept of zoo_sample.cfg that users then copy over to zoo.cfg. Is that model worth consideration? Edit: Anyways, I think we should ship future releases with a bin/solr.conf.template or bin/solr.yml.template where all is commented out and thus falls back to app defaults, unless people either configure environment variabele and/or rename the file to bin/solr.conf. This ^ makes sense, where users copy over the template file to solr.conf.
          Hide
          janhoy Jan Høydahl added a comment -

          New version of patch

          • Tests back compat
          • Removes bin/solr.in.sh and solr.in.cmd, adds bin/solr.conf.sample
          • Integrated into bin/solr, instead of searching for and sourcing solr.in.sh, we do a call to SolrCliConfig.java which does it all in pure Java, returning a long string which are eval'ed in bin/solr.

          Todo:

          • Integrate into bin/solr.cmd, test on Windows
          • More testing of quoting and corner cases, this may be a bit fragile
          • Currently still values in solr.conf will override existing env in shell, need to do it other way around
          • Decide name of file (solr.conf, solr.properties, solr.in.conf)?
          • The solr.conf file is currently parsed by same code as solr.in.sh, could perhaps look into using some standard properties / config parser from some lib?
          • Documentation
          Show
          janhoy Jan Høydahl added a comment - New version of patch Tests back compat Removes bin/solr.in.sh and solr.in.cmd , adds bin/solr.conf.sample Integrated into bin/solr , instead of searching for and sourcing solr.in.sh , we do a call to SolrCliConfig.java which does it all in pure Java, returning a long string which are eval 'ed in bin/solr. Todo: Integrate into bin/solr.cmd , test on Windows More testing of quoting and corner cases, this may be a bit fragile Currently still values in solr.conf will override existing env in shell, need to do it other way around Decide name of file (solr.conf, solr.properties, solr.in.conf)? The solr.conf file is currently parsed by same code as solr.in.sh , could perhaps look into using some standard properties / config parser from some lib? Documentation
          Hide
          janhoy Jan Høydahl added a comment -

          Should the keys of the new file be UPPERCASE_UNDERSCORE_DELIMITED as before, or should we use dot.separated, or allow both i.e. SOLR_HOME vs solr.home. I see a benefit of keeping keys equal to the env vars to make it easier to lookup what vars can be set, and easier to document. Also easier to migrate from custom solr.in.sh

          Now that solr.conf is not a script anymore, should we barf and exit at unknown keys? We could easily build a list of all supported ones.

          What about SOLR_OPTS that we have today? The current parser understands SOLR_OPTS = $SOLR_OPTS -Dfoo so we could continue to allow that syntax. But since it is no longer bash or CMD.exe that will parse the file, we may choose to support specifying SysProps directly as keys, e.g.

          SOLR_HOME=/opt/solr
          -Dsolr.autoSoftCommit.maxTime=3000
          

          Of course once we spit it back out as a string to be eval'ed, all these need to be concatenated into the SOLR_OPTS var again...

          Letting SolrCliConfig.java handle all the logic of resolving location, parsing config, deciding override rules between ENV and config etc is very nice, but I'm not sure how robust the roundtrip back to a string which is eval'ed is. The reasoning is that over time we'll continue with SOLR-7043 and make bin/solr smaller and smaller as we move logic over to SolrCLI.java. Then at the end of the day, SolrCLI will be able to interact with the SolrCliConfig object directly for much of the decision logic.

          Show
          janhoy Jan Høydahl added a comment - Should the keys of the new file be UPPERCASE_UNDERSCORE_DELIMITED as before, or should we use dot.separated, or allow both i.e. SOLR_HOME vs solr.home . I see a benefit of keeping keys equal to the env vars to make it easier to lookup what vars can be set, and easier to document. Also easier to migrate from custom solr.in.sh Now that solr.conf is not a script anymore, should we barf and exit at unknown keys? We could easily build a list of all supported ones. What about SOLR_OPTS that we have today? The current parser understands SOLR_OPTS = $SOLR_OPTS -Dfoo so we could continue to allow that syntax. But since it is no longer bash or CMD.exe that will parse the file, we may choose to support specifying SysProps directly as keys, e.g. SOLR_HOME=/opt/solr -Dsolr.autoSoftCommit.maxTime=3000 Of course once we spit it back out as a string to be eval'ed, all these need to be concatenated into the SOLR_OPTS var again... Letting SolrCliConfig.java handle all the logic of resolving location, parsing config, deciding override rules between ENV and config etc is very nice, but I'm not sure how robust the roundtrip back to a string which is eval'ed is. The reasoning is that over time we'll continue with SOLR-7043 and make bin/solr smaller and smaller as we move logic over to SolrCLI.java . Then at the end of the day, SolrCLI will be able to interact with the SolrCliConfig object directly for much of the decision logic.
          Hide
          janhoy Jan Høydahl added a comment -

          Welcoming feedback from others about the overall approach taken here, or if there is a better path

          Show
          janhoy Jan Høydahl added a comment - Welcoming feedback from others about the overall approach taken here, or if there is a better path
          Hide
          covolution Gethin James added a comment -

          I having been thinking about the general approach. These days external configuration via env variables or config file is super important. Its great that you have kept the ability to specify SOLR_INCLUDE or look up the config file from a series of well known directories.

          I like the approach taken in SolrResourceLoader.locateSolrHome which looks for a property in order, ie. jndi -> system property or default value. Perhaps we can generalize that approach for configuration, to look for jndi -> system property -> ENV_VAR or default value?

          I didn't think I felt strongly about the actual config file format but I probably favor yaml. If a yaml file is used instead of solr.conf then text editors can do highlighting. It's also machine readable / easier to parse. It can also be versioned, (e.g. docker-compose) where you can move from one version of the configuration to another whilst still being backwards compatible.

          I could probably help out with this in the coming weeks.

          Show
          covolution Gethin James added a comment - I having been thinking about the general approach. These days external configuration via env variables or config file is super important. Its great that you have kept the ability to specify SOLR_INCLUDE or look up the config file from a series of well known directories. I like the approach taken in SolrResourceLoader.locateSolrHome which looks for a property in order, ie. jndi -> system property or default value. Perhaps we can generalize that approach for configuration, to look for jndi -> system property -> ENV_VAR or default value? I didn't think I felt strongly about the actual config file format but I probably favor yaml. If a yaml file is used instead of solr.conf then text editors can do highlighting. It's also machine readable / easier to parse. It can also be versioned, (e.g. docker-compose) where you can move from one version of the configuration to another whilst still being backwards compatible. I could probably help out with this in the coming weeks.
          Hide
          janhoy Jan Høydahl added a comment -

          Perhaps we can generalize that approach for configuration, to look for jndi -> system property -> ENV_VAR or default value?

          Do you propose such generalization in SolrCliConfig to prepare a final list of configs before starting Solr? Or do you propose to change the way Solr core internally looks for all these values, e.g. when substituting ${solr.foo.bar:default} and looking up System.getProperty(), to instead look in these locations?

          I'm still open to discuss the new file format. If we choose YAML, how would we map between key names in ENV, YML and SYSPROP? I would love to have a well defined mapping here, e.g.

          ENV          YAML        SYSPROP 
          SOLR_HOME    solr.home   solr.solr.home
          SOLR_PORT    solr.port   jetty.port
          RMI_PORT     rmi.port    com.sun.management.jmxremote.port
          LOG4J_PROPS  log4j.props log4j.configuration
          ...
          

          I.e. have some convention that env vars are upper case underscore separated while sys props and yaml keys are lowercase dot separated. Many of the sysprops already follow that convention, but as you see from the few examples above, there are many exceptions. If we go for YAML, the structure of the variables will also be clearer, so instead of rmi.port we might want to say solr.monitoring.jmx.port etc.

          Show
          janhoy Jan Høydahl added a comment - Perhaps we can generalize that approach for configuration, to look for jndi -> system property -> ENV_VAR or default value? Do you propose such generalization in SolrCliConfig to prepare a final list of configs before starting Solr? Or do you propose to change the way Solr core internally looks for all these values, e.g. when substituting ${solr.foo.bar:default } and looking up System.getProperty() , to instead look in these locations? I'm still open to discuss the new file format. If we choose YAML, how would we map between key names in ENV, YML and SYSPROP? I would love to have a well defined mapping here, e.g. ENV YAML SYSPROP SOLR_HOME solr.home solr.solr.home SOLR_PORT solr.port jetty.port RMI_PORT rmi.port com.sun.management.jmxremote.port LOG4J_PROPS log4j.props log4j.configuration ... I.e. have some convention that env vars are upper case underscore separated while sys props and yaml keys are lowercase dot separated. Many of the sysprops already follow that convention, but as you see from the few examples above, there are many exceptions. If we go for YAML, the structure of the variables will also be clearer, so instead of rmi.port we might want to say solr.monitoring.jmx.port etc.
          Hide
          janhoy Jan Høydahl added a comment -

          If we replaced start.jar with our own startsolr.jar then the whole command line would look better, i.e. we could use -Dsolr.port=8983 -Dsolr.stop.port=7983, and our ugly ps -ef parsing could be more certain that it found a solr process instead of looking for getting false start.jar matches etc. But this is for another JIRA.

          Feel free Gethin James to jump in and play with some YAML parsing, var mappings, jndi etc.

          Perhaps the immediate goals of this issue should be

          • Possible to override any setting through env.var (now solr.in.xx wins every time)
            • This could be committed as a sub task if so be
          • If you choose to use the config file it should be on a standard format, not platform specific
            • The end result from parsing the file could still be as in this patch, that all is converted to env.vars that the existing bin/solr scripts already know how to parse
          • It should be easy for users to map between env.vars and config file keys. And if we rename env.vars in this process, perhaps deprecate old version and support both for 7.x
          Show
          janhoy Jan Høydahl added a comment - If we replaced start.jar with our own startsolr.jar then the whole command line would look better, i.e. we could use -Dsolr.port=8983 -Dsolr.stop.port=7983 , and our ugly ps -ef parsing could be more certain that it found a solr process instead of looking for getting false start.jar matches etc. But this is for another JIRA. Feel free Gethin James to jump in and play with some YAML parsing, var mappings, jndi etc. Perhaps the immediate goals of this issue should be Possible to override any setting through env.var (now solr.in.xx wins every time) This could be committed as a sub task if so be If you choose to use the config file it should be on a standard format, not platform specific The end result from parsing the file could still be as in this patch, that all is converted to env.vars that the existing bin/solr scripts already know how to parse It should be easy for users to map between env.vars and config file keys. And if we rename env.vars in this process, perhaps deprecate old version and support both for 7.x
          Hide
          gerlowskija Jason Gerlowski added a comment - - edited

          Trivially updated patch resolves a few conflicts on master.

          Welcoming feedback from others about the overall approach taken here, or if there is a better path

          Happy to chime in on a few specific aspects:

          • +1 for keeping property file format, at least for now.
          • +1 for keeping UPPERCASE_UNDERSCORE_DELIMITED key values
          • +1 for having env vars take precedence/override solr.conf values
          • +1 for exiting-in-error for unknown keys: less trappy for users. This change would stand alone pretty well as its own JIRA though.

          The solr.conf file is currently parsed by same code as solr.in.sh, could perhaps look into using some standard properties / config parser from some lib

          The Java Properties class looks like it'd get us pretty far on its own. It's got parsing, getters for props, etc. Archaius offers built in support for "cascading" configuration sources (e.g. use ENV-VARS if present, if not use config-file values, if those aren't present, ... ) Though that might bring in a little more than we're looking for. That said, neither of those libraries are well-equipped to parse the sorts of entries we've got in bin/solr.in.cmd, so until we drop support for the old formats, we're going to see limited value from a library. At least as I see things; could be missing something.

          I've got some free time in the next two weeks, so I'd like to take this forward if there's enough consensus on what the next steps look like. As I understand it, the next few things are:

          • add more tests
          • Port the bin/solr changes over to bin/solr.cmd
          • Switch the precedence/override-order on environment-variables/config-file values.

          Does that cover most of the consensus reached so far? Is there anything that you'd prefer to be handled in a sub-issue Jan Høydahl?

          Show
          gerlowskija Jason Gerlowski added a comment - - edited Trivially updated patch resolves a few conflicts on master. Welcoming feedback from others about the overall approach taken here, or if there is a better path Happy to chime in on a few specific aspects: +1 for keeping property file format, at least for now. +1 for keeping UPPERCASE_UNDERSCORE_DELIMITED key values +1 for having env vars take precedence/override solr.conf values +1 for exiting-in-error for unknown keys: less trappy for users. This change would stand alone pretty well as its own JIRA though. The solr.conf file is currently parsed by same code as solr.in.sh, could perhaps look into using some standard properties / config parser from some lib The Java Properties class looks like it'd get us pretty far on its own. It's got parsing, getters for props, etc. Archaius offers built in support for "cascading" configuration sources (e.g. use ENV-VARS if present, if not use config-file values, if those aren't present, ... ) Though that might bring in a little more than we're looking for. That said, neither of those libraries are well-equipped to parse the sorts of entries we've got in bin/solr.in.cmd, so until we drop support for the old formats, we're going to see limited value from a library. At least as I see things; could be missing something. I've got some free time in the next two weeks, so I'd like to take this forward if there's enough consensus on what the next steps look like. As I understand it, the next few things are: add more tests Port the bin/solr changes over to bin/solr.cmd Switch the precedence/override-order on environment-variables/config-file values. Does that cover most of the consensus reached so far? Is there anything that you'd prefer to be handled in a sub-issue Jan Høydahl ?
          Hide
          janhoy Jan Høydahl added a comment -

          We should aim towards a robust impl which contains your bullets above and spin off some less important follow-up JIRAs which can be done later.

          Ideally we'd be able to introduce support for new format in a 7.x minor version and at the same time keep full support for the old configs. That way people can try it in 7.x if they want. If the back-compat support is good enough we could even deprecate old configs in a 7.x minor version and make the new config the only one in 8.0.

          Please go ahead making the steps you plan, seeking feedback as you go.

          Question: My current code parses both .in.sh, .in.cmd and .conf into a key-value hashmap which is then used to generate a platform-dependent string which can be EVAL'ed. While this would in theory allow someone to use a .in.sh config on Windows and vice versa, that is not an important feature going forward. Is this just complicating things? Could we replace all of this with a simpler logic? Perhaps a two-step rocket:

          1. Call SolrCliConfig --locate to locate the correct config file, return type absolute config file path as string
          2. If new format then call SolrCliConfig --parse and EVAL the result as the patch does now
          3. But if old format, simply source the file . "$ABS_PATH" in the shell/cmd as we do today - this gives better back-compat
            I initially intended to move much more decision logic over to SolrCliConfig, such as setting defaults for SSL variables if not configured etc. But that could still be done as a separate step-3 invocation, e.g. SolrCliConfig --setDefaults?
          Show
          janhoy Jan Høydahl added a comment - We should aim towards a robust impl which contains your bullets above and spin off some less important follow-up JIRAs which can be done later. Ideally we'd be able to introduce support for new format in a 7.x minor version and at the same time keep full support for the old configs. That way people can try it in 7.x if they want. If the back-compat support is good enough we could even deprecate old configs in a 7.x minor version and make the new config the only one in 8.0. Please go ahead making the steps you plan, seeking feedback as you go. Question: My current code parses both .in.sh , .in.cmd and .conf into a key-value hashmap which is then used to generate a platform-dependent string which can be EVAL'ed. While this would in theory allow someone to use a .in.sh config on Windows and vice versa, that is not an important feature going forward. Is this just complicating things? Could we replace all of this with a simpler logic? Perhaps a two-step rocket: Call SolrCliConfig --locate to locate the correct config file, return type absolute config file path as string If new format then call SolrCliConfig --parse and EVAL the result as the patch does now But if old format, simply source the file . "$ABS_PATH" in the shell/cmd as we do today - this gives better back-compat I initially intended to move much more decision logic over to SolrCliConfig, such as setting defaults for SSL variables if not configured etc. But that could still be done as a separate step-3 invocation, e.g. SolrCliConfig --setDefaults ?
          Hide
          gerlowskija Jason Gerlowski added a comment -

          I like the "two-step rocket" approach, and will move forward with that.

          I'm tempted to vote the other way...for the approach that your existing patch takes. (maintain backcompat by emulating as much current shell/cmd logic as possible in Java-land). It does a great job of trimming logic from the bash/cmd scripts, which is a change I really want to see (see SOLR-11206). But it's hard to make the argument that that is required for this JIRA. The best thing is probably the simplest thing, as you pointed out above.

          So I'll go forward with the approach you laid out above in your previous comment.

          Show
          gerlowskija Jason Gerlowski added a comment - I like the "two-step rocket" approach, and will move forward with that. I'm tempted to vote the other way...for the approach that your existing patch takes. (maintain backcompat by emulating as much current shell/cmd logic as possible in Java-land). It does a great job of trimming logic from the bash/cmd scripts, which is a change I really want to see (see SOLR-11206 ). But it's hard to make the argument that that is required for this JIRA. The best thing is probably the simplest thing, as you pointed out above. So I'll go forward with the approach you laid out above in your previous comment.
          Hide
          janhoy Jan Høydahl added a comment -

          Gethin James, Ishan Chattopadhyaya, Noble Paul: Please chime in so we get a good consensus before writing a lot of code

          My thinking is that if the back-compat logic is 95% equal as today (with exception of resolving solr.in.sh location in Java-land) but we still keep all env.variable names as today, then we have a chance of deprecating the old configs in 7.x and introduce the new format from 8.0.

          Then, once we have the new solr.conf or solr.yml, we can start supporting lowercase dot separated keys as an alternative, we can start renaming some keys while keeping support for the old name as an alias etc.

          Show
          janhoy Jan Høydahl added a comment - Gethin James , Ishan Chattopadhyaya , Noble Paul : Please chime in so we get a good consensus before writing a lot of code My thinking is that if the back-compat logic is 95% equal as today (with exception of resolving solr.in.sh location in Java-land) but we still keep all env.variable names as today, then we have a chance of deprecating the old configs in 7.x and introduce the new format from 8.0. Then, once we have the new solr.conf or solr.yml, we can start supporting lowercase dot separated keys as an alternative, we can start renaming some keys while keeping support for the old name as an alias etc.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Jan Høydahl, I'm in the process of implementing the "2-step rocket" approach you outlined above (absent opinions from anyone else, I decided to start on it).

          I noticed that your current patch expands environment variables referenced in the config file regardless of format (solr.in.sh, solr.in.cmd, solr.conf). I wanted to raise the question of whether that was something you intentionally chose to support in the new solr.conf format, or whether that was just a by product of how you implemented your patch.

          IMO, env-var expansion made sense when the configuration files were executable bash/cmd scripts, but fits less well now that we're going with system independent (and non-executable) file format.

          Just wanted to throw the question out there...

          Show
          gerlowskija Jason Gerlowski added a comment - Jan Høydahl , I'm in the process of implementing the "2-step rocket" approach you outlined above (absent opinions from anyone else, I decided to start on it). I noticed that your current patch expands environment variables referenced in the config file regardless of format (solr.in.sh, solr.in.cmd, solr.conf). I wanted to raise the question of whether that was something you intentionally chose to support in the new solr.conf format, or whether that was just a by product of how you implemented your patch. IMO, env-var expansion made sense when the configuration files were executable bash/cmd scripts, but fits less well now that we're going with system independent (and non-executable) file format. Just wanted to throw the question out there...
          Hide
          janhoy Jan Høydahl added a comment -

          That was just a bi-product of using the same parser. I did not intend to support var expansion. So using a standard Properties or Yaml parser is the way to go.

          Show
          janhoy Jan Høydahl added a comment - That was just a bi-product of using the same parser. I did not intend to support var expansion. So using a standard Properties or Yaml parser is the way to go.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Ah good. In that case, I've attached my initial stab at the 2-step approach you laid out above. I left out the unit-tests that you included in your patch, but I'm just doing that for now. I didn't want to spend time on unit tests that would change drastically if we decided we didn't like this design.

          IMO it turned out pretty nicely. Really simplified a lot of the related Java code. If people are ok with this approach, next steps include:

          • re-write unit tests.
          • env-var precedence/overriding
          • default values
          • Windows support
          Show
          gerlowskija Jason Gerlowski added a comment - Ah good. In that case, I've attached my initial stab at the 2-step approach you laid out above. I left out the unit-tests that you included in your patch, but I'm just doing that for now. I didn't want to spend time on unit tests that would change drastically if we decided we didn't like this design. IMO it turned out pretty nicely. Really simplified a lot of the related Java code. If people are ok with this approach, next steps include: re-write unit tests. env-var precedence/overriding default values Windows support
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Attached patch adds in support for environment-variable overriding of solr.conf files.

          One question this patch raises is how we want to treat the environment-variable-overrides when no config-file is present/specified. If a user uses the solr.conf format, I think it's clear that env-var overrides should be in place. When a user uses solr.in.sh then back-compatibility dictates that we probably shouldn't provide env-var overrides. But when no configuration file is present, it's unclear whether we should choose the old behavior (no env-overrides) or the new behavior (env-var-overrides).

          This patch chooses to provide env-var overrides when no configuration file can be found. That seemed like a relatively safe decision, since chances are low that users have environment-variables with matching names in their scope that they DON'T want passed to Solr. But I'm not attached to this decision at all. I'm happy to change it if others disagree. Any thoughts Jan Høydahl.

          I'm going to do the default values next, and then move on writing unit tests (pending any suggestions/change-requests from others). Is there anywhere specific I should look for defaults to these configuration values? I can use the values that are commented out in the solr.conf/solr.in.sh/solr.in.cmd files currently, but I wasn't sure that was right...

          Show
          gerlowskija Jason Gerlowski added a comment - Attached patch adds in support for environment-variable overriding of solr.conf files. One question this patch raises is how we want to treat the environment-variable-overrides when no config-file is present/specified. If a user uses the solr.conf format, I think it's clear that env-var overrides should be in place. When a user uses solr.in.sh then back-compatibility dictates that we probably shouldn't provide env-var overrides. But when no configuration file is present, it's unclear whether we should choose the old behavior (no env-overrides) or the new behavior (env-var-overrides). This patch chooses to provide env-var overrides when no configuration file can be found. That seemed like a relatively safe decision, since chances are low that users have environment-variables with matching names in their scope that they DON'T want passed to Solr. But I'm not attached to this decision at all. I'm happy to change it if others disagree. Any thoughts Jan Høydahl . I'm going to do the default values next, and then move on writing unit tests (pending any suggestions/change-requests from others). Is there anywhere specific I should look for defaults to these configuration values? I can use the values that are commented out in the solr.conf/solr.in.sh/solr.in.cmd files currently, but I wasn't sure that was right...
          Hide
          janhoy Jan Høydahl added a comment -

          +1 to env vars overriding the defaults if no solr.conf file is found.

          Defaults are hard coded in shell scripts and/or SolrCLI. I was planning to introduce a new defaults properties file for those, see this comment, which is not user editable but is visible in WEB-INF/classes, separate from code. Don't know if this is feasible.

          One of those defaults that the script currently hardcodes is GC_TUNE, which can also be overridden. I wonder if we should consider introducing a separate config file solr_jvm.conf where each line will be treated as a flag to be passed directly to the JVM. This way we avoid one loooong SOLR_OPTS line in solr.conf. Some of the values in this new file would still be commented out to let SolrCLIConfig handle different defaults for Java8 and Java9 etc. What do you think?

          Show
          janhoy Jan Høydahl added a comment - +1 to env vars overriding the defaults if no solr.conf file is found. Defaults are hard coded in shell scripts and/or SolrCLI. I was planning to introduce a new defaults properties file for those, see this comment , which is not user editable but is visible in WEB-INF/classes, separate from code. Don't know if this is feasible. One of those defaults that the script currently hardcodes is GC_TUNE , which can also be overridden. I wonder if we should consider introducing a separate config file solr_jvm.conf where each line will be treated as a flag to be passed directly to the JVM. This way we avoid one loooong SOLR_OPTS line in solr.conf . Some of the values in this new file would still be commented out to let SolrCLIConfig handle different defaults for Java8 and Java9 etc. What do you think?
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Attached patch adds in the Java code to read in a solr/server/solr-defaults.conf file. Turned out nicely.

          As I started to move values from bin/solr etc into the defaults file though, I noticed that many values could be in the defaults file if not for tiny bits of logic involved in their initialization. Examples:

          • SOLR_PORT, STOP_PORT, and RMI_PORT could all be set in a properties file, but they are supposed to track together. (Usually, STOP_PORT is SOLR_PORT - 1000; RMI_PORT is SOLR_PORT + 1000)
          • SOLR_URL_SCHEME relies on SOLR_SSL_ENABLED, as do many of the other SSL options.
          • SOLR_PID_DIR, SOLR_LOGS_DIR, DEFAULT_SERVER_DIR all rely on the value of SOLR_TIP.

          Anyway, I guess the point I'm working up to is that we might be able to pull out more "defaults" if they live in a Java constants file. It's less standard, but has some advantages. I could go either way on it, just wanted to get some opinions.

          (If we still go with a properties/conf file, there are other defaults I can pull out. I just stopped for the night when I ran into the pattern described above.)

          Show
          gerlowskija Jason Gerlowski added a comment - Attached patch adds in the Java code to read in a solr/server/solr-defaults.conf file. Turned out nicely. As I started to move values from bin/solr etc into the defaults file though, I noticed that many values could be in the defaults file if not for tiny bits of logic involved in their initialization. Examples: SOLR_PORT , STOP_PORT , and RMI_PORT could all be set in a properties file, but they are supposed to track together. (Usually, STOP_PORT is SOLR_PORT - 1000; RMI_PORT is SOLR_PORT + 1000) SOLR_URL_SCHEME relies on SOLR_SSL_ENABLED , as do many of the other SSL options. SOLR_PID_DIR , SOLR_LOGS_DIR , DEFAULT_SERVER_DIR all rely on the value of SOLR_TIP . Anyway, I guess the point I'm working up to is that we might be able to pull out more "defaults" if they live in a Java constants file. It's less standard, but has some advantages. I could go either way on it, just wanted to get some opinions. (If we still go with a properties/conf file, there are other defaults I can pull out. I just stopped for the night when I ran into the pattern described above.)
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Adds tests for the configuration precedence and the toBashScript/toCmdScript functionality.

          Next steps are:

          • Windows support
          • pulling out more default values, separate GC options file maybe.

          But this is getting pretty close to being ready.

          Jan Høydahl do you have any opinions on my question earlier about the format/location we keep the defaults in? I made a point earlier that it might make sense to keep them in Java as static constants. Though I also might just be being crazy. Anyway, feedback appreciated.

          Show
          gerlowskija Jason Gerlowski added a comment - Adds tests for the configuration precedence and the toBashScript/toCmdScript functionality. Next steps are: Windows support pulling out more default values, separate GC options file maybe . But this is getting pretty close to being ready. Jan Høydahl do you have any opinions on my question earlier about the format/location we keep the defaults in? I made a point earlier that it might make sense to keep them in Java as static constants. Though I also might just be being crazy. Anyway, feedback appreciated.
          Hide
          janhoy Jan Høydahl added a comment -

          For SOLR_PORT, STOP_PORT and RMI_PORT, I guess it is sufficient to add a default for SOLR_PORT in the defaults-properties file and let the other ports be relative to that, unless specified as ENV or in solr.conf.

          Perhaps we should support variable substitution in solr.conf and defaults.properties in the same way we do in solrconfig.xml, so one could write

          solr.pid.dir = ${solr.tip}/bin
          

          Or specify the defaults as relative paths, documenting that they are relative to SOLR_TIP?

          Feel free to implement the constants in whatever way you feel makes most sense. It is always possible to change it later if we need.

          Show
          janhoy Jan Høydahl added a comment - For SOLR_PORT, STOP_PORT and RMI_PORT, I guess it is sufficient to add a default for SOLR_PORT in the defaults-properties file and let the other ports be relative to that, unless specified as ENV or in solr.conf. Perhaps we should support variable substitution in solr.conf and defaults.properties in the same way we do in solrconfig.xml, so one could write solr.pid.dir = ${solr.tip}/bin Or specify the defaults as relative paths, documenting that they are relative to SOLR_TIP? Feel free to implement the constants in whatever way you feel makes most sense. It is always possible to change it later if we need.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Updated patch moves this along quite a bit.

          • Replaced the use of Java 7 Properties, with PropertiesConfiguration from Apache's commons-configuration (already used as a dependency). This gets us property expansion "for free", as Jan suggested in his previous comment.
          • Added more tests ensuring that our file-parsing can handle strings/numeric values/weird characters/whitespace, etc.
          • Adds a first pass at bin/solr.cmd support. Still need some testing on this though.

          Anyway, this isn't ready for review yet, but it should be ready soon.

          Show
          gerlowskija Jason Gerlowski added a comment - Updated patch moves this along quite a bit. Replaced the use of Java 7 Properties, with PropertiesConfiguration from Apache's commons-configuration (already used as a dependency). This gets us property expansion "for free", as Jan suggested in his previous comment. Added more tests ensuring that our file-parsing can handle strings/numeric values/weird characters/whitespace, etc. Adds a first pass at bin/solr.cmd support. Still need some testing on this though. Anyway, this isn't ready for review yet, but it should be ready soon.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Updated patch improves Windows support, catching a few solr.cmd bugs in the earlier patch.

          The main remaining step is actually something that hasn't been mentioned here yet: the AuthTool (i.e bin/solr auth ) makes edits to the configuration file as a part of enabling/disabling cluster auth. The config-appending logic in that class will have to be revised to handle the new configuration format when the "INCLUDE FILE" has a "*.conf" extension.

          Show
          gerlowskija Jason Gerlowski added a comment - Updated patch improves Windows support, catching a few solr.cmd bugs in the earlier patch. The main remaining step is actually something that hasn't been mentioned here yet: the AuthTool (i.e bin/solr auth ) makes edits to the configuration file as a part of enabling/disabling cluster auth. The config-appending logic in that class will have to be revised to handle the new configuration format when the "INCLUDE FILE" has a "*.conf" extension.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          This patch finished all the work that remained to be done on this (AFAIK). Added in this patch:

          • additional Windows (solr.cmd) bugfixes.
          • install_solr_service.sh now looks for and installs solr.conf files, as well as existing solr.in.sh files
          • Updates the config-manipulation code in bin/solr auth to be able to write the new configuration format.
          • pulls out a lot more default values into "solr-defaults.conf", including GC logging and tuning options.
          • updates asciidoc documentation to refer to the new config file format, and mention that the platform-dependent config files are still usable but deprecated.

          Tests and precommit all pass. AFAIK this is fully ready for review. Would appreciate any feedback anyone has.

          Show
          gerlowskija Jason Gerlowski added a comment - This patch finished all the work that remained to be done on this (AFAIK). Added in this patch: additional Windows (solr.cmd) bugfixes. install_solr_service.sh now looks for and installs solr.conf files, as well as existing solr.in.sh files Updates the config-manipulation code in bin/solr auth to be able to write the new configuration format. pulls out a lot more default values into "solr-defaults.conf", including GC logging and tuning options. updates asciidoc documentation to refer to the new config file format, and mention that the platform-dependent config files are still usable but deprecated. Tests and precommit all pass. AFAIK this is fully ready for review. Would appreciate any feedback anyone has.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Resolves a few merge conflicts so patch can be applied to latest master.

          Jan Høydahl do you have any time this week to provide some feedback on this?

          Show
          gerlowskija Jason Gerlowski added a comment - Resolves a few merge conflicts so patch can be applied to latest master. Jan Høydahl do you have any time this week to provide some feedback on this?
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Updates patch to apply cleanly against latest master.

          Would appreciate any feedback that anyone can offer.

          Show
          gerlowskija Jason Gerlowski added a comment - Updates patch to apply cleanly against latest master . Would appreciate any feedback that anyone can offer.
          Hide
          elyograg Shawn Heisey added a comment -

          The early discussion talked about yaml and .properties.

          This may be moot, but I'm going to say it anyway. Hopefully we can avoid any flamewar, or at least that it will be brief: Unless we're going to switch the entire config system to yaml, I think it would be a bad idea to introduce yaml configuration for this. My objection has nothing to do with the format itself. Solr already has too many different formats for config files, we should not introduce another one unless that introduction actually reduces the format count – but if we're going to consolidate everything on a single format, I think it should be json. On the idea of using .properties: Solr already uses that format for configuration in other places. That format is perfect for key=value settings, and it is completely cross-platform. It is also handled natively by Java. Later it looks like the decision is to go with a .conf file that is pretty much identical in format to the current solr.in.sh script. As long as the operation is bulletproof with that format on both Windows and Linux, and we don't increase the number of different ways that things have to be configured, it may be a reasonable approach, though I think .properties may be better.

          Format aside: I haven't had a chance to apply the patch to verify the end result. I did notice that in the "defaults" config, SOLR_STOP_WAIT is set to only ten seconds. That should be 180. I realize that the "sample" config has it at 180, but it shouldn't take explicitly setting the value to get the current default. Ten seconds is not enough time to wait for Solr to stop unless it's a very small installation, which is why the value was bumped to 180. It rarely actually takes three minutes to stop or start Solr – the shell script will continue before then if it can. The cmd script needs improvement in this area. I have done some experiments with getting the same "waiting up to N seconds" behavior on Windows, and I think we can duplicate it.

          Unless I just overlooked the code, it does look like there's no attempt to parse or convert an existing solr.in.* file. I think we do need to handle a minor version upgrade, probably by converting an existing script to a .conf file.

          On the changes to the service installer script, I notice that if $SOLR_VAR_DIR/solr.conf exists, it is moved into the service-dependent /etc/default file without checking to see if that file already exists. This could cause problems where a user's carefully crafted config file is destroyed by a service install/upgrade. I don't know how likely that situation is in the wild, but it is never a good idea to blindly move/copy config files, not knowing whether you're overwriting something that already exists. I can see that the existing service installer script also does blind config file copies. Since we're changing things, this is an opportunity to do better.

          Another thought: I think it would be a good idea to move a lot more of what the solr script does into SolrCLI, including parsing the platform-independent config file, so that the cmd/shell code is very minimal. I need to sit down sometime and see what I can come up towards that goal.

          I might have more thoughts after applying and reviewing the patch with a closer eye.

          Show
          elyograg Shawn Heisey added a comment - The early discussion talked about yaml and .properties. This may be moot, but I'm going to say it anyway. Hopefully we can avoid any flamewar, or at least that it will be brief: Unless we're going to switch the entire config system to yaml, I think it would be a bad idea to introduce yaml configuration for this. My objection has nothing to do with the format itself. Solr already has too many different formats for config files, we should not introduce another one unless that introduction actually reduces the format count – but if we're going to consolidate everything on a single format, I think it should be json. On the idea of using .properties: Solr already uses that format for configuration in other places. That format is perfect for key=value settings, and it is completely cross-platform. It is also handled natively by Java. Later it looks like the decision is to go with a .conf file that is pretty much identical in format to the current solr.in.sh script. As long as the operation is bulletproof with that format on both Windows and Linux, and we don't increase the number of different ways that things have to be configured, it may be a reasonable approach, though I think .properties may be better. Format aside: I haven't had a chance to apply the patch to verify the end result. I did notice that in the "defaults" config, SOLR_STOP_WAIT is set to only ten seconds. That should be 180. I realize that the "sample" config has it at 180, but it shouldn't take explicitly setting the value to get the current default. Ten seconds is not enough time to wait for Solr to stop unless it's a very small installation, which is why the value was bumped to 180. It rarely actually takes three minutes to stop or start Solr – the shell script will continue before then if it can. The cmd script needs improvement in this area. I have done some experiments with getting the same "waiting up to N seconds" behavior on Windows, and I think we can duplicate it. Unless I just overlooked the code, it does look like there's no attempt to parse or convert an existing solr.in.* file. I think we do need to handle a minor version upgrade, probably by converting an existing script to a .conf file. On the changes to the service installer script, I notice that if $SOLR_VAR_DIR/solr.conf exists, it is moved into the service-dependent /etc/default file without checking to see if that file already exists. This could cause problems where a user's carefully crafted config file is destroyed by a service install/upgrade. I don't know how likely that situation is in the wild, but it is never a good idea to blindly move/copy config files, not knowing whether you're overwriting something that already exists. I can see that the existing service installer script also does blind config file copies. Since we're changing things, this is an opportunity to do better. Another thought: I think it would be a good idea to move a lot more of what the solr script does into SolrCLI, including parsing the platform-independent config file, so that the cmd/shell code is very minimal. I need to sit down sometime and see what I can come up towards that goal. I might have more thoughts after applying and reviewing the patch with a closer eye.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Thanks for giving this a look Shawn. I'll be working this evening on making some of the changes you suggested, but wanted to answer a few questions in the meantime.

          Unless we're going to switch the entire config system to yaml, I think it would be a bad idea to introduce yaml configuration for this.

          Agreed and well said.

          it looks like the decision is to go with a .conf file... it may be a reasonable approach, though I think .properties may be better.

          The name of the file is .conf, but it's actually prop-file syntax that's used. In fact, the code uses the native Java support to parse and use the config. So maybe we should pick a different extension/filename, but sounds like we're in agreement on the format.

          it does look like there's no attempt to parse or convert an existing solr.in.*

          I think you just missed it at first glance. The attached patch still supports solr.in.sh and solr.in.cmd files. For the Linux version, search for the . "$SOLR_INCLUDE" in the patched bin/solr.

          it would be a good idea to move a lot more of what the solr script does into SolrCLI, including parsing the platform-independent config file

          If I understand your suggestion correctly, the current patch already does this. See SolrConfigCreator.java. If you've got something else in mind, or can see a way to have even less bash/batch logic though, I'm all for it.

          Show
          gerlowskija Jason Gerlowski added a comment - Thanks for giving this a look Shawn. I'll be working this evening on making some of the changes you suggested, but wanted to answer a few questions in the meantime. Unless we're going to switch the entire config system to yaml, I think it would be a bad idea to introduce yaml configuration for this. Agreed and well said. it looks like the decision is to go with a .conf file... it may be a reasonable approach, though I think .properties may be better. The name of the file is .conf , but it's actually prop-file syntax that's used. In fact, the code uses the native Java support to parse and use the config. So maybe we should pick a different extension/filename, but sounds like we're in agreement on the format. it does look like there's no attempt to parse or convert an existing solr.in.* I think you just missed it at first glance. The attached patch still supports solr.in.sh and solr.in.cmd files. For the Linux version, search for the . "$SOLR_INCLUDE" in the patched bin/solr . it would be a good idea to move a lot more of what the solr script does into SolrCLI, including parsing the platform-independent config file If I understand your suggestion correctly, the current patch already does this. See SolrConfigCreator.java . If you've got something else in mind, or can see a way to have even less bash/batch logic though, I'm all for it.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          Slightly updated patch. Still a little shaky on the install_solr_service.sh changes. I'll be taking a look at those more today, though I think they're what Shawn suggested in his comment above. The script will now error out instead of overwriting these files if they exist (unless a new "overwrite" flag -o is provided)

          Show
          gerlowskija Jason Gerlowski added a comment - Slightly updated patch. Still a little shaky on the install_solr_service.sh changes. I'll be taking a look at those more today, though I think they're what Shawn suggested in his comment above. The script will now error out instead of overwriting these files if they exist (unless a new "overwrite" flag -o is provided)
          Hide
          elyograg Shawn Heisey added a comment -

          The name of the file is .conf, but it's actually prop-file syntax that's used.

          Interesting. Being pedantic for a moment: I understand the desire to make things easy for novice users and give them a config file they can recognize easily ... but when the user is a veteran of Java-based software, they're going to be a little bit irritated if they ever learn that the file is handled as java properties and doesn't have a .properties extension. It would bug me.

          Side note: Due to the proliferation of configuration formats that I already mentioned, I do think we need a separate issue for a really major change: Re-work how every aspect of the application is configured, in particular to reduce the number of configuration formats. That's probably something to do in master only, because a significant change like that would best be introduced with a new major version.

          If you've got something else in mind, or can see a way to have even less bash/batch logic though, I'm all for it.

          I was indeed thinking about general improvements to SolrCLI, making the actual script extremely simple. I don't think we can eliminate the script, though if that were possible, it would be really cool.

          One pretty major shift (that also needs its own issue) I would like to make possible is the idea of Solr being two Java programs – one program that serves as a "control" process (with a very small max heap) that can start/stop/restart the actual application (Jetty for now). Imagine a situation where you can edit things like Solr's max heap in the UI and also restart Solr from the UI.

          Show
          elyograg Shawn Heisey added a comment - The name of the file is .conf, but it's actually prop-file syntax that's used. Interesting. Being pedantic for a moment: I understand the desire to make things easy for novice users and give them a config file they can recognize easily ... but when the user is a veteran of Java-based software, they're going to be a little bit irritated if they ever learn that the file is handled as java properties and doesn't have a .properties extension. It would bug me. Side note: Due to the proliferation of configuration formats that I already mentioned, I do think we need a separate issue for a really major change: Re-work how every aspect of the application is configured, in particular to reduce the number of configuration formats. That's probably something to do in master only, because a significant change like that would best be introduced with a new major version. If you've got something else in mind, or can see a way to have even less bash/batch logic though, I'm all for it. I was indeed thinking about general improvements to SolrCLI, making the actual script extremely simple. I don't think we can eliminate the script, though if that were possible, it would be really cool. One pretty major shift (that also needs its own issue) I would like to make possible is the idea of Solr being two Java programs – one program that serves as a "control" process (with a very small max heap) that can start/stop/restart the actual application (Jetty for now). Imagine a situation where you can edit things like Solr's max heap in the UI and also restart Solr from the UI.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          I'm not specifically attached to the .conf extension, but your concerns make sense to me. It's a common enough format, we should just call it what it is. I'll happily update the patch to change it, unless Jan Høydahl had a particular reason for choosing that name. I suspect it was chosen to be format-agnostic back when the format itself was up in the air, but can be changed now that the format is decided. But I'll let him shed light on it if I'm wrong.

          Show
          gerlowskija Jason Gerlowski added a comment - I'm not specifically attached to the .conf extension, but your concerns make sense to me. It's a common enough format, we should just call it what it is. I'll happily update the patch to change it, unless Jan Høydahl had a particular reason for choosing that name. I suspect it was chosen to be format-agnostic back when the format itself was up in the air, but can be changed now that the format is decided. But I'll let him shed light on it if I'm wrong.
          Hide
          gerlowskija Jason Gerlowski added a comment -

          I've updated the config file names to solr.properties and solr-default.properties in the attached patch.

          This doesn't preclude a veto from Jan or anyone else that has a good reason for the .conf extension. If we end up sticking with .conf, we can just ignore the latest patch.

          Show
          gerlowskija Jason Gerlowski added a comment - I've updated the config file names to solr.properties and solr-default.properties in the attached patch. This doesn't preclude a veto from Jan or anyone else that has a good reason for the .conf extension. If we end up sticking with .conf, we can just ignore the latest patch.

            People

            • Assignee:
              janhoy Jan Høydahl
              Reporter:
              janhoy Jan Høydahl
            • Votes:
              3 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:

                Development