You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucene.apache.org by Erick Erickson <er...@gmail.com> on 2014/09/04 07:28:15 UTC
Some refactorings of MiniSolrCloudCluster and its test?
I have potential use for MiniSolrCloudCluster, and was just poking
around at the code and a couple of improvements came to mind,
wondering if I'm going off the deep end here.
1> It seems like the methods in TestMiniSolrCloudCluster
> uploadConfigToZk
> uploadConfigFileToZk
> createCollection
> waitForRecoveriesToFinish
should be moved to MiniSolrCloudCluster. It strikes me that these
methods would be generally useful for any test subclassing
MiniSolrCloudCluster. Which I intend to do I think.
2> There exist two rules:
@Rule
public TestRule solrTestRules = RuleChain
.outerRule(new SystemPropertiesRestoreRule());
@ClassRule
public static TestRule solrClassRules = RuleChain.outerRule(
new SystemPropertiesRestoreRule()).around(
new RevertDefaultThreadHandlerRule());
yet the shutdown() method explicitly clears a bunch of system
properties. I'm a little fuzzy on the ClassRule above, but is clearing
the system props really necessary in the shutdown method? And if I
move the methods to the MiniSolrCloudCluster that set the system
props, I assume that I should move both rules to the base class too.
And a special bonus for anyone who can give me a clue why both are
needed, it's late and I'm going to sleep on it before tracking it
down.
Just to be clear, if people think these changes are a good idea, I'll
take care of it as part of what I'm working on now.
Thanks!
Erick
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: Some refactorings of MiniSolrCloudCluster and its test?
Posted by Erick Erickson <er...@gmail.com>.
Steve:
I just created a JIRA for this here:
https://issues.apache.org/jira/browse/SOLR-6483
If I could trouble you to attach a patch with your changes I can see
about reconciling them with anything I do.
I really like the idea of not setting system props, that removes one
of the "interesting" bits about the rules.
On Thu, Sep 4, 2014 at 1:24 PM, Steve Davids <sd...@gmail.com> wrote:
> I have recently changed some of my base tests to utilize the
> MiniSolrCloudCluster and I ended up making a few tweaks from the
> TestMiniSolrCloudCluster:
>
> I used ZkController.uploadConfigDir(zkClient, configDir, coreName) instead
> of pulling in uploadConfigFileToZk & uploadConfigToZk, so perhaps just
> providing a uploadConfigDir method would be the best choice here.
> I did use createCollection but added an optional properties file that added
> each property from the given file to the collections admin request with the
> 'property.' prefix to each property key. I had some specific properties that
> need to be loaded for the core to load properly.
>
> The use of these properties allows us to ditch the System Properties being
> set, instead just use these property request parameters. ie set a request
> parameter of: property.solr.tests.maxBufferedDocs=100000
>
> Personally, I didn't use the waitForRecoveriesToFinish though I can see that
> being a useful option to provide. One issue is that this method performs
> fail assertions, it would be nice to switch that over to an exception so we
> don't necessarily need to force junit assertions in the MiniSolrCloudCluster
> this allows a little implementation flexibility.
> I provided a CloudSolrServer getSolrServer() method addition
>
> If you want, I can help out with some of these changes.
>
> -Steve
>
>
>
> On Thu, Sep 4, 2014 at 1:28 AM, Erick Erickson <er...@gmail.com>
> wrote:
>>
>> I have potential use for MiniSolrCloudCluster, and was just poking
>> around at the code and a couple of improvements came to mind,
>> wondering if I'm going off the deep end here.
>>
>> 1> It seems like the methods in TestMiniSolrCloudCluster
>> > uploadConfigToZk
>> > uploadConfigFileToZk
>> > createCollection
>> > waitForRecoveriesToFinish
>>
>> should be moved to MiniSolrCloudCluster. It strikes me that these
>> methods would be generally useful for any test subclassing
>> MiniSolrCloudCluster. Which I intend to do I think.
>>
>> 2> There exist two rules:
>> @Rule
>> public TestRule solrTestRules = RuleChain
>> .outerRule(new SystemPropertiesRestoreRule());
>>
>> @ClassRule
>> public static TestRule solrClassRules = RuleChain.outerRule(
>> new SystemPropertiesRestoreRule()).around(
>> new RevertDefaultThreadHandlerRule());
>>
>>
>> yet the shutdown() method explicitly clears a bunch of system
>> properties. I'm a little fuzzy on the ClassRule above, but is clearing
>> the system props really necessary in the shutdown method? And if I
>> move the methods to the MiniSolrCloudCluster that set the system
>> props, I assume that I should move both rules to the base class too.
>>
>> And a special bonus for anyone who can give me a clue why both are
>> needed, it's late and I'm going to sleep on it before tracking it
>> down.
>>
>> Just to be clear, if people think these changes are a good idea, I'll
>> take care of it as part of what I'm working on now.
>>
>> Thanks!
>> Erick
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
>> For additional commands, e-mail: dev-help@lucene.apache.org
>>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org
Re: Some refactorings of MiniSolrCloudCluster and its test?
Posted by Steve Davids <sd...@gmail.com>.
I have recently changed some of my base tests to utilize the
MiniSolrCloudCluster and I ended up making a few tweaks from the
TestMiniSolrCloudCluster:
- I used ZkController.uploadConfigDir(zkClient, configDir, coreName)
instead of pulling in uploadConfigFileToZk & uploadConfigToZk, so perhaps
just providing a uploadConfigDir method would be the best choice here.
- I did use createCollection but added an optional properties file that
added each property from the given file to the collections admin request
with the 'property.' prefix to each property key. I had some specific
properties that need to be loaded for the core to load properly.
- The use of these properties allows us to ditch the System
Properties being set, instead just use these property request parameters.
ie set a request parameter of: property.solr.tests.maxBufferedDocs=
100000
- Personally, I didn't use the waitForRecoveriesToFinish though I can
see that being a useful option to provide. One issue is that this method
performs fail assertions, it would be nice to switch that over to an
exception so we don't necessarily need to force junit assertions in the
MiniSolrCloudCluster this allows a little implementation flexibility.
- I provided a CloudSolrServer getSolrServer() method addition
If you want, I can help out with some of these changes.
-Steve
On Thu, Sep 4, 2014 at 1:28 AM, Erick Erickson <er...@gmail.com>
wrote:
> I have potential use for MiniSolrCloudCluster, and was just poking
> around at the code and a couple of improvements came to mind,
> wondering if I'm going off the deep end here.
>
> 1> It seems like the methods in TestMiniSolrCloudCluster
> > uploadConfigToZk
> > uploadConfigFileToZk
> > createCollection
> > waitForRecoveriesToFinish
>
> should be moved to MiniSolrCloudCluster. It strikes me that these
> methods would be generally useful for any test subclassing
> MiniSolrCloudCluster. Which I intend to do I think.
>
> 2> There exist two rules:
> @Rule
> public TestRule solrTestRules = RuleChain
> .outerRule(new SystemPropertiesRestoreRule());
>
> @ClassRule
> public static TestRule solrClassRules = RuleChain.outerRule(
> new SystemPropertiesRestoreRule()).around(
> new RevertDefaultThreadHandlerRule());
>
>
> yet the shutdown() method explicitly clears a bunch of system
> properties. I'm a little fuzzy on the ClassRule above, but is clearing
> the system props really necessary in the shutdown method? And if I
> move the methods to the MiniSolrCloudCluster that set the system
> props, I assume that I should move both rules to the base class too.
>
> And a special bonus for anyone who can give me a clue why both are
> needed, it's late and I'm going to sleep on it before tracking it
> down.
>
> Just to be clear, if people think these changes are a good idea, I'll
> take care of it as part of what I'm working on now.
>
> Thanks!
> Erick
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
> For additional commands, e-mail: dev-help@lucene.apache.org
>
>