hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eugene Koifman (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HIVE-4531) [WebHCat] Collecting task logs to hdfs
Date Mon, 16 Sep 2013 19:18:53 GMT

    [ https://issues.apache.org/jira/browse/HIVE-4531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13768648#comment-13768648
] 

Eugene Koifman commented on HIVE-4531:
--------------------------------------

Review Comments:
Should this include e2e tests in addition (or instead of unit tests).  If (when :) Hadoop
changes the log file format this will break, but Unit tests won't catch this since the data
that the tests parse is "static".

Here is a bunch of little things/nits:

o Server.java has “    if (enablelog == true && !TempletonUtils.isset(statusdir))
     throw new BadParam("enablelog is only applicable when statusdir is set");”  in 4 different
places.  Can this be a method?
o What is the purpose of Server#misc()?
o TempletonControllerJob: import org.apache.hive.hcatalog.templeton.Main; - unused import
oo Line 173 - indentation is off
oo Line 295 - writer.close() - This writer is connected to System.err.  What are the implications
of closing this?  What if something tries to write to it later?
o TempletonUtils has unused imports - checkstyle needs to be run on the whole patch.
o TestJobIDParser mixes JUnit3 and JUnit4.  It should either not extend TestCase (I vote for
this) or not use @Test annotations
o Can JobIDParser (and all subclasses) be made package scoped since they are not used outside
templeton pacakge?  Similarly, can methods be made as private as possible?
o JobIDParser#parseJobID() has “fname” param which is not used.  What is the intent? 
Should it be used in openStatusFile() call?  If not, better to remove it.
o JobIDParser#openStatusFile() creas a Reader.  Where/when is it being closed?
o Could the 2 member variables in JobIDParser be made private (even final)?
o Why is TestJobIDParser using findJobID() directly?  Could it not use parseJobID()?
o Can JobIDParser have 1 line of class level javadoc about the purpose of this class?

                
> [WebHCat] Collecting task logs to hdfs
> --------------------------------------
>
>                 Key: HIVE-4531
>                 URL: https://issues.apache.org/jira/browse/HIVE-4531
>             Project: Hive
>          Issue Type: New Feature
>          Components: HCatalog
>            Reporter: Daniel Dai
>            Assignee: Daniel Dai
>             Fix For: 0.12.0
>
>         Attachments: HIVE-4531-1.patch, HIVE-4531-2.patch, HIVE-4531-3.patch, HIVE-4531-4.patch,
HIVE-4531-5.patch, HIVE-4531-6.patch, HIVE-4531-7.patch, HIVE-4531-8.patch, samplestatusdirwithlist.tar.gz
>
>
> It would be nice we collect task logs after job finish. This is similar to what Amazon
EMR does.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message