|
New functionality needs to go into trunk, rather than the 0.16 branch, which along with the 0.17 and 0.18 branches are frozen for new features.
Craig, this looks like a good addition. A few comments:
1. By default you should not be able kill jobs from the web UI (see https://issues.apache.org/jira/browse/HADOOP-1484 Finally, I notice that the JSPs have a lot of embedded Java (this is true with the current code: this patch is just following the current style), which makes them hard to test and hard to extend or reuse. Is there a better design we should adopt so that we can aim to improve our code over time? Thoughts? >Finally, I notice that the JSPs have a lot of embedded Java (this is true with the current code: this patch is just following the current style), which makes them hard to test and hard to extend or reuse. Is there a better design we should adopt so that we can aim to improve our code over time? Thoughts?
I've noticed that too. It's not ideal, and the use of JSP forces hadoop to have the jasper runtimes. Some options 1. Move to JSP tags and define custom tags for lots of the reporting. 2. Move all the html generation to the java source.
Here's another option, nice and simple
3. Move the Java code from JSP pages into associated Java source. There's no reason why you can't have Java source that takes as arguments the request, response and JSPWriter and does the work inside its own code. No need for JSP tags, almost no mixing of Java and JSP content. Having looked at some of the JSP pages, I'm going to suggest some JSP cleanups as separate issues. > 3. Move the Java code from JSP pages into associated Java source.
This is the technique used on the HDFS pages with the org.apache.hadoop.hdfs.server.namenode.JspHelper class. Perhaps we should do the same with the MapReduce pages. The trick is keeping the markup and code separate, as far as is possible. If you want to separate presentation from code without needing a JSP runtime, you might want to take a look at Apache Velocity (http://velocity.apache.org/engine/devel/getting-started.html
 ), which is a lighter-weight template language that nonetheless lets you defined named objects in your Java code and access them from Velocity templates using bean properties. I've used it for showing some tables before, it's not too bad. It would probably require only a bit more extra work than moving the Java logic into Java.
isn't the downside that the web pages can't be changed without going at or around the source? I had a look at JspHelper and don't see that's it's much better than the current JSP files (no class attributes, hardcoded styes). I think I'd suggest a barebones templating language like StringTemplate (BSD) over Java helpers - or is there a suitable ASF lib? This is pretty cool. I noticed however that if I click on change when no jobs are selected you get an Internal Server Error. Can we get that fixed?
hi owen - i am having trouble reproducing this. a few questions: what browser are you using? are you seeing this error in the browser, or in the logs for the jobtracker? finally, what operation are you trying to do (priority change or kill, or both). thanks!
Ah, sorry about that. I'm using Mac OS & Firefox 3.0.1.
I had no jobs selected and changed the priority and pressed "change". Clicked "ok" and then get: HTTP ERROR: 500
Internal Server Error
RequestURI=/jobtracker.jsp
Powered by Jetty://
back in the browser. More importantly, here is the exception stack from the job tracker log: 2008-08-18 16:30:27,576 WARN /: /jobtracker.jsp:
java.lang.NullPointerException
at org.apache.hadoop.mapred.jobtracker_jsp.processButtons(jobtracker_jsp
.java:41)
at org.apache.hadoop.mapred.jobtracker_jsp._jspService(jobtracker_jsp.ja
va:196)
at org.apache.jasper.runtime.HttpJspBase.service(HttpJspBase.java:94)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:802)
at org.mortbay.jetty.servlet.ServletHolder.handle(ServletHolder.java:427
)
at org.mortbay.jetty.servlet.WebApplicationHandler.dispatch(WebApplicati
onHandler.java:475)
at org.mortbay.jetty.servlet.ServletHandler.handle(ServletHandler.java:5
67)
at org.mortbay.http.HttpContext.handle(HttpContext.java:1565)
at org.mortbay.jetty.servlet.WebApplicationContext.handle(WebApplication
Context.java:635)
at org.mortbay.http.HttpContext.handle(HttpContext.java:1517)
at org.mortbay.http.HttpServer.service(HttpServer.java:954)
at org.mortbay.http.HttpConnection.service(HttpConnection.java:814)
at org.mortbay.http.HttpConnection.handleNext(HttpConnection.java:981)
at org.mortbay.http.HttpConnection.handle(HttpConnection.java:831)
at org.mortbay.http.SocketListener.handleConnection(SocketListener.java:
244)
at org.mortbay.util.ThreadedServer.handle(ThreadedServer.java:357)
at org.mortbay.util.ThreadPool$PoolThread.run(ThreadPool.java:534)
fix for null pointer exception in browser when no checkbox is specified and priority is changed.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12388955/hadoop-3866-v2.patch against trunk revision 689380. +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 release audit. The applied patch generated 273 release audit warnings (more than the trunk's current 271 warnings). +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3118/testReport/ This message is automatically generated. Added license headers to hadoop.css and jobtracker.js.
There are no unit tests for this change since it is a change in the web UI (which doesn't have any automated testing at the moment). The contrib test failure is unrelated to this change. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12389014/hadoop-3866-v3.patch against trunk revision 689602. +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 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/3126/testReport/ This message is automatically generated. > There are no unit tests for this change since it is a change in the web UI (which doesn't have any automated testing at the moment). The contrib test failure is unrelated to this change.
You mean, because nobody has written any tests for the UI, you don't need to add any either ? Just checking I just committed this. Thanks, Craig!
Integrated in Hadoop-trunk #595 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/595/
) |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 |
|
Improvement
Closed
Major
Improve Hadoop Jobtracker Admin
http://issues.apache.org/jira/secure/attachment/12387230/admin-1.patch
against trunk revision 681243.
+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.
Please justify why no tests are needed for this patch.
-1 patch. The patch command could not apply the patch.
Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/2988/console
This message is automatically generated.