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

Move user customization out of solr.in.* scripts

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 5.2.1
    • Fix Version/s: 6.3
    • Component/s: scripts and tools
    • Labels:
      None

      Description

      I've seen a fair number of users customizing solr.in.* scripts to make changes to their Solr installs. I think the documentation suggests this, though I haven't confirmed.

      One possible problem with this is that we might make changes in those scripts which such a user would want in their setup, but if they replace the script with the one in the new version, they will lose their customizations.

      I propose instead that we have the startup script look for and utilize a user customization script, in a similar manner to linux init scripts that look for /etc/default/packagename, but are able to function without it. I'm not entirely sure where the script should live or what it should be called. One idea is server/etc/userconfig.{sh,cmd} ... but I haven't put a lot of thought into it yet.

      If the internal behavior of our scripts is largely replaced by a small java app as detailed in SOLR-7043, then the same thing should apply there – have a config file for a user to specify settings, but work perfectly if that config file is absent.

        Issue Links

          Activity

          Hide
          ichattopadhyaya Ishan Chattopadhyaya added a comment -

          Shouldn't the deployment instructions suggest that the solr.in.sh be kept somewhere else, such that upgrades to newer Solr version doesn't overwrite it?
          I think the install_solr_server.sh already puts the solr.in.sh into $SOLR_VAR_DIR while other solr installation files into $SOLR_INSTALL_DIR. Does this mitigate this problem?

          Show
          ichattopadhyaya Ishan Chattopadhyaya added a comment - Shouldn't the deployment instructions suggest that the solr.in.sh be kept somewhere else, such that upgrades to newer Solr version doesn't overwrite it? I think the install_solr_server.sh already puts the solr.in.sh into $SOLR_VAR_DIR while other solr installation files into $SOLR_INSTALL_DIR. Does this mitigate this problem?
          Hide
          elyograg Shawn Heisey added a comment -

          I have not yet run the install script, or even looked at it. That's on my todo list – I will soon begin efforts to deploy 5.x into my dev environment.

          Show
          elyograg Shawn Heisey added a comment - I have not yet run the install script, or even looked at it. That's on my todo list – I will soon begin efforts to deploy 5.x into my dev environment.
          Hide
          thelabdude Timothy Potter added a comment -

          Would appreciate a good review of that service installer / init.d stuff Shawn Heisey ...

          In the meantime, yes, the installer does setup /var/solr/solr.in.sh which takes precedence over /opt/solr/bin/solr.in.sh (the one in the distro).

          As for upgrading, the approach I documented was to lay down a new Solr installation but keep your existing /var/solr directory as-is, including /var/solr/solr.in.sh. However, we should add mention about doing a diff with the new bin/solr.in.sh to see if any new goodies have been introduced to that file that you may need to include / override with your customized /var/solr/solr.in.sh when doing the upgrade.

          Show
          thelabdude Timothy Potter added a comment - Would appreciate a good review of that service installer / init.d stuff Shawn Heisey ... In the meantime, yes, the installer does setup /var/solr/solr.in.sh which takes precedence over /opt/solr/bin/solr.in.sh (the one in the distro). As for upgrading, the approach I documented was to lay down a new Solr installation but keep your existing /var/solr directory as-is, including /var/solr/solr.in.sh . However, we should add mention about doing a diff with the new bin/solr.in.sh to see if any new goodies have been introduced to that file that you may need to include / override with your customized /var/solr/solr.in.sh when doing the upgrade.
          Hide
          elyograg Shawn Heisey added a comment -

          If the user doesn't replace their solr.in.sh script, then they probably won't have an issue on upgrade ... but then they might be missing out on a bugfix or a performance-enhancing update.

          Now that I get a good look at solr.in.sh, the only things it does out of the box are set the max heap and the GC tuning options. All the other things in there are commented ... so now it makes a lot more sense as to why it would be set up the way it is. I think those default options should be in the main solr script, and it should be possible to override them in an additional script, with a sample version of that additional script provided in the download.

          Show
          elyograg Shawn Heisey added a comment - If the user doesn't replace their solr.in.sh script, then they probably won't have an issue on upgrade ... but then they might be missing out on a bugfix or a performance-enhancing update. Now that I get a good look at solr.in.sh, the only things it does out of the box are set the max heap and the GC tuning options. All the other things in there are commented ... so now it makes a lot more sense as to why it would be set up the way it is. I think those default options should be in the main solr script, and it should be possible to override them in an additional script, with a sample version of that additional script provided in the download.
          Hide
          elyograg Shawn Heisey added a comment -

          I've now used the install_solr_service script on my dev server. Overall, I think it does a good job, and some of what I initially thought was strange (in particular the solr.in.* script), makes much more sense now.

          Timothy Potter, I will collect my thoughts and try to present some coherent bikeshedding. I don't expect everyone to agree with my ideas, but I do firmly believe that having the conversation is helpful.

          Show
          elyograg Shawn Heisey added a comment - I've now used the install_solr_service script on my dev server. Overall, I think it does a good job, and some of what I initially thought was strange (in particular the solr.in.* script), makes much more sense now. Timothy Potter , I will collect my thoughts and try to present some coherent bikeshedding. I don't expect everyone to agree with my ideas, but I do firmly believe that having the conversation is helpful.
          Hide
          elyograg Shawn Heisey added a comment -

          I've had some time for my thoughts to gel on this subject.

          I think that we should move the bin/solr.in.* scripts to something like etc/startupenv.* instead, and place versions with "sample" in the filename in the download. I've thought about placing those sample files in server/etc instead, since that directory already exists. I'm open to discussion on both the filenames and the location. I like the idea of an etc directory at the top level ... if most of what a user might want to change in server/etc is configurable by environment variables in etc (I think we're probably there or mostly there now), we can hopefully keep beginners from messing with the jetty config and permanent scripts directly.

          The main solr script should contain sensible defaults for the variables that can be overridden in the startup environment, then read that config script if it exists.

          Show
          elyograg Shawn Heisey added a comment - I've had some time for my thoughts to gel on this subject. I think that we should move the bin/solr.in.* scripts to something like etc/startupenv.* instead, and place versions with "sample" in the filename in the download. I've thought about placing those sample files in server/etc instead, since that directory already exists. I'm open to discussion on both the filenames and the location. I like the idea of an etc directory at the top level ... if most of what a user might want to change in server/etc is configurable by environment variables in etc (I think we're probably there or mostly there now), we can hopefully keep beginners from messing with the jetty config and permanent scripts directly. The main solr script should contain sensible defaults for the variables that can be overridden in the startup environment, then read that config script if it exists.
          Hide
          janhoy Jan Høydahl added a comment -

          I think this discussion should merge with SOLR-7871 (Platform independent config file instead of solr.in.sh and solr.in.cmd)

          The steps of development could then be

          1. Move logic from bin/solr to SolrCLI.java (SOLR-7043)
          2. Support new platform independent start config (SOLR-7871) (while keeping support for current include scripts)
          3. Deprecate old solr.in. {sh|cmd}

            in next major release

          The new config file should ship as a template only as you suggest, and if not found there will be good defaults.

          Show
          janhoy Jan Høydahl added a comment - I think this discussion should merge with SOLR-7871 (Platform independent config file instead of solr.in.sh and solr.in.cmd) The steps of development could then be Move logic from bin/solr to SolrCLI.java ( SOLR-7043 ) Support new platform independent start config ( SOLR-7871 ) (while keeping support for current include scripts) Deprecate old solr.in. {sh|cmd} in next major release The new config file should ship as a template only as you suggest, and if not found there will be good defaults.
          Hide
          dbejean Dominique Béjean added a comment -

          Personally, my prefered solution is to customize the "solr.in.sh" script an put it in "/etc/default" or "/var/solr"
          The problem with this method is the location search order in "bin/solr" script.
          Today, it is :

          1. ./
          2. $HOME/.solr.in.sh
          3. /usr/share/solr
          4. /usr/local/share/solr
          5. /var/solr/
          6. /opt/solr

          In my opinion, "./" should be the last location

          Imagine you put "solr.in.sh" in "/etc/default"

          First issue : coherence
          If you use the "init.d/solr" script, the "/etc/default/solr/solr.in.sh" is used
          If you use the "bin/solr" script, the "bin/solr.in.sh" is used

          Second issue : upgrade
          In order to fix the first issue, "bin/solr.in.sh" have to be deleted
          If you upgrade Solr, a new "bin/solr.in.sh" exists and it is necessary to not forget to delete it !

          I suggest to update the "for include in ..." loop in "bin/solr" script in order to have

          1. $HOME/.solr.in.sh
          2. /usr/share/solr
          3. /usr/local/share/solr
          4. /var/solr/
          5. /opt/solr
          6. ./

          as location search order.

          Furthermore in "init.d/solr", the SOLR_ENV variable is not necessary anymore.

          Show
          dbejean Dominique Béjean added a comment - Personally, my prefered solution is to customize the "solr.in.sh" script an put it in "/etc/default" or "/var/solr" The problem with this method is the location search order in "bin/solr" script. Today, it is : ./ $HOME/.solr.in.sh /usr/share/solr /usr/local/share/solr /var/solr/ /opt/solr In my opinion, "./" should be the last location Imagine you put "solr.in.sh" in "/etc/default" First issue : coherence If you use the "init.d/solr" script, the "/etc/default/solr/solr.in.sh" is used If you use the "bin/solr" script, the "bin/solr.in.sh" is used Second issue : upgrade In order to fix the first issue, "bin/solr.in.sh" have to be deleted If you upgrade Solr, a new "bin/solr.in.sh" exists and it is necessary to not forget to delete it ! I suggest to update the "for include in ..." loop in "bin/solr" script in order to have $HOME/.solr.in.sh /usr/share/solr /usr/local/share/solr /var/solr/ /opt/solr ./ as location search order. Furthermore in "init.d/solr", the SOLR_ENV variable is not necessary anymore.
          Hide
          dsmiley David Smiley added a comment -

          This issue recently came up in the context of the official Solr docker image: https://github.com/docker-solr/docker-solr/issues/55#issuecomment-244907190

          The solr.in.sh script is mostly commented but there are exceptions, such as setting SOLR_HEAP. As-such, this will over-ride any existing environment variable definition for this, leading to a bad user experience. My preference would be for the default settings to be embedded within the bin/solr script as conditionals (i.e. only define these vars if they are not already defined). The bin/solr script already does this for a ton of stuff, so this approach is very consistent – a good thing. At that point, we can comment out everything that isn't already commented out in solr.in.sh.

          Perhaps the issue title could be renamed to "The default solr.in.sh shouldn't do anything; add more defaults to bin/solr" if we agree on this.

          Show
          dsmiley David Smiley added a comment - This issue recently came up in the context of the official Solr docker image: https://github.com/docker-solr/docker-solr/issues/55#issuecomment-244907190 The solr.in.sh script is mostly commented but there are exceptions, such as setting SOLR_HEAP. As-such, this will over-ride any existing environment variable definition for this, leading to a bad user experience. My preference would be for the default settings to be embedded within the bin/solr script as conditionals (i.e. only define these vars if they are not already defined). The bin/solr script already does this for a ton of stuff, so this approach is very consistent – a good thing. At that point, we can comment out everything that isn't already commented out in solr.in.sh. Perhaps the issue title could be renamed to "The default solr.in.sh shouldn't do anything; add more defaults to bin/solr" if we agree on this.
          Hide
          dsmiley David Smiley added a comment -

          Jan Høydahl, your plan in SOLR-7871 looks good too, and apparently supersedes this. But that issue isn't ready yet? If that won't make it to 6.3 then I could commit what I propose here soon. It will be very straight-forward.

          Show
          dsmiley David Smiley added a comment - Jan Høydahl , your plan in SOLR-7871 looks good too, and apparently supersedes this. But that issue isn't ready yet? If that won't make it to 6.3 then I could commit what I propose here soon. It will be very straight-forward.
          Hide
          thelabdude Timothy Potter added a comment -

          fwiw - my original thinking was that solr.in.sh provided settings that were typical for users to override and the ones that get defaults in bin/solr were less common to be overridden. I'm with this approach too though

          Show
          thelabdude Timothy Potter added a comment - fwiw - my original thinking was that solr.in.sh provided settings that were typical for users to override and the ones that get defaults in bin/solr were less common to be overridden. I'm with this approach too though
          Hide
          janhoy Jan Høydahl added a comment -

          I don't think SOLR-7871 will make it for 6.3 so please go ahead with the quick patch

          Show
          janhoy Jan Høydahl added a comment - I don't think SOLR-7871 will make it for 6.3 so please go ahead with the quick patch
          Hide
          elyograg Shawn Heisey added a comment -

          I like David's idea. The bin/solr script should work with all defaults if solr.in.sh is missing, or if it doesn't actually set any environment variables.

          I suspect that if the include script were deleted, Solr's behavior would change. I did not check bin/solr extensively, but I did notice that the garbage collection tuning would disappear.

          Show
          elyograg Shawn Heisey added a comment - I like David's idea. The bin/solr script should work with all defaults if solr.in.sh is missing, or if it doesn't actually set any environment variables. I suspect that if the include script were deleted, Solr's behavior would change. I did not check bin/solr extensively, but I did notice that the garbage collection tuning would disappear.
          Hide
          dsmiley David Smiley added a comment -

          Here's a patch:

          • SOLR_HEAP: it was already defaulted
          • GC_LOG_OPTS: moved the default to bin/solr
          • GC_TUNE: moved the default to bin/solr
          • ENABLE_REMOTE_JMX_OPTS: it was already defaulted
          • -Xss256k: moved to bin/solr as a new SOLR_STACK_SIZE var since we want it appended to SOLR_OPTS

          And then, a few minutes ago, it hits me there's the Windows side to worry about – argh! I'm looking forward to Jan's progress on SOLR-7871

          Unless someone wants to help me with the Windows side... perhaps I could reduce the scope to the most simplest case where it was already defaulted: SOLR_HEAP and ENABLE_REMOTE_JMX_OPTS ?

          Show
          dsmiley David Smiley added a comment - Here's a patch: SOLR_HEAP: it was already defaulted GC_LOG_OPTS: moved the default to bin/solr GC_TUNE: moved the default to bin/solr ENABLE_REMOTE_JMX_OPTS: it was already defaulted -Xss256k: moved to bin/solr as a new SOLR_STACK_SIZE var since we want it appended to SOLR_OPTS And then, a few minutes ago, it hits me there's the Windows side to worry about – argh! I'm looking forward to Jan's progress on SOLR-7871 Unless someone wants to help me with the Windows side... perhaps I could reduce the scope to the most simplest case where it was already defaulted: SOLR_HEAP and ENABLE_REMOTE_JMX_OPTS ?
          Hide
          dsmiley David Smiley added a comment -

          Oh and I did a little testing on my machine (a Mac) of setting the env vars in advance vs not vs setting to blank, and it worked. Also, BTW, the bin/solr script had yet to make use of the distinction between an env var that is blank vs undefined (unset). For a couple of these vars here, this was pertinent so I used a technique here: http://stackoverflow.com/a/13864829/92186

          Show
          dsmiley David Smiley added a comment - Oh and I did a little testing on my machine (a Mac) of setting the env vars in advance vs not vs setting to blank, and it worked. Also, BTW, the bin/solr script had yet to make use of the distinction between an env var that is blank vs undefined (unset). For a couple of these vars here, this was pertinent so I used a technique here: http://stackoverflow.com/a/13864829/92186
          Hide
          dsmiley David Smiley added a comment -

          Here's an updated patch that addresses the Windows side. I also did a little tweaking to make the declaration order a little more consistent between the Bash & Windows scripts. I did a little testing in Windows but I should do more.

          Jan Høydahl might you take a look please?

          Show
          dsmiley David Smiley added a comment - Here's an updated patch that addresses the Windows side. I also did a little tweaking to make the declaration order a little more consistent between the Bash & Windows scripts. I did a little testing in Windows but I should do more. Jan Høydahl might you take a look please?
          Hide
          janhoy Jan Høydahl added a comment -

          This is a nice improvement. Looks ok. Did not do actual testing.

          You could also remove this part from solr.cmd, like you did in the linux script:

          IF "!JAVA_MAJOR_VERSION!"=="7" (
              ...
          )
          
          Show
          janhoy Jan Høydahl added a comment - This is a nice improvement. Looks ok. Did not do actual testing. You could also remove this part from solr.cmd, like you did in the linux script: IF "!JAVA_MAJOR_VERSION!" == "7" ( ... )
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit fe77dff09406b0c848a269a6bfee490ea6c67015 in lucene-solr's branch refs/heads/master from David Smiley
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fe77dff ]

          SOLR-7850: Move defaults in bin/solr.in.sh into bin/solr (incl. Windows)

          Show
          jira-bot ASF subversion and git services added a comment - Commit fe77dff09406b0c848a269a6bfee490ea6c67015 in lucene-solr's branch refs/heads/master from David Smiley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fe77dff ] SOLR-7850 : Move defaults in bin/solr.in.sh into bin/solr (incl. Windows)
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit b4cb5763034662152fd49b24cb72a1562d4d472e in lucene-solr's branch refs/heads/branch_6x from David Smiley
          [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b4cb576 ]

          SOLR-7850: Move defaults in bin/solr.in.sh into bin/solr (incl. Windows)

          (cherry picked from commit fe77dff)

          Show
          jira-bot ASF subversion and git services added a comment - Commit b4cb5763034662152fd49b24cb72a1562d4d472e in lucene-solr's branch refs/heads/branch_6x from David Smiley [ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=b4cb576 ] SOLR-7850 : Move defaults in bin/solr.in.sh into bin/solr (incl. Windows) (cherry picked from commit fe77dff)
          Hide
          dsmiley David Smiley added a comment -

          I removed the Java 7 condition, and did a bit more manual testing on Windows & Unix. Then I committed... though accidentally messed up the JIRA reference in the commit message & CHANGES.txt: SOLR-7580 I fixed it.

          Thanks for the review Jan.

          Show
          dsmiley David Smiley added a comment - I removed the Java 7 condition, and did a bit more manual testing on Windows & Unix. Then I committed... though accidentally messed up the JIRA reference in the commit message & CHANGES.txt: SOLR-7580 I fixed it. Thanks for the review Jan.
          Show
          janhoy Jan Høydahl added a comment - Minor edit to refGuide https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=50856198&selectedPageVersions=51&selectedPageVersions=50
          Hide
          shalinmangar Shalin Shekhar Mangar added a comment -

          Closing after 6.3.0 release.

          Show
          shalinmangar Shalin Shekhar Mangar added a comment - Closing after 6.3.0 release.

            People

            • Assignee:
              dsmiley David Smiley
              Reporter:
              elyograg Shawn Heisey
            • Votes:
              2 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development