[ 
https://issues.apache.org/jira/browse/SOLR-6943?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14274290#comment-14274290
 ] 

Mike Drob commented on SOLR-6943:
---------------------------------

My thoughts:

{code:title=HdfsDirectoryFactory.java}
+    Integer value = params.getInt(name, defaultValue);
{code}
When calling {{getConfig}}, for a boolean the precedence is param value, system 
value, passed default. For strings it is the same order. For ints, it looks 
like it is param value, passed default, and then system value. Should be 
consistent with the other two. The problem is on this line.

{code:title=HDFSTestUtil.java}
+      Timer timer = new Timer();
{code}
Probably outside of the scope of this issue, but using a Timer is sometimes 
unsafe. Since all Timer objects share a thread, delays or issues in one Timer 
execution can propogate to other executions (Java Concurrency In Practice, 
p123). We should consider replacing with a {{ScheduledThreadPoolExecutor}}. A 
follow-on issue is fine for this, I expcet the actual impact to be minimal.

{code:title=HdfsDirectoryFactoryTest.java}
+  public void testInitArgsOrSysPropConfig() throws Exception {
{code}
This test covers a lot of ground, it would be nice to see it broken down into 
several smaller tests - one for each scenario that you're trying to do, I 
think. Not sure if the testing framework is easily amenable to that, however.

{code}
+  public static class MockCoreDescriptor extends CoreDescriptor {
{code}
Does this enable something that EasyMock does not?

{code}
+++ solr/core/src/test/org/apache/solr/util/MockSolrResourceLoader.java 
(revision 0)
{code}
This class looks unused.

> HdfsDirectoryFactory should fall back to system props for most of it's config 
> if it is not found in solrconfig.xml.
> -------------------------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-6943
>                 URL: https://issues.apache.org/jira/browse/SOLR-6943
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Mark Miller
>            Assignee: Mark Miller
>             Fix For: 5.0, Trunk
>
>         Attachments: SOLR-6943.patch, SOLR-6943.patch
>
>
> The new server and config sets has undone the work I did to make hdfs easy 
> out of the box. Rather than count on config for that, we should just allow 
> most of this config to be specified at the sys property level. This improves 
> the global cache config situation as well.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to