[
https://issues.apache.org/jira/browse/SOLR-646?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622393#action_12622393
]
Hoss Man commented on SOLR-646:
-------------------------------
I started digging into this patch today, making notes for myself as i go along,
and revising them as i understand more and more about the code. i still don't
feel like i fully grasp a lot of this patch, but i have to go offline pretty
soon, and i'm not sure i'll get a chance to dig into this again for a few days,
so i wanted to post my rough notes so far. (i've done some minimal cleanup to
make them wiki formatted, but they're still be in my convoluted stream of
consciousness rambling)
NOTE: the patch doesn't apply tothe trunk anymore (hey look at that,
MultiCore.java is gone) so these are based on applying the patch to r684307.
----
* are there any unit tests specificly for this new stuff? At a minimum the
existing test configs should excercies this functionality so we have some
confidence that it works.
* the changes to the example configs seem overly contrived:
** excessivly using includes
** property for type name in *both* the fieldtype and field (which
makes it irrelevant what the actual value is -- even if the property
isn't evaluated the literal string "${id_type:id_t}" is a valid
fieldtype name)
* we should reduce the usages in the example configs, and make them represent
slightly more practical usecases...
** <import> to include different postCommit fragements depending
on master/slave property.
** pick dataDir based on corename
** <import> to add *some* field declarations to schema (not all)
** property to override which fieldtype name a field should use (leave the
<fieldtype> declaration static)
* <fragement> should be used more consistently. Even if the file being
included already has a root node (and is wellformed) the <import> tag should
inforce that the imported file be a valid fragement file so we know the file
really was ment to be inlcuded.
* <import> seems like it should be named <include>
* I really don't understand ParamMap.imports(). for starters the javadocs seem
like cut/paste mistake. Beyond that: if I'm understanding the Jira comments,
the String[] represents a "pair" of bits of info about a resource the first
being the relative path as specified in the <import> tag, the second being the
directory it was found in (or maybe the absolute path??) ... so what then is
the key to the Map<String,String[]> ... what is the "symbolic property" ????
... going deeper down the rabit hole, why do we need new public methods in
SolrResourceLoader? what do they do that the existing methos didn't take care
of? (they have no javadocs)
* is the new "private InputStream open(String,String[])" method in
SolrResourceLoader suppose to be modifying the array that is passed to it?
(it's not a documented side effect) ... even after readingteh docs for this
method, and all 10 lines of code in the method, i'm still not sure i understand
what it's doing or why (what does "convenient to export" and "convenient to
import" mean?)
* Do we really need PropertyMap, Evaluator, and all of hte public methods they
have to be public? On the whole it seems like a lot of new plumbing is being
exposed here -- and we already have more exposed plumbing then we probably
should.
It seems like the only API most code should need to worry about should be
something like...
{code}
/**
* Given a DOM Document, and some properties identifing the current
* context, generates a new DOM Document when imports and property
* value substitutions performed.
* @param doc The Document to evaluate
* @param context existing variables that should be considered when
* processing doc
* @param loader for resolving filenames
*/
public static dom.Document evaluateProps(dom.Document doc,
Properties context,
SolrResourceLoader loader);
{code}
...and even that could probably be encapsulated and hidden inside of the
Config class and called as part of it's constructor (with a new Properties or
SolrParams constructor arg to specify the outer "context")
* there seems to be an awful lot of code scattered arround checking if "x !=
PropertyMap.SYSTEM_PROPERTIES" and doing something special in that case ...
whatever the concerns are about this, it should probably be handled by the impl
of PropertyMap.SYSTEM_PROPERTIES (even if that means it needs to be a special
subclass of PropertyMap) so we don't have to worry about new code being added
without remember to be as paranoid.
* this is really unclear, even with the coment ...
{code}
// if this is a fragment, isfrag becomes true (thus the '=', *not* '==')
if (isfrag = "fragment".equals(walk.getNodeName()))
{code}
... we can replace those two lines with these two lines and it's even
clearer...
{code}
isfrag = "fragment".equals(walk.getNodeName())
if (isfrag)
{code}
* Should we be worried about removing some public static methods from DOMUtil?
It would probably be better to just stop using them, and mark them deprecated
(no reason to risk breaking someone who decided to use them, even if the odds
are low that anyone is).
----
On the whole: there is more complexity here then I would expect for the goal we
are trying to accomplish, and it's not clear to me that it's necessary .... i
haven't had the "Ahhhh, i see now" moment that makes me understand why the new
code jumps through as many hoops as it does.
> Configuration properties in multicore.xml
> -----------------------------------------
>
> Key: SOLR-646
> URL: https://issues.apache.org/jira/browse/SOLR-646
> Project: Solr
> Issue Type: New Feature
> Affects Versions: 1.3
> Reporter: Henri Biestro
> Assignee: Ryan McKinley
> Fix For: 1.3
>
> Attachments: solr-646.patch, solr-646.patch, solr-646.patch,
> solr-646.patch
>
>
> This patch refers to 'generalized configuration properties' as specified by
> [HossMan|https://issues.apache.org/jira/browse/SOLR-350?focusedCommentId=12562834#action_12562834]
> This means configuration & schema files can use expression based on
> properties defined in multicore.xml.
> h3. Use cases:
> Share the same schema and/or config file between multiple cores but point to
> different dataDirs using a <dataDir>${core.datadir}</dataDir>
> Change the general layout between data, config & schema directories.
> Define 'installation' wide properties (for replication for instance) in
> multicore.properties (different base/install/data directories on different
> hosts).
> h3. Syntax:
> h4. Defining properties:
> Either in the multicore.properties file (usual property format):
> {code:xml}
> env.value=an installation value
> env.other_value=another installation value
> {code}
> In the multicore.xml:
> {code:xml}
> <multicore'>
> <property name='mp0'>a value</property> <!-- a basic property -->
> <property name='mp1'>${env.value}</property> }<!-- a property whose value
> is an expression -->
> <core name='core_name'>
> <property name='p0'>some value</property>
> <property name='p1'>${mp0}-and some data</property>
> </core>
> </multicore>
> {code}
> h4. Using properties:
> Besides used defined properties, the following core descriptor properties are
> automatically defined in each core context, namely:
> {code}
> solr.core.instanceDir
> solr.core.instancePath
> solr.core.name
> solr.core.configName
> solr.core.schemaName
> {code}
> All properties can be used in any attribute or text node of a solrconfig.xml
> or schema.xml as in:
> {code:xml}
> <dataDir>${core.dataDir}</dataDir>
> {code}
> h4. Technical specifications
> Multicore.xml can define properties at the multicore & each core level.
> Properties defined in the multicore scope can override system properties.
> Properties defined in a core scope can override multicore & system properties.
> Property definitions can use expressions to define their name & value; these
> expressions are evaluated in their outer scope context .
> Multicore serialization keeps properties as written (ie as expressions if
> they were defined so).
> The core descriptor properties are automatically defined in each core
> context, namely:
> solr.core.instanceDir
> solr.core.instancePath
> solr.core.name
> solr.core.configName
> solr.core.schemaName
> h4. Test:
> The following (contrived) multicore.xml:
> {code:xml}
> <multicore adminPath='/admin/multicore' persistent='true'>
> <property name='revision'>33</property> <!-- a basic property -->
> <property name='zero'>0</property> <!-- used to expand the core0 name -->
> <property name='one'>1</property> <!-- used to expand the core1 name -->
> <property name='id_type'>bogus</property> <!-- a bogus type that will be
> overriden -->
> <property name='updateHandler'>bogus</property> <!-- a bogus updateHandler
> that will be overriden -->
> <core name='core${zero}' instanceDir='core0/'> <!-- the name is expanded
> -->
> <property name='id_type'>core${zero}_id</property> <!-- so is a text node
> -->
> <property name='updateHandler'>solr.DirectUpdateHandler2</property> <!--
> a property can be overriden -->
> <property name='revision'>11</property>
> </core>
> <core name='core${one}' instanceDir='core1/'>
> <property name='id_type'>core${one}_id</property>
> <property name='updateHandler'>solr.DirectUpdateHandler2</property>
> <property name='revision'>22</property>
> </core>
> </multicore>
> {code}
> allows this config.xml:
> {code:xml}
> <config>
> <!-- use the defined update handler property -->
> <updateHandler class="${updateHandler}" />
> <requestDispatcher handleSelect="true" >
> <requestParsers enableRemoteStreaming="false"
> multipartUploadLimitInKB="2048" />
> </requestDispatcher>
>
> <requestHandler name="standard" class="solr.StandardRequestHandler"
> default="true" />
> <requestHandler name="/update" class="solr.XmlUpdateRequestHandler" />
> <requestHandler name="/admin/luke"
> class="org.apache.solr.handler.admin.LukeRequestHandler" />
>
> <!-- config for the admin interface -->
> <admin>
> <defaultQuery>solr</defaultQuery>
> <gettableFiles>solrconfig.xml schema.xml admin-extra.html</gettableFiles>
> <pingQuery>
> qt=standard&q=solrpingquery
> </pingQuery>
> </admin>
> </config>
> {code}
> and this schema.xml:
> {code:xml}
> <schema name="example core zero" version="1.1">
> <types>
> <!-- define a type name dynamically -->
> <fieldtype name="${id_type:id_t}" class="solr.StrField"
> sortMissingLast="true" omitNorms="true"/>
> <fieldtype name="string" class="solr.StrField" sortMissingLast="true"
> omitNorms="true"/>
> </types>
> <fields>
> <!-- the type of unique key defined above -->
> <field name="id" type="${id_type:id_t}" indexed="true"
> stored="true" multiValued="false" required="true"/>
> <field name="type" type="string" indexed="true" stored="true"
> multiValued="false" />
> <field name="name" type="string" indexed="true" stored="true"
> multiValued="false" />
> <field name="${solr.core.name:core}" type="string" indexed="true"
> stored="true" multiValued="false" />
> </fields>
> <uniqueKey>id</uniqueKey>
> <defaultSearchField>name</defaultSearchField>
> <solrQueryParser defaultOperator="OR"/>
> </schema>
> {code}
> Allow the trunk test to work.
> h3. Coding notes:
> The code itself refactored some of DOMUtil (the ant based property
> substitution) into one added class (PropertyMap & PropertyMap.Evaluator).
> The PropertyMap are chained (one link chain between core to multicore map);
> those maps are owned by each core's ResourceLoader.
> Config is modified a little to accommodate delaying & specializing property
> expansions.
> Multicore is modified so it properly parses & serializes.
> Tested against the example above.
> Reviews & comments more than welcome.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
