...Did i get that all right?
wow! you are good!
The one key point you did not mention is that the ThreadLocal approach
is proposed as a stop-gap measure until we have an API breaking release.
In the 2.0 release, the init() method signature will change and the
ThreadLocal hack will go away.
1) the ThreadLocal appraoch scares me ... it seems like it would be very
easy for us to screw it up down the road, when making changes to classes
such that we may not allways remember to "set" the core/config when we
should, and it would be very hard to spot the problem (particularly in
single threaded tests)
Currently the ThreadLocal core/config is set within the core/config
constructor and in TestHarness.
Also, "down the road" it will be gone.
2) As far as analysis factory "stuff" goes -- let's not give anyone
(including the IndexSchema) access to the SolrCore ... let's even
deprecate the IndexSchema's usage of the SolrConfig interface, and instead
refactor the classpath related stuff in SolrConfig/Config into a new
interface/utility class (for the sake of argument: ResourceFinder)
that just knows how to find config files and instantiate classes using a
classpath. after the IndexSchema has inited a factory (using the 1.2
sigs) it can use a marker interface like we've discussed before ie:
ResourceFinderInitable) to tell that factory about the ResourceFinder (if
it wants to go get a stopwords file, or instantiate another Tokenizer for
parsing a synonm file etc)
i) backwards compatible, just adds a new optional interface
ii) no crazy queing or callback loops - the ResourceFinder will be
completley initialized by teh SOlrConfig well before hte indexSchema
uses it.
iii) keeps IndexSchema inependent of SolrCore, which is nice; and may
someday help us promote the IndexSchema concept to a lucene contrib
We are not (nor have been) suggesting that analysis stuff gets the core
- it just needs to be able to load resources. Refactoring the Resource
loading out of Config seems like a good idea. Then we could pass that
to init() rather then SolrConfig.
I don't follow i) -- my complaint about the multiple init() interfaces
is that it just feels wierd to have:
void init( Map<String,String> args )
*and*
void init( ResourceFinder finder )
when what you really want is:
void init( ResourceFinder finder, Map<String,String> args )
The proposal in 414 is to say "In the future, we will change this
interface" for now, use:
void init( Map<String,String> args )
{
ResourceFinder finder = DeprecatedPluginUtils.getResourceFinder();
}
Looking at it again, the default init( args ) method saves the args for
later use anyway so it is not that big of deal wait till the second
(optional) init() method -- but it just feels wrong.
3) as far as non schema related things go (ie: request handlers, etc...)
and making them aware of the SolrCore .. what if we revist the marker
interface callback idea for a minute: the reason it got crazy and required
complex queing and callbacks was because the SolrCore isn't fully
initialized when the plugins are initialized -- the same problem as if we
added SolrCore to the init signature for the plugins; and because Henri
wanted to account for both a plugin triggering initialization of other
plugins that need to know aboutthe core, and forwanting to "delay" their
initialization until other plugins are done.
what if we just don't support any of that complexity?
make the semantics simple:
a) Foo.init( ...whatever current args are...) will be called exactly once.
b) if Foo implements SolrCoreInitalizable, then:
init(SolrCore) will be called exactly once, at some point after
the previouslu mentioned init method and before any work is
exepcted from this plugin.
c) there is no implied order that plugins will be initialized, or that
SolrCoreInitalizable.init will be called in -- the orders may not
be the same.
Yes, the simplified version is more palatable and I think covers all the
major use cases.
Again, I just like the clarity of:
void init( SolrCore, NamedList );
vs
void init( NamedList );
void init( SolrCore );
Re 1/2 initalized cores... most use cases seem to be ok without
worrrying about it. If there is a problem, perhaps we could have a
"CoreInitalized" event listener -- things that absolutely need to be
executed after the core is completed could queue up. (I don't see an
immediate need for this, but it is an option)
if we *really* want to support something "defered" solrCore
initializability, ...
I think we agreed it is not necessary.
That would all play icely with AbstractPluginLoader as well right?
Any solution will have to play nice with AbstractPluginLoader ;)
- - - -
To sum up, I see two directions:
1. Keep existing init() apis. Add more init() functions that pass in
SolrCore or a ResourceFinder.
2. Keep existing init() apis, but advertise they will change in the
future. As a stop-gap measure make the parameters that will be
available in the future available through ThreadLocal. (SOLR-414)
I vote for #2 because after the 2.0 release, we will have a clean API
with all init options available in a single place. I am ok with #1 also.
ryan
