|
This patch moves the http server to the NameNode class.
Additionally I removed one redundant NN constructor and redundant fields handlerCount, startTime. I changed dfshealth.jsp and nn_browsedfscontent.jsp to not use getFSNamesystem() method. I could not get rid of the namenodeAddress field in FSNamesystem because it would require changing JSPHelper constructor, which is used in too many if not all other jsps. One more change is that I moved field supportAppends from NameNode to FSNamesystem where all such fields are used to be.
The patch does not apply to trunk.
Patch synchronized with trunk.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12393549/MoveHttpServer.patch  against trunk revision 712344. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3558/testReport/ This message is automatically generated. Even though this will be tricky for me to merge in with my lifecycle stuff, I think it is good, hence the : +1
HADOOP-4383 would benefit from it, as it would not need to rummage in the namesystem to get the relevant web port. FSNamesystem.getNameNodeInfoPort() is public -if anyone is using it and its sibling methods their app will break. And as NameNode.fsnamesystem is public, this is effectively a bit of a public API that is being broken. I know my code will break, and I'm happy with that, but I don't know who else is using it. In FSNamesystem.close(), it is better not to take dir.close() out from finally{}. If pendingReplications.stop() throws a RuntimeException, e.g. NPE, then dir won't be closed and, as a result, the image won't be closed.
Thanks for the comments.
In this version I restored the final section and deprecated FSNamesystem.getDFSNameNodeAddress() in order to be able to remove it later. +1 new patch looks good.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12393663/MoveHttpServer.patch against trunk revision 712729. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 Eclipse classpath. The patch retains Eclipse classpath integrity. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3571/testReport/ This message is automatically generated. I just committed this.
OK, My code is up in sync with this; I even have a test for MiniDFSCluster that gets the port number and asserts that it is open
Edit release note for publication.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Improvement
Closed
Major
Move http server from FSNamesystem into NameNode.
One of the consequences of the current design is that we expose FSNamesystem to jsp-s and servlets through a static public method getFSNamesystem(), which returns this. This seems to be an unnecessary shortcut, because all jsp-s and servlets have a reference to the NameNode instance and can access FSNamesystem data via name-node.
This also creates redundant fields in FSNamesystem like name-node's host, port and infoPort.