[
https://issues.apache.org/jira/browse/SOLR-6195?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14042792#comment-14042792
]
Hoss Man commented on SOLR-6195:
--------------------------------
bq. From the point of view of the code I think this still belongs in
UpdateHandlerInfo ..
Why?
Prior to adding this option _nothing_ in UpdateHandlerInfo had anything to do
with the IndexWriter or merges -- and yet there are several indexwriter /
mergering related settings that already exist in SolrIndexConfig.
If your point is simply that at the moment the UpdateHandler already fetches
the UpdateHandlerInfo from the SolrConfig and you think that's cleaner then
having it directly ask for the SolrIndexConfig -- well ... i disagree, but not
enough to argue over it that strongly. I think the setting conceptually makes
more sense in indexConfig, and i think it's important to keep these config
objects so that they closely model the config concepts -- which means the
canonical place to "store" this setting should be in SolrIndexConfig. But i've
got no problem adding a convenience method to UpdateHandlerInfo that proxies
out the SolrIndexConfig (which SolrConfig can give UpdateHandlerInfo a
reference to on construction).
hows that sound?
bq. As far as tests go, this is kind of tricky to test properly...
It's one thing to say it's really hard to test that the option does the right
thing on shutdown -- my concern is that, as a config option, we don't even have
a test that it's parsed correctly and you don't get an NPE or something like
that if you try to turn it on.
I'd be happy with just that - but off the top of my head if you wanted to be
sure it worked properly i bet we could mock out a MergePolicy that does nothing
untill it sees a semaphore that's triggered by a static metod. index a doc,
commit, index another doc, commit, do an optimize in a background thread, then
call close on the core in a thread with a timeout, etc...
> replace updateHandler/indexWriter/closeWaitsForMerges with
> indexConfig/closeWriterWaitsForMerges
> ------------------------------------------------------------------------------------------------
>
> Key: SOLR-6195
> URL: https://issues.apache.org/jira/browse/SOLR-6195
> Project: Solr
> Issue Type: Improvement
> Reporter: Hoss Man
>
> SOLR-6125 added a new "closeWaitsForMerges" config option, but in my opinion
> this was done too hastily w/o enough consideration to how/shere the option is
> configured.
> * it's currently "updateHandler/indexWriter/closeWaitsForMerges"
> ** there has never been an "updateHandler/indexWriter" section in
> solrconfig.xml until now
> ** this is the only setting that currently exists in this section
> * this setting, although used by DirectUpdateHandler2, does not (from a user
> perspective really fit with the other existing settings in {{<updateHandler>}}
> ** i think from a user perspective, it would make much more sense in
> {{<indexConfig>}} along side the other merge related settings.
--
This message was sent by Atlassian JIRA
(v6.2#6252)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
