[
https://issues.apache.org/jira/browse/SOLR-7789?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14711244#comment-14711244
]
Mark Miller commented on SOLR-7789:
-----------------------------------
This all looks pretty nice and well tested to me.
RE the API for accessing the queue, I first leaned towards Shalin's thinking,
but Greg has sold me. What queue you get is really an implementation detail. As
long as this is commented nicely somewhere, I think it makes sense.
General Notes:
I think having separate end points for /collections and /configs and separating
the code the best we can as well, rather than dumping it all in one class,
makes sense to me.
Sometimes we use ConfigSet and sometimes ConfigSets? Do we have the right
distinction? Can we call it out explicitly somewhere?
More comments giving overview and rationale of class breakup? As Shalin noted,
the class hierarchy is a bit denser to get a handle on.
Not the biggest fan of adding more tests with easymock - this stuff is such a
pain to ramp up on for most committers when refactoring or making changes. I
guess what can you do though. Many committers have expressed dislike with
having to deal with the collections API test like this, but it’s hard to argue
around once someone presents working, useful tests. Some deeper things are just
difficult to build simple mocks for.
*OverseerCollectionConfigSetProcessor*
* Typo - * 1) collection-related Overseer messagess
* Should we just pass the zkclient and simplify this constructor a bit? These
boilerplate type class have such long constructors.
Overseer.getCollectionQueue(zkStateReader.getZkClient(), stats),
Overseer.getRunningMap(zkStateReader.getZkClient()),å
Overseer.getCompletedMap(zkStateReader.getZkClient()),
Overseer.getFailureMap(zkStateReader.getZkClient())
*Tests*
I looked at a bit at what new tests I could easily overwhelm.
*TestConfigSetsAPIExclusivity*
* Beasting Test fails:
{noformat}
[junit4] 2> NOTE: reproduce with: ant test
-Dtestcase=TestConfigSetsAPIExclusivity -Dtests.method=testAPIExclusivity
-Dtests.seed=586D6E5E92126B0D -Dtests.slow=true -Dtests.locale=ar_LB
-Dtests.timezone=America/Nassau -Dtests.asserts=true -Dtests.file.encoding=UTF-8
[junit4] FAILURE 190s | TestConfigSetsAPIExclusivity.testAPIExclusivity <<<
[junit4] > Throwable #1: java.lang.AssertionError: Unexpected exception:
org.apache.solr.client.solrj.impl.HttpSolrClient$RemoteSolrException: Error
from server at http://127.0.0.1:50330/solr: create the configset time out:180s
expected:<0> but was:<1>
[junit4] > at
__randomizedtesting.SeedInfo.seed([586D6E5E92126B0D:251877A5CBB6D244]:0)
[junit4] > at
org.apache.solr.cloud.TestConfigSetsAPIExclusivity.testAPIExclusivity(TestConfigSetsAPIExclusivity.java:95)
[junit4] > at java.lang.Thread.run(Thread.java:745)
[junit4] 2> 189719 INFO
(SUITE-TestConfigSetsAPIExclusivity-seed#[586D6E5E92126B0D]-worker) [ ]
o.a.s.SolrTestCaseJ4 ###deleteCore
{noformat}
{noformat}
[junit4] 2> NOTE: reproduce with: ant test
-Dtestcase=TestConfigSetsAPIExclusivity -Dtests.method=testAPIExclusivity
-Dtests.seed=A74936D70D0532AD -Dtests.slow=true -Dtests.locale=ar_MA
-Dtests.timezone=America/Resolute -Dtests.asserts=true
-Dtests.file.encoding=UTF-8
[junit4] FAILURE 188s | TestConfigSetsAPIExclusivity.testAPIExclusivity <<<
[junit4] > Throwable #1: java.lang.AssertionError: Unexpected exception:
org.apache.solr.client.solrj.impl.HttpSolrClient$RemoteSolrException: Error
from server at http://127.0.0.1:40187/solr: delete the configset time out:180s
expected:<0> but was:<1>
[junit4] > at
__randomizedtesting.SeedInfo.seed([A74936D70D0532AD:DA3C2F2C54A18BE4]:0)
[junit4] > at
org.apache.solr.cloud.TestConfigSetsAPIExclusivity.testAPIExclusivity(TestConfigSetsAPIExclusivity.java:95)
[junit4] > at java.lang.Thread.run(Thread.java:745)
{noformat}
Everything else looks okay to me.
> Introduce a ConfigSet management API
> ------------------------------------
>
> Key: SOLR-7789
> URL: https://issues.apache.org/jira/browse/SOLR-7789
> Project: Solr
> Issue Type: New Feature
> Reporter: Gregory Chanan
> Assignee: Gregory Chanan
> Attachments: SOLR-7789.patch, SOLR-7789.patch, SOLR-7789.patch,
> SOLR-7789.patch
>
>
> SOLR-5955 describes a feature to automatically create a ConfigSet, based on
> another one, from a collection API call (i.e. one step collection creation).
> Discussion there yielded SOLR-7742, Immutable ConfigSet support. To close
> the loop, we need support for a ConfigSet management API.
> The simplest ConfigSet API could have one operation:
> create a new config set, based on an existing one, possible modifying the
> ConfigSet properties. Note you need to be able to modify the ConfigSet
> properties at creation time because otherwise Immutable could not be changed.
> Another logical operation to support is ConfigSet deletion; that may be more
> complicated to implement than creation because you need to handle the case
> where a collection is already using the configuration.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
