Solr
  1. Solr
  2. SOLR-716

Support properties in configuration files

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.3
    • Component/s: None
    • Labels:
      None

      Description

      Initially suggested by Hoss at https://issues.apache.org/jira/browse/SOLR-350?focusedCommentId=12562834#action_12562834 and taken forward by Henri in SOLR-646

      1. Allows users to define global as well as core-specific properties in solr.xml which can be used in solrconfig.xml and schema.xml
        <solr persistent="false">
        
          <property name="var" value="value" />
        
          <cores adminPath="/admin/cores">
            <core name="core0" instanceDir="core0">
              <property name="var" value="value" />
            </core>
            <core name="core1" instanceDir="core1" />
          </cores>
        </solr>
        
      2. The following core-specific properties will be added automatically:
        • solr.core.instanceDir
        • solr.core.name
        • solr.core.configName
        • solr.core.schemaName
      3. The variable substitution will be done in this fall-back order – core-specific, implicit, global, system properties.
      4. The properties defined in solr.xml should also be persisted back as is (without evaluation).
      1. solr-716.patch
        43 kB
        Henri Biestro
      2. SOLR-716.patch
        32 kB
        Shalin Shekhar Mangar
      3. solr-716.patch
        36 kB
        Henri Biestro
      4. SOLR-716.patch
        31 kB
        Shalin Shekhar Mangar
      5. SOLR-716.patch
        32 kB
        Shalin Shekhar Mangar

        Issue Links

          Activity

          Uwe Schindler made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Shalin Shekhar Mangar made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Resolved [ 5 ]
          Hide
          Shalin Shekhar Mangar added a comment -

          Committed revision 688359.

          Thank you Henri for your support and flexibility! Let us work towards all required enhancements in the next release.

          Thanks to everybody who helped out in the related issue.

          Show
          Shalin Shekhar Mangar added a comment - Committed revision 688359. Thank you Henri for your support and flexibility! Let us work towards all required enhancements in the next release. Thanks to everybody who helped out in the related issue.
          Henri Biestro made changes -
          Attachment solr-716.patch [ 12388798 ]
          Hide
          Henri Biestro added a comment -

          Might be post-commit but this will give a base to solr-646.
          Fixed all issues raised by Shalin in previous version.
          Added the persistence verification test. (thus the size bloat).

          Show
          Henri Biestro added a comment - Might be post-commit but this will give a base to solr-646. Fixed all issues raised by Shalin in previous version. Added the persistence verification test. (thus the size bloat).
          Shalin Shekhar Mangar made changes -
          Attachment SOLR-716.patch [ 12388797 ]
          Hide
          Shalin Shekhar Mangar added a comment -

          No changes in functionality. A bit of javadocs added.

          I shall commit this shortly.

          Show
          Shalin Shekhar Mangar added a comment - No changes in functionality. A bit of javadocs added. I shall commit this shortly.
          Hide
          Shalin Shekhar Mangar added a comment -

          So I can define properties in a core scope based on properties defined in the core-container scope. Seems like a natural & expected feature.

          I've been thinking more on this. The core scope already inherits all properties defined in container scope. You can also override them. The only thing which is not possible is having expressions in the attributes of the <core> node such as name, instanceDir etc. Is that really necessary? If you enable expressions in a file whose information can be modified at run-time then it becomes a bit more complicated to persist back. You'd need to track both expressions and keep their evaluated values separately instead of just one object.

          I feel that this discussion should not hold up the 1.3 release any more. Let us go ahead with the patch I posted and handle enhancements as part of SOLR-646 with 1.3.1 or 1.4 which ever comes first.

          Show
          Shalin Shekhar Mangar added a comment - So I can define properties in a core scope based on properties defined in the core-container scope. Seems like a natural & expected feature. I've been thinking more on this. The core scope already inherits all properties defined in container scope. You can also override them. The only thing which is not possible is having expressions in the attributes of the <core> node such as name, instanceDir etc. Is that really necessary? If you enable expressions in a file whose information can be modified at run-time then it becomes a bit more complicated to persist back. You'd need to track both expressions and keep their evaluated values separately instead of just one object. I feel that this discussion should not hold up the 1.3 release any more. Let us go ahead with the patch I posted and handle enhancements as part of SOLR-646 with 1.3.1 or 1.4 which ever comes first.
          Hide
          Shalin Shekhar Mangar added a comment -

          Oh no, no problem here. Interfere is the wrong word – more eyes and hands can only help

          I only wanted to understand how it helps since I haven't used multiple cores a lot.

          The patch looks good, some comments:

          1. Why is the public constructor of CoreContainer made package private?
          2. Can we remove the properties parameter in CoreContainer#evalExpressions. It is confusing to call this method with two properties instances.
          3. Note that the way you create coreProperties in CoreContainer#create(SolrCore) method, the implicit properties are set after SolrConfig is created. Unless I'm misunderstanding something, you will not be able to use the implicit/automatic properties in your solrconfig.xml.
          4. Can you please help me understand what this bit of code in CoreContainer#persist is doing?
            // restore the core expr name if it was one and has not changed
                String cname = StrUtils.join(aliases, ',');
                Map<String,String> pcore = dcore.getCoreExpressions();
                if (pcore != null) {
                  String namex = pcore.get(null);
                  if (namex != null) {
                    String namev = DOMUtil.substituteProperty(namex, loader.getProperties());
                    if (namev != null && namev.equals(cname))
                      cname = namex;
                  }
                }
            
          Show
          Shalin Shekhar Mangar added a comment - Oh no, no problem here. Interfere is the wrong word – more eyes and hands can only help I only wanted to understand how it helps since I haven't used multiple cores a lot. The patch looks good, some comments: Why is the public constructor of CoreContainer made package private? Can we remove the properties parameter in CoreContainer#evalExpressions. It is confusing to call this method with two properties instances. Note that the way you create coreProperties in CoreContainer#create(SolrCore) method, the implicit properties are set after SolrConfig is created. Unless I'm misunderstanding something, you will not be able to use the implicit/automatic properties in your solrconfig.xml. Can you please help me understand what this bit of code in CoreContainer#persist is doing? // restore the core expr name if it was one and has not changed String cname = StrUtils.join(aliases, ','); Map< String , String > pcore = dcore.getCoreExpressions(); if (pcore != null ) { String namex = pcore.get( null ); if (namex != null ) { String namev = DOMUtil.substituteProperty(namex, loader.getProperties()); if (namev != null && namev.equals(cname)) cname = namex; } }
          Hide
          Henri Biestro added a comment -

          So I can define properties in a core scope based on properties defined in the core-container scope.
          Seems like a natural & expected feature.
          Most of the code to allow idem-potent persistence allows it almost for free.
          I did not expect that getting back all the solr-646 feature set (beside include) with very little modifications to your code would re-create a problem.
          Sorry, I wont interfere again.

          Show
          Henri Biestro added a comment - So I can define properties in a core scope based on properties defined in the core-container scope. Seems like a natural & expected feature. Most of the code to allow idem-potent persistence allows it almost for free. I did not expect that getting back all the solr-646 feature set (beside include) with very little modifications to your code would re-create a problem. Sorry, I wont interfere again.
          Hide
          Shalin Shekhar Mangar added a comment -

          Henri – I'd like some help in understanding how having expressions in solr.xml itself help make multi cores easier to use. Can we see a use-case?

          Show
          Shalin Shekhar Mangar added a comment - Henri – I'd like some help in understanding how having expressions in solr.xml itself help make multi cores easier to use. Can we see a use-case?
          Henri Biestro made changes -
          Attachment solr-716.patch [ 12388790 ]
          Hide
          Henri Biestro added a comment - - edited

          property expressions can be used in solr.xml & are preserved during solr.xml persistence (solr-716.patch).

          If you were though, you'd need to keep the property expression map & the property value map separated

          CoreDescriptor members can be property expressions - thus a map of expressions in both CoreDescriptor & CoreContainer to keep them around so we can reevaluate & persist them.
          .
          substituteProperties is made explicit so we can choose when to run it (instead of a ctor modification) and the substituteProperties() method would have been necessary anyway in that case.

          Show
          Henri Biestro added a comment - - edited property expressions can be used in solr.xml & are preserved during solr.xml persistence (solr-716.patch). If you were though, you'd need to keep the property expression map & the property value map separated CoreDescriptor members can be property expressions - thus a map of expressions in both CoreDescriptor & CoreContainer to keep them around so we can reevaluate & persist them. . substituteProperties is made explicit so we can choose when to run it (instead of a ctor modification) and the substituteProperties() method would have been necessary anyway in that case.
          Hide
          Noble Paul added a comment - - edited

          I guess what is already there meets most of the requirements . May be more enhancements can come in Solr1.4.

          Show
          Noble Paul added a comment - - edited I guess what is already there meets most of the requirements . May be more enhancements can come in Solr1.4.
          Hide
          Shalin Shekhar Mangar added a comment - - edited

          You could do a second evaluation pass to expand the property values (if they are property expressions) created for the CoreContainer and for each CoreDescriptor.

          We can do that but that will require modifying the Config constructor to pass in a boolean parameter "substituteProperties". If set to false, Config should not try to substitute properties on the given node. When creating the Config object for solr.xml itself, we don't know the properties because we haven't got that point yet so we must suppress substitution. Later when we have read the global properties and we start reading the "core" node, we can substitute global+system properties on the "core" sub-tree. We can do that but I'm not sure if that's necessary. Is it very useful?

          If you were though, you'd need to keep the property expression map & the property value map separated.

          I didn't understand what you meant. Can you elaborate?

          Show
          Shalin Shekhar Mangar added a comment - - edited You could do a second evaluation pass to expand the property values (if they are property expressions) created for the CoreContainer and for each CoreDescriptor. We can do that but that will require modifying the Config constructor to pass in a boolean parameter "substituteProperties". If set to false, Config should not try to substitute properties on the given node. When creating the Config object for solr.xml itself, we don't know the properties because we haven't got that point yet so we must suppress substitution. Later when we have read the global properties and we start reading the "core" node, we can substitute global+system properties on the "core" sub-tree. We can do that but I'm not sure if that's necessary. Is it very useful? If you were though, you'd need to keep the property expression map & the property value map separated. I didn't understand what you meant. Can you elaborate?
          Hide
          Noble Paul added a comment -

          It would seem preferable & possible to only store the properties (versus the CoreDescriptor) in the SolrResourceLoader; this is the only piece needed for evaluation.

          CoreDescriptor can be easily obtained from SolrCore anyway and SolrCore is available to every single class in Solr . SolrResourceLoader is our own class and it does not expose the object (it is private and there is no getter). This enables the resourceloader to get access to any other property from CodeDescriptor (in future) if needed .

          users cannot specify a variable inside solr.xml

          Is it very useful ? considering that it can only substitute system properties?

          Show
          Noble Paul added a comment - It would seem preferable & possible to only store the properties (versus the CoreDescriptor) in the SolrResourceLoader; this is the only piece needed for evaluation. CoreDescriptor can be easily obtained from SolrCore anyway and SolrCore is available to every single class in Solr . SolrResourceLoader is our own class and it does not expose the object (it is private and there is no getter). This enables the resourceloader to get access to any other property from CodeDescriptor (in future) if needed . users cannot specify a variable inside solr.xml Is it very useful ? considering that it can only substitute system properties?
          Shalin Shekhar Mangar made changes -
          Attachment SOLR-716.patch [ 12388789 ]
          Hide
          Shalin Shekhar Mangar added a comment -

          Changes

          1. Per Henri's comment, SolrResourceLoader now keeps a reference to the Properties instance instead of keeping a reference to CoreDescriptor
          Show
          Shalin Shekhar Mangar added a comment - Changes Per Henri's comment, SolrResourceLoader now keeps a reference to the Properties instance instead of keeping a reference to CoreDescriptor
          Hide
          Henri Biestro added a comment - - edited

          SolrResourceLoader keeps a reference to the CoreDescriptor

          It would seem preferable & possible to only store the properties (versus the CoreDescriptor) in the SolrResourceLoader; this is the only piece needed for evaluation.
          IMHO, the CoreDescriptor giving potential access to everything (through the CoreContainer), it should not be shared/stored outside of the SolrCore without careful impact consideration.

          users cannot specify a variable inside solr.xml

          You could do a second evaluation pass to expand the property values (if they are property expressions) created for the CoreContainer and for each CoreDescriptor.
          If you were though, you'd need to keep the property expression map & the property value map separated.

          Show
          Henri Biestro added a comment - - edited SolrResourceLoader keeps a reference to the CoreDescriptor It would seem preferable & possible to only store the properties (versus the CoreDescriptor) in the SolrResourceLoader; this is the only piece needed for evaluation. IMHO, the CoreDescriptor giving potential access to everything (through the CoreContainer), it should not be shared/stored outside of the SolrCore without careful impact consideration. users cannot specify a variable inside solr.xml You could do a second evaluation pass to expand the property values (if they are property expressions) created for the CoreContainer and for each CoreDescriptor. If you were though, you'd need to keep the property expression map & the property value map separated.
          Shalin Shekhar Mangar made changes -
          Attachment SOLR-716.patch [ 12388777 ]
          Hide
          Shalin Shekhar Mangar added a comment -

          Changes

          1. SolrResourceLoader keeps a reference to the CoreDescriptor to get access to the core specific properties without breaking API compatibility.
          2. CoreContainer and CoreDescriptor keep a Properties instance each for global and core-specific properties respectively
          3. DOMUtil#substituteSystemProperties has been refactored (without breaking compatibility) to another method which accepts a Properties instance. This instance is checked for a value failing which, System#getProperty is used.
          4. TestSolrProperties is a test for this borrowed from Henri's patch in SOLR-646. It is actually in SolrJ test sources rather than main sources for ease of testing.

          The only thing not supported is that users cannot specify a variable inside solr.xml itself. Variables can only be used in the solrconfig.xml and schema.xml files (actually they can be used in any instance of Config class).

          Reviews and comments are invited.

          Show
          Shalin Shekhar Mangar added a comment - Changes SolrResourceLoader keeps a reference to the CoreDescriptor to get access to the core specific properties without breaking API compatibility. CoreContainer and CoreDescriptor keep a Properties instance each for global and core-specific properties respectively DOMUtil#substituteSystemProperties has been refactored (without breaking compatibility) to another method which accepts a Properties instance. This instance is checked for a value failing which, System#getProperty is used. TestSolrProperties is a test for this borrowed from Henri's patch in SOLR-646 . It is actually in SolrJ test sources rather than main sources for ease of testing. The only thing not supported is that users cannot specify a variable inside solr.xml itself. Variables can only be used in the solrconfig.xml and schema.xml files (actually they can be used in any instance of Config class). Reviews and comments are invited.
          Shalin Shekhar Mangar made changes -
          Field Original Value New Value
          Link This issue is related to SOLR-646 [ SOLR-646 ]
          Shalin Shekhar Mangar created issue -

            People

            • Assignee:
              Shalin Shekhar Mangar
              Reporter:
              Shalin Shekhar Mangar
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development