Re: Review Request 14180: HIVE-4531: [WebHCat] Collecting task logs to hdfs

2013-09-20 Thread Daniel Dai


 On Sept. 18, 2013, 11:49 p.m., Eugene Koifman wrote:
  trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LogRetriever.java,
   line 129
  https://reviews.apache.org/r/14180/diff/1/?file=353367#file353367line129
 
  finally {
  if(listWriter != null ) listWriter.close()
  }
  would be better

Actually I shall not close the listWriter in this case.


 On Sept. 18, 2013, 11:49 p.m., Eugene Koifman wrote:
  trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LogRetriever.java,
   line 199
  https://reviews.apache.org/r/14180/diff/1/?file=353367#file353367line199
 
  ArrayListString

Cannot create a generic array of ArrayListString


 On Sept. 18, 2013, 11:49 p.m., Eugene Koifman wrote:
  trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LogRetriever.java,
   line 204
  https://reviews.apache.org/r/14180/diff/1/?file=353367#file353367line204
 
  connection not closed

There is no close method in URLConnection. Close the underlining inputstream 
should be enough according to the docs.


 On Sept. 18, 2013, 11:49 p.m., Eugene Koifman wrote:
  trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LogRetriever.java,
   line 227
  https://reviews.apache.org/r/14180/diff/1/?file=353367#file353367line227
 
  shouldn't the connection be closed?

See previous comment.


 On Sept. 18, 2013, 11:49 p.m., Eugene Koifman wrote:
  trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LogRetriever.java,
   line 340
  https://reviews.apache.org/r/14180/diff/1/?file=353367#file353367line340
 
  close connection

See previous comment.


 On Sept. 18, 2013, 11:49 p.m., Eugene Koifman wrote:
  trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LogRetriever.java,
   line 221
  https://reviews.apache.org/r/14180/diff/1/?file=353367#file353367line221
 
  it seems better that this method use a try/catch(IOException)/finally 
  and handle cleaning up resources here, rather than make every caller do 
  this - all they do is write the stack trace to System.err

This is consistent with other similar methods:getCompletedAttempts, 
getFailedAttempts, etc. One awkward thing I don't like handling it in the 
method is: You will put method body in try/catch block, and in finally, you 
need to close the file within another try/catch block:
finally {
if (writer!=null) {
try {
writer.close();
catch (IOException e) {
}
}


 On Sept. 18, 2013, 11:49 p.m., Eugene Koifman wrote:
  trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/tool/TempletonControllerJob.java,
   line 294
  https://reviews.apache.org/r/14180/diff/1/?file=353372#file353372line294
 
  why is this necessary?  
  There are 2 Watchers created in separate threads in this class.  Both 
  use System.err to log error messages.  If one closes 'err' while the other 
  gets an error right after - it will be a problem.  I think this 
  writer.close() creates a race condition...

I don't realize there are two watcher. I don't remember why I add that, 
probably because one issue I see. I am fine to remove it and keep a close eye 
on that for a while.


- Daniel


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14180/#review26246
---


On Sept. 18, 2013, 3:20 p.m., Daniel Dai wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/14180/
 ---
 
 (Updated Sept. 18, 2013, 3:20 p.m.)
 
 
 Review request for hive.
 
 
 Bugs: HIVE-4531
 https://issues.apache.org/jira/browse/HIVE-4531
 
 
 Repository: hive
 
 
 Description
 ---
 
 SEE HIVE-4531.
 
 
 Diffs
 -
 
   trunk/hcatalog/src/docs/src/documentation/content/xdocs/hive.xml 1524447 
   trunk/hcatalog/src/docs/src/documentation/content/xdocs/mapreducejar.xml 
 1524447 
   
 trunk/hcatalog/src/docs/src/documentation/content/xdocs/mapreducestreaming.xml
  1524447 
   trunk/hcatalog/src/docs/src/documentation/content/xdocs/pig.xml 1524447 
   
 trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/HiveDelegator.java
  1524447 
   
 trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/HiveJobIDParser.java
  PRE-CREATION 
   
 trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/JarDelegator.java
  1524447 
   
 trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/JarJobIDParser.java
  PRE-CREATION 
   
 trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/JobIDParser.java
  PRE-CREATION 
   
 

Re: Review Request 14180: HIVE-4531: [WebHCat] Collecting task logs to hdfs

2013-09-18 Thread Daniel Dai

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14180/
---

(Updated Sept. 18, 2013, 3:20 p.m.)


Review request for hive.


Changes
---

HIVE-4531-9.patch


Bugs: HIVE-4531
https://issues.apache.org/jira/browse/HIVE-4531


Repository: hive


Description
---

SEE HIVE-4531.


Diffs (updated)
-

  trunk/hcatalog/src/docs/src/documentation/content/xdocs/hive.xml 1524447 
  trunk/hcatalog/src/docs/src/documentation/content/xdocs/mapreducejar.xml 
1524447 
  
trunk/hcatalog/src/docs/src/documentation/content/xdocs/mapreducestreaming.xml 
1524447 
  trunk/hcatalog/src/docs/src/documentation/content/xdocs/pig.xml 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/HiveDelegator.java
 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/HiveJobIDParser.java
 PRE-CREATION 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/JarDelegator.java
 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/JarJobIDParser.java
 PRE-CREATION 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/JobIDParser.java
 PRE-CREATION 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LauncherDelegator.java
 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/LogRetriever.java
 PRE-CREATION 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/PigDelegator.java
 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/PigJobIDParser.java
 PRE-CREATION 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/Server.java
 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/StreamingDelegator.java
 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/tool/TempletonControllerJob.java
 1524447 
  
trunk/hcatalog/webhcat/svr/src/main/java/org/apache/hive/hcatalog/templeton/tool/TempletonUtils.java
 1524447 
  trunk/hcatalog/webhcat/svr/src/test/data/status/hive/stderr PRE-CREATION 
  trunk/hcatalog/webhcat/svr/src/test/data/status/jar/stderr PRE-CREATION 
  trunk/hcatalog/webhcat/svr/src/test/data/status/pig/stderr PRE-CREATION 
  trunk/hcatalog/webhcat/svr/src/test/data/status/streaming/stderr PRE-CREATION 
  
trunk/hcatalog/webhcat/svr/src/test/java/org/apache/hive/hcatalog/templeton/TestJobIDParser.java
 PRE-CREATION 
  
trunk/hcatalog/webhcat/svr/src/test/java/org/apache/hive/hcatalog/templeton/tool/TestTempletonUtils.java
 1524447 

Diff: https://reviews.apache.org/r/14180/diff/


Testing
---

WebHCat unit tests
e2e tests in HIVE-5078 under both Linux/Windows


Thanks,

Daniel Dai



Review Request 14180: HIVE-4531: [WebHCat] Collecting task logs to hdfs

2013-09-17 Thread Daniel Dai

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14180/
---

Review request for hive.


Bugs: HIVE-4531
https://issues.apache.org/jira/browse/HIVE-4531


Repository: hive


Description
---

SEE HIVE-4531.


Diffs
-


Diff: https://reviews.apache.org/r/14180/diff/


Testing
---

WebHCat unit tests
e2e tests in HIVE-5078 under both Linux/Windows


Thanks,

Daniel Dai