You are viewing a plain text version of this content. The canonical link for it is here.
Posted to hdfs-issues@hadoop.apache.org by "Jakob Homan (JIRA)" <ji...@apache.org> on 2010/10/14 02:55:33 UTC
[jira] Created: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Provide builder for constructing instances of MiniDFSCluster
------------------------------------------------------------
Key: HDFS-1456
URL: https://issues.apache.org/jira/browse/HDFS-1456
Project: Hadoop HDFS
Issue Type: Improvement
Components: test
Affects Versions: 0.22.0
Reporter: Jakob Homan
Assignee: Jakob Homan
Fix For: 0.22.0
Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
{noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Homan updated HDFS-1456:
------------------------------
Attachment: HDFS-1456.patch
Patch creates a new Builder class for construction MiniDFSClusters. What before was:
{noformat} cluster = new MiniDFSCluster(0, conf, NUM_DATA_NODES, true, false, true, null,
null, null, null);{noformat}
can now be expressed as
{noformat} cluster = new MiniDFSCluster.Builder(conf)
.numDataNodes(NUM_DATA_NODES)
.manageNameDfsDirs(false).build();{noformat}
I've converted a few instances to the new Builder. If people like this idea, I'll convert the rest, mainly through automation to avoid human error, but I wanted an easy-to-read patch before one filled with auto-refactoring. We can deprecate the MiniDFSConstructors as well.
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Konstantin Boudnik (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921570#action_12921570 ]
Konstantin Boudnik commented on HDFS-1456:
------------------------------------------
At first I was about to suggest to name setters more traditionally but then I absolutely liked the
{noformat}
cluster = new MiniDFSCluster.Builder(config)
.manageDataDfsDirs(false)
.manageNameDfsDirs(false).build();
{noformat}
+1 on the patch.
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Homan updated HDFS-1456:
------------------------------
Attachment: HDFS-1456-2.patch
Here's a patch with a slightly refined builder and all the calls to the MiniDFSCluster constructors replaced with calls to the builder. Most of the refactoring was automatic since most of the calls to the MiniDFS follow a standard pattern. I noted that quite a large percentage of the calls weren't using the most efficient constructor available. This is a lot of code churn, but as they stands, the MiniDFSCluster constructors are so hideous, I think it's worth the change to get rid of them.
All the tests pass, although TestBlockRecovery, TestBlockTokenWithDFS and TestPipelines are all pretty flaky both on OSX and Ubuntu.
Test-patch:
{noformat} [exec] +1 overall.
[exec]
[exec] +1 @author. The patch does not contain any @author tags.
[exec]
[exec] +1 tests included. The patch appears to include 424 new or modified tests.
[exec]
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec]
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec]
[exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
[exec]
[exec] +1 release audit. The applied patch does not increase the total number of release audit warnings.
[exec]
[exec] +1 system tests framework. The patch passed system tests framework compile.
{noformat}
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456-2.patch, HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Homan updated HDFS-1456:
------------------------------
Attachment: HDFS-1456-for-1052-branch.patch
Patch merged with the 1052 branch to be committed there. Same as trunk, just some lines didn't merge cleanly.
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456-2.patch, HDFS-1456-for-1052-branch.patch, HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922312#action_12922312 ]
Hudson commented on HDFS-1456:
------------------------------
Integrated in Hadoop-Hdfs-trunk-Commit #411 (See [https://hudson.apache.org/hudson/job/Hadoop-Hdfs-trunk-Commit/411/])
HDFS-1456. Provide builder for constructing instances of MiniDFSCluster.
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456-2.patch, HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Jakob Homan (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jakob Homan resolved HDFS-1456.
-------------------------------
Resolution: Fixed
Hadoop Flags: [Reviewed]
The commented-out calls were in the original code and got converted during the autorefactor. To minimize change - heh - I left them in. Thanks for the review. I've committed this. Resolving as fixed.
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456-2.patch, HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Boris Shkolnik (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12922240#action_12922240 ]
Boris Shkolnik commented on HDFS-1456:
--------------------------------------
except for some leftover comments , like this (// cluster = new MiniDFSCluster.Builder(conf).numDataNodes(0).format(false))
it looks good.
I looked thru the patch at most of the substitutions , but since it so huge, I heavily rely on the fact that all the tests have passed.
+1
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456-2.patch, HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Konstantin Boudnik (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12921241#action_12921241 ]
Konstantin Boudnik commented on HDFS-1456:
------------------------------------------
I'm yet to look at the patch but idea is +1 without any doubts.
Speaking of faster running tests: if MiniDFSCluster can be fixed to be reusable by a set of consequent tests then we might be able to run these tests in same VM (think TestNG) and save on VM startup time for every test class.
Last time I've tried to do this (almost 2 years ago) I failed because tests were leaving a lot of garbage after them (including semi-dead MiniDFSCluster in some cases) that clean exit was impossible.
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HDFS-1456) Provide builder for constructing
instances of MiniDFSCluster
Posted by "Philip Zeyliger (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HDFS-1456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12920835#action_12920835 ]
Philip Zeyliger commented on HDFS-1456:
---------------------------------------
+1. I'm a big fan of this suggestion.
One thing that might be interesting would be to "cache" MiniDFSClusters. I have a theory (not tested) that many of the tests would work just fine using the same cluster over and over again. If we had some sort of default cached instance, the tests might pass faster.
> Provide builder for constructing instances of MiniDFSCluster
> ------------------------------------------------------------
>
> Key: HDFS-1456
> URL: https://issues.apache.org/jira/browse/HDFS-1456
> Project: Hadoop HDFS
> Issue Type: Improvement
> Components: test
> Affects Versions: 0.22.0
> Reporter: Jakob Homan
> Assignee: Jakob Homan
> Fix For: 0.22.0
>
> Attachments: HDFS-1456.patch
>
>
> Time to fix a broken window. Of the 293 occurences of "new MiniDFSCluster("... most look something like:
> {noformat}cluster = new MiniDFSCluster(0, config, numDatanodes, true, false, true, null, null, null, null);{noformat}
> The largest constructor takes 10 parameters, and even the overloaded constructors can be difficult to read as their mainaly nulls or booleans.
> We should provide a Builder for constructing MiniDFSClusters to improve readability.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.