Re: Review Request 41582: HIVE-12713: Miscellaneous improvements in driver compile and execute logging

2015-12-21 Thread Xuefu Zhang 

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

Ship it!


Ship It!

- Xuefu Zhang


On Dec. 21, 2015, 3:46 p.m., Chaoyu Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41582/
> ---
> 
> (Updated Dec. 21, 2015, 3:46 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-12713
> https://issues.apache.org/jira/browse/HIVE-12713
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Patch improves the driver compile and execute logging in following:
> 1. ensuring that only the redacted query to be logged out
> 2. removing redundant variable substitution in HS2 SQLOperation
> 3. logging out the query and its compilation time without having to enable 
> PerfLogger debug, to help identify badly written queries which take a lot of 
> time to compile and probably cause other good queries to be queued 
> (HIVE-12516)
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 98ebd50 
>   
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java
>  7cc0acf 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> e9206b9 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/cbo_rp_TestJdbcDriver2.java
>  c66f166 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithMr.java
>  d21571e 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithTez.java
>  8b21647 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 3d5f3b5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java c33bb66 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 
> d90dd0d 
> 
> Diff: https://reviews.apache.org/r/41582/diff/
> 
> 
> Testing
> ---
> 
> 1. Manual tests
> 2. Precommit tests
> 
> 
> Thanks,
> 
> Chaoyu Tang
> 
>

Re: Review Request 41582: HIVE-12713: Miscellaneous improvements in driver compile and execute logging

2015-12-21 Thread Chaoyu Tang 

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

(Updated Dec. 21, 2015, 3:46 p.m.)


Review request for hive.


Changes
---

Revised the patch based on the review


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


Repository: hive-git


Description
---

Patch improves the driver compile and execute logging in following:
1. ensuring that only the redacted query to be logged out
2. removing redundant variable substitution in HS2 SQLOperation
3. logging out the query and its compilation time without having to enable 
PerfLogger debug, to help identify badly written queries which take a lot of 
time to compile and probably cause other good queries to be queued (HIVE-12516)


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 98ebd50 
  
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 
7cc0acf 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
e9206b9 
  
itests/hive-unit/src/test/java/org/apache/hive/jdbc/cbo_rp_TestJdbcDriver2.java 
c66f166 
  
itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithMr.java
 d21571e 
  
itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithTez.java
 8b21647 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 3d5f3b5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java c33bb66 
  service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 
d90dd0d 

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


Testing
---

1. Manual tests
2. Precommit tests


Thanks,

Chaoyu Tang

Re: Review Request 41582: HIVE-12713: Miscellaneous improvements in driver compile and execute logging

2015-12-21 Thread Chaoyu Tang 


> On Dec. 21, 2015, 3:13 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/Driver.java, line 407
> > 
> >
> > To clarify, the above log redaction is needed because of the addition 
> > of this line, right?

Yes, the redaction logic, which was already there, is moved to the beginning of 
the method and also before this logging is called.


> On Dec. 21, 2015, 3:13 p.m., Xuefu Zhang wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java, line 185
> > 
> >
> > It might be better if we check if debug log is enabled. Same below

Yeah, it is recommended to call isLogEnabled for any debug logging message 
which needs to be concatenated. Fixed. Thanks


- Chaoyu


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


On Dec. 20, 2015, 1:05 a.m., Chaoyu Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41582/
> ---
> 
> (Updated Dec. 20, 2015, 1:05 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-12713
> https://issues.apache.org/jira/browse/HIVE-12713
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Patch improves the driver compile and execute logging in following:
> 1. ensuring that only the redacted query to be logged out
> 2. removing redundant variable substitution in HS2 SQLOperation
> 3. logging out the query and its compilation time without having to enable 
> PerfLogger debug, to help identify badly written queries which take a lot of 
> time to compile and probably cause other good queries to be queued 
> (HIVE-12516)
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 98ebd50 
>   
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java
>  7cc0acf 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> e9206b9 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/cbo_rp_TestJdbcDriver2.java
>  c66f166 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithMr.java
>  d21571e 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithTez.java
>  8b21647 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 3d5f3b5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java c33bb66 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 
> d90dd0d 
> 
> Diff: https://reviews.apache.org/r/41582/diff/
> 
> 
> Testing
> ---
> 
> 1. Manual tests
> 2. Precommit tests
> 
> 
> Thanks,
> 
> Chaoyu Tang
> 
>

Re: Review Request 41582: HIVE-12713: Miscellaneous improvements in driver compile and execute logging

2015-12-21 Thread Xuefu Zhang 

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



ql/src/java/org/apache/hadoop/hive/ql/Driver.java (line 407)


To clarify, the above log redaction is needed because of the addition of 
this line, right?



ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java (line 185)


It might be better if we check if debug log is enabled. Same below


- Xuefu Zhang


On Dec. 20, 2015, 1:05 a.m., Chaoyu Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41582/
> ---
> 
> (Updated Dec. 20, 2015, 1:05 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-12713
> https://issues.apache.org/jira/browse/HIVE-12713
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> Patch improves the driver compile and execute logging in following:
> 1. ensuring that only the redacted query to be logged out
> 2. removing redundant variable substitution in HS2 SQLOperation
> 3. logging out the query and its compilation time without having to enable 
> PerfLogger debug, to help identify badly written queries which take a lot of 
> time to compile and probably cause other good queries to be queued 
> (HIVE-12516)
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 98ebd50 
>   
> itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java
>  7cc0acf 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> e9206b9 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/cbo_rp_TestJdbcDriver2.java
>  c66f166 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithMr.java
>  d21571e 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithTez.java
>  8b21647 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 3d5f3b5 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java c33bb66 
>   service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 
> d90dd0d 
> 
> Diff: https://reviews.apache.org/r/41582/diff/
> 
> 
> Testing
> ---
> 
> 1. Manual tests
> 2. Precommit tests
> 
> 
> Thanks,
> 
> Chaoyu Tang
> 
>

Re: Review Request 41582: HIVE-12713: Miscellaneous improvements in driver compile and execute logging

2015-12-19 Thread Chaoyu Tang 

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

(Updated Dec. 20, 2015, 1:05 a.m.)


Review request for hive.


Changes
---

remove the trailing empty space.


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


Repository: hive-git


Description
---

Patch improves the driver compile and execute logging in following:
1. ensuring that only the redacted query to be logged out
2. removing redundant variable substitution in HS2 SQLOperation
3. logging out the query and its compilation time without having to enable 
PerfLogger debug, to help identify badly written queries which take a lot of 
time to compile and probably cause other good queries to be queued (HIVE-12516)


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 98ebd50 
  
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 
7cc0acf 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
e9206b9 
  
itests/hive-unit/src/test/java/org/apache/hive/jdbc/cbo_rp_TestJdbcDriver2.java 
c66f166 
  
itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithMr.java
 d21571e 
  
itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithTez.java
 8b21647 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 3d5f3b5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java c33bb66 
  service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 
d90dd0d 

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


Testing
---

1. Manual tests
2. Precommit tests


Thanks,

Chaoyu Tang

Review Request 41582: HIVE-12713: Miscellaneous improvements in driver compile and execute logging

2015-12-18 Thread Chaoyu Tang 

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

Review request for hive.


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


Repository: hive-git


Description
---

Patch improves the driver compile and execute logging in following:
1. ensuring that only the redacted query to be logged out
2. removing redundant variable substitution in HS2 SQLOperation
3. logging out the query and its compilation time without having to enable 
PerfLogger debug, to help identify badly written queries which take a lot of 
time to compile and probably cause other good queries to be queued (HIVE-12516)


Diffs
-

  common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 98ebd50 
  
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestBeeLineWithArgs.java 
7cc0acf 
  itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
e9206b9 
  
itests/hive-unit/src/test/java/org/apache/hive/jdbc/cbo_rp_TestJdbcDriver2.java 
c66f166 
  
itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithMr.java
 d21571e 
  
itests/hive-unit/src/test/java/org/apache/hive/service/cli/operation/TestOperationLoggingAPIWithTez.java
 8b21647 
  ql/src/java/org/apache/hadoop/hive/ql/Driver.java 3d5f3b5 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ParseDriver.java c33bb66 
  service/src/java/org/apache/hive/service/cli/operation/SQLOperation.java 
d90dd0d 

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


Testing
---

1. Manual tests
2. Precommit tests


Thanks,

Chaoyu Tang