[jira] [Commented] (SOLR-6943) HdfsDirectoryFactory should fall back to system props for most of it's config if it is not found in solrconfig.xml.

["Mike Drob (JIRA)" <ji...@apache.org>]

    [ 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: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org