Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Hive >> mail # dev >> Re: Review Request: HIVE-4513 - disable hivehistory logs by default


Copy link to this message
-
Re: Review Request: HIVE-4513 - disable hivehistory logs by default

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

data/conf/hive-site.xml
<https://reviews.apache.org/r/11029/#comment44263>

    Is there a reason for this to be set to true for tests? Unless there is, we should set config in tests to the default values, since we should test default configs.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44264>

    doesn't read right. I guess you wanted
    ... statistics into a file.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44266>

    This is existing comment which doesnt read right. But since we are doing major surgery on HiveHistory, it will be good to update to make it more sensible.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44268>

    I think word job is not required in this comment.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44269>

    I think query is a better word than job here.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44270>

    Better worded as
    Called at the end of query.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44271>

    Again use of word job is confusing, we shall use query here as well.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44272>

    Incorrect comment.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistory.java
<https://reviews.apache.org/r/11029/#comment44274>

    Function name is IdtoTable, but comment says table to id. One of this needs to be corrected.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44275>

    Similar comment as in HiveHistory.java

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44277>

    Should this be hive.ql.exec.HiveHistoryImpl to avoid confusion?

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44278>

    and instead of an ?

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44280>

    In case of incorrect config, should this throw an exception instead of silent return, otherwise there will be errors later when something is tried to be written in history file.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44281>

    Same comment as above.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44283>

    This should be static class variable, otherwise nextInt() will return same value for each invocation.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44284>

    Instead of / we shall use File.Seprator

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44287>

    Consider using File.createNewFile here.

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44288>

    Use  System.getProperty("line.separator") instead of \n

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryImpl.java
<https://reviews.apache.org/r/11029/#comment44289>

    start of query ?

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryUtil.java
<https://reviews.apache.org/r/11029/#comment44291>

    Missing apache header

ql/src/java/org/apache/hadoop/hive/ql/history/HiveHistoryViewer.java
<https://reviews.apache.org/r/11029/#comment44292>

    HiveHistoryViewer.class
- Ashutosh Chauhan
On May 13, 2013, 10:12 p.m., Thejas Nair wrote: