Re: Review Request: Add method to PerfLogger to perform cleanup/final steps.

2011-09-22 Thread Kevin Wilfong

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

(Updated 2011-09-22 23:54:37.554390)


Review request for hive, Yongqiang He and Ning Zhang.


Summary (updated)
---

I added a method called finishPerfLogger, which is called at the end of the run 
method in Driver to allow PerfLogger subclasses to perform any cleanup tasks or 
logging that requires all the perf stats to have been calculated.  It takes as 
parameters a Log, to be consistent with the other methods in PerfLogger, as 
well as the QueryPlan from Driver in order to allow information such as the 
query ID to be logged.

I should note that finishPerfLogger is not guaranteed to run.  This was my 
intention.


This addresses bug HIVE-2461.
https://issues.apache.org/jira/browse/HIVE-2461


Diffs
-

  trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1173881 
  trunk/ql/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 1173881 

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


Testing
---

I created a subclass of PerfLogger which implemented the finishPerfLogger 
method, and verified it was successfully called and executed.

I also ran some of the unit tests to verify nothing was broken.


Thanks,

Kevin



Re: Review Request: Add method to PerfLogger to perform cleanup/final steps.

2011-09-22 Thread Yongqiang He

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



trunk/ql/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java
https://reviews.apache.org/r/2023/#comment4579

one nitpick is that do you think call this function a 'close' will be more 
clear? we already have a PerfLogEnd().


- Yongqiang


On 2011-09-22 23:54:37, Kevin Wilfong wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/2023/
 ---
 
 (Updated 2011-09-22 23:54:37)
 
 
 Review request for hive, Yongqiang He and Ning Zhang.
 
 
 Summary
 ---
 
 I added a method called finishPerfLogger, which is called at the end of the 
 run method in Driver to allow PerfLogger subclasses to perform any cleanup 
 tasks or logging that requires all the perf stats to have been calculated.  
 It takes as parameters a Log, to be consistent with the other methods in 
 PerfLogger, as well as the QueryPlan from Driver in order to allow 
 information such as the query ID to be logged.
 
 I should note that finishPerfLogger is not guaranteed to run.  This was my 
 intention.
 
 
 This addresses bug HIVE-2461.
 https://issues.apache.org/jira/browse/HIVE-2461
 
 
 Diffs
 -
 
   trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1173881 
   trunk/ql/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 1173881 
 
 Diff: https://reviews.apache.org/r/2023/diff
 
 
 Testing
 ---
 
 I created a subclass of PerfLogger which implemented the finishPerfLogger 
 method, and verified it was successfully called and executed.
 
 I also ran some of the unit tests to verify nothing was broken.
 
 
 Thanks,
 
 Kevin
 




Re: Review Request: Add method to PerfLogger to perform cleanup/final steps.

2011-09-22 Thread Kevin Wilfong

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

(Updated 2011-09-23 00:08:56.319090)


Review request for hive, Yongqiang He and Ning Zhang.


Changes
---

Thanks Yongqiang.  I didn't like the name either, but the only other one I 
could come up with was finalize which I didn't want to use for obvious reasons.


Summary
---

I added a method called finishPerfLogger, which is called at the end of the run 
method in Driver to allow PerfLogger subclasses to perform any cleanup tasks or 
logging that requires all the perf stats to have been calculated.  It takes as 
parameters a Log, to be consistent with the other methods in PerfLogger, as 
well as the QueryPlan from Driver in order to allow information such as the 
query ID to be logged.

I should note that finishPerfLogger is not guaranteed to run.  This was my 
intention.


This addresses bug HIVE-2461.
https://issues.apache.org/jira/browse/HIVE-2461


Diffs (updated)
-

  trunk/ql/src/java/org/apache/hadoop/hive/ql/Driver.java 1173881 
  trunk/ql/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 1173881 

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


Testing
---

I created a subclass of PerfLogger which implemented the finishPerfLogger 
method, and verified it was successfully called and executed.

I also ran some of the unit tests to verify nothing was broken.


Thanks,

Kevin