You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by "Johan Oskarsson (JIRA)" <ji...@apache.org> on 2008/08/12 12:04:44 UTC
[jira] Created: (HADOOP-3935) Extract classes from DataNode.java
Extract classes from DataNode.java
----------------------------------
Key: HADOOP-3935
URL: https://issues.apache.org/jira/browse/HADOOP-3935
Project: Hadoop Core
Issue Type: Improvement
Components: dfs
Reporter: Johan Oskarsson
Assignee: Johan Oskarsson
Priority: Trivial
DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622925#action_12622925 ]
Raghu Angadi commented on HADOOP-3935:
--------------------------------------
Even if there is a +1, normal procedure is to commit only after a patch goes through Hudson. Hudson does quite a bit more than running unit tests.
Even if we know some tests will fail, we still run it through Hudson. And before committing we just describe why a particular failure is ok and expected.
Also, traditionally for Hadoop, the commit message followed Doug's format : "I just committed this. [if committer and developer is different] thanks Johan!".
Just FYI.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Fix For: 0.19.0
>
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12621859#action_12621859 ]
Raghu Angadi commented on HADOOP-3935:
--------------------------------------
BlockReceiver and PacketResponder implement parts of the same operation/protocol. Should they be in the same file?
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622638#action_12622638 ]
Tsz Wo (Nicholas), SZE commented on HADOOP-3935:
------------------------------------------------
Hi Johan, for a committing a patch, we need to follow the guidelines in http://wiki.apache.org/hadoop/HowToContribute . In particular, we should use Submit Patch link like other contributors, and then wait for a "+1" from another committer before committing, as stated in the "Contributing your work" session.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Fix For: 0.19.0
>
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Johan Oskarsson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622944#action_12622944 ]
Johan Oskarsson commented on HADOOP-3935:
-----------------------------------------
Ok, I'll follow these guidelines next time. Thanks for the info.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Fix For: 0.19.0
>
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622318#action_12622318 ]
Raghu Angadi commented on HADOOP-3935:
--------------------------------------
looks good. Looks like it the case, but I will mention anyway : since we can not verify the blocks of code transferred, please make sure there are as few changes as possible a part from cut-n-paste.
Regd DataXceiverServer, it is so integrated with DataXceiver that it could just be a static class 'DataXceiver.Server' or 'DataXceiver.Acceptor'. But this is not required.
On the similar lines, DFSClient.java is also ripe for such a split. In a seperate jira, we should add hdfs/client directory and and split DFSClient there.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622401#action_12622401 ]
Konstantin Shvachko commented on HADOOP-3935:
---------------------------------------------
Johan, thanks for doing this. Patch looks much better now. A few more things.
- Throttler is still public and we should make it package private.
I just removed public from everywhere in the Throttler class, and moved TestBlockReplacement into package
org.apache.hadoop.hdfs.server.datanode and everything compiled alright. I think this test belongs to the datanode package anyway.
- Why I want to rename Throttler to something less generic? Because if you read it together with the package name it says
"hadoop datanode throttler". Which by intention it is not. It throttles block transfers for the block scanner and for the balancer
but not the data-node. Another consideration is that we have many other throttling mechanisms in different places and may have
more in the future so we will have to distinguish them.
My first proposition was not good. How about BlockTransferThrottler or please feel free to use your own variant.
- I also noticed that DataBlockScanner can be made package private if we move TestInterDatanodeProtocol into the same package as above.
This is optional for this patch because it is not directly related to splitting data-node, we can do it separately.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Johan Oskarsson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johan Oskarsson updated HADOOP-3935:
------------------------------------
Attachment: HADOOP-3935.patch
This is an initial patch for review. Comments and suggestions are very welcome. I have noticed the hadoop codebase is quite keen on inner classes so perhaps this goes against some unwritten code guideline.
I have broken out the following classes into their own files:
BlockReceiver
BlockSender
DataXceiver
DataXceiverServer
PacketResponder
Throttler
It brings DataNode.java down from about 3000 to about 1000 lines and makes the code easier to overview/navigate in my opinion.
I had to change the visibility of some variables in DataNode and pass in some objects to the contructors in the above classes.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Johan Oskarsson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johan Oskarsson updated HADOOP-3935:
------------------------------------
Attachment: HADOOP-3935.patch
* I have moved PacketResponder to BlockReceiver.
* Changed the classes to use the DataNode logger.
* Changed public method comments to javadocs
* The reason for renaming DataXceiveServer to DataXceiveServer is simply because I assumed it was a typo since the other class is called DataXceiver.
* I left Throttler more or less as is since it is being used by multiple classes and a test outside of the package. As mentioned it is fairly generic and could possibly be used by other classes.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622623#action_12622623 ]
Konstantin Shvachko commented on HADOOP-3935:
---------------------------------------------
+1
> There is one test that fails but it is unrelated
Should the patch still be submitted?
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622009#action_12622009 ]
Konstantin Shvachko commented on HADOOP-3935:
---------------------------------------------
I missed the balancerThrottler. Agreed, it should be a separate class then.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12621938#action_12621938 ]
Raghu Angadi commented on HADOOP-3935:
--------------------------------------
I haven't looked at the patch itself. To add to Konstantin's comments :
- better not make these classes public unless required.
- Throttler is used by both DataBlockScanner and data rebalancing (as an argument to BlockScanner). A separate class is ok. It is a pretty generic class and in that sense usable by other parts.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Johan Oskarsson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johan Oskarsson resolved HADOOP-3935.
-------------------------------------
Resolution: Fixed
Fix Version/s: 0.19.0
Hadoop Flags: [Reviewed]
Committed, thanks for the help guys.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Fix For: 0.19.0
>
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Johan Oskarsson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622844#action_12622844 ]
Johan Oskarsson commented on HADOOP-3935:
-----------------------------------------
Hi. Yes, perhaps I got a bit trigger happy. Although I did get a +1 from another committer and I assumed that the "submit patch to hudson" wouldn't be able to pick up on the renaming of the test using patch.sh and would fail. Perhaps there is a better way to do this which I am not aware of?
Glad to get feedback though, need to properly get the hang of the process
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Fix For: 0.19.0
>
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Johan Oskarsson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Johan Oskarsson updated HADOOP-3935:
------------------------------------
Attachment: HADOOP-3935.patch
patch.sh
Thanks for the feedback, I have renamed the Throttler and changed the visibility. I also moved the test (see patch.sh). I left the other two points for another time, there's still some refactoring work that could be done but I feel they're out of the scope for this ticket.
I assume the hudson patch processor can't handle file rename scripts, so unless someone has any objections I'll commit these changes soon. There is one test that fails but it is unrelated to these changes, see HADOOP-3950.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Hudson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12624802#action_12624802 ]
Hudson commented on HADOOP-3935:
--------------------------------
Integrated in Hadoop-trunk #581 (See [http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/581/])
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Fix For: 0.19.0
>
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (HADOOP-3935) Extract classes from
DataNode.java
Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12621902#action_12621902 ]
rangadi edited comment on HADOOP-3935 at 8/12/08 11:02 AM:
----------------------------------------------------------------
+1 for splitting btw. especially since we now have a datanode directory.
was (Author: rangadi):
+1 splitting btw. especially since we now have a datanode directory.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (HADOOP-3935) Extract classes from
DataNode.java
Posted by "Tsz Wo (Nicholas), SZE (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622638#action_12622638 ]
szetszwo edited comment on HADOOP-3935 at 8/14/08 11:51 AM:
--------------------------------------------------------------------------
Hi Johan, for a committing a patch, we need to follow the guidelines in http://wiki.apache.org/hadoop/HowToContribute . In particular, we should use "Submit Patch" link, and then wait for a "+1" from another committer before committing, as stated in the "Contributing your work" section.
was (Author: szetszwo):
Hi Johan, for a committing a patch, we need to follow the guidelines in http://wiki.apache.org/hadoop/HowToContribute . In particular, we should use Submit Patch link like other contributors, and then wait for a "+1" from another committer before committing, as stated in the "Contributing your work" session.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Fix For: 0.19.0
>
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch, HADOOP-3935.patch, patch.sh
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12621902#action_12621902 ]
Raghu Angadi commented on HADOOP-3935:
--------------------------------------
+1 splitting btw. especially since we now have a datanode directory.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (HADOOP-3935) Extract classes from
DataNode.java
Posted by "Johan Oskarsson (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12622260#action_12622260 ]
johanoskarsson edited comment on HADOOP-3935 at 8/13/08 8:53 AM:
------------------------------------------------------------------
* I have moved PacketResponder to BlockReceiver.
* Changed the classes to use the DataNode logger.
* Changed public method comments to javadocs
* The reason for renaming DataXceiveServer to DataXceiverServer is simply because I assumed it was a typo since the other class is called DataXceiver.
* I left Throttler more or less as is since it is being used by multiple classes and a test outside of the package. As mentioned it is fairly generic and could possibly be used by other classes.
was (Author: johanoskarsson):
* I have moved PacketResponder to BlockReceiver.
* Changed the classes to use the DataNode logger.
* Changed public method comments to javadocs
* The reason for renaming DataXceiveServer to DataXceiveServer is simply because I assumed it was a typo since the other class is called DataXceiver.
* I left Throttler more or less as is since it is being used by multiple classes and a test outside of the package. As mentioned it is fairly generic and could possibly be used by other classes.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch, HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (HADOOP-3935) Extract classes from DataNode.java
Posted by "Konstantin Shvachko (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12621934#action_12621934 ]
Konstantin Shvachko commented on HADOOP-3935:
---------------------------------------------
This is a good idea to split the DataNode code. Some comments
- What is the reason for renaming DataXceiveServer to DataXceive-r-Server?
- I agree with Raghu PacketResponder is not a separate class and should be inside BlockReceiver.
- I also think Throttler class should be contained in DataBlockScanner, because this is the only reason it exists.
- Throttler can be renamed to something less general like BlockScannerThrottler since it is not a subclass anymore.
- Throttler should not be public class and should contain public methods at all.
- The same is applicable to all other separated classes/methods unless it is unavoidable.
- You should use the DataNode logger instead of creating new ones. E.g. for the BlockSender it should look like
{code}
class BlockSender {
private static final Log LOG = DataNode.LOG;
}
{code}
This is important, because there are programs and scripts out there analyzing logs.
- Could you please transform //-style and /*-style comments that are in front of methods to JavaDoc-style /** */
especially if they have to be public.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Issue Comment Edited: (HADOOP-3935) Extract classes from
DataNode.java
Posted by "Raghu Angadi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HADOOP-3935?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12621938#action_12621938 ]
rangadi edited comment on HADOOP-3935 at 8/12/08 12:46 PM:
----------------------------------------------------------------
I haven't looked at the patch itself. To add to Konstantin's comments :
- better not make these classes public unless required.
- Throttler is used by both DataBlockScanner and data rebalancing (as an argument to BlockSender). A separate class is ok. It is a pretty generic class and in that sense usable by other parts.
Edit : typo
was (Author: rangadi):
I haven't looked at the patch itself. To add to Konstantin's comments :
- better not make these classes public unless required.
- Throttler is used by both DataBlockScanner and data rebalancing (as an argument to BlockScanner). A separate class is ok. It is a pretty generic class and in that sense usable by other parts.
> Extract classes from DataNode.java
> ----------------------------------
>
> Key: HADOOP-3935
> URL: https://issues.apache.org/jira/browse/HADOOP-3935
> Project: Hadoop Core
> Issue Type: Improvement
> Components: dfs
> Reporter: Johan Oskarsson
> Assignee: Johan Oskarsson
> Priority: Trivial
> Attachments: HADOOP-3935.patch
>
>
> DataNode.java is becoming hard to navigate with over 3000 lines of code. I suggest moving some of the classes out into their own files in the same package. This will also make it easier to see how the classes depend on each other and to see what code belongs where.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.