hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesús Camacho Rodríguez <jcamachorodrig...@hortonworks.com>
Subject Re: Review Request 65304: HIVE-18513 Query results caching
Date Mon, 29 Jan 2018 18:26:19 GMT

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 3699 (patched)
<https://reviews.apache.org/r/65304/#comment276084>

    Is this the permission for each results directory? Does this mean that results cannot
be shared by different users?
    
    Why does this need to be configurable? (I would assume this is not something that you
let the user decide).



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 3710 (patched)
<https://reviews.apache.org/r/65304/#comment276085>

    What is _entry_ referring to? Does it mean the SQL query string size? We should extend
the description to make it clear.



ql/src/java/org/apache/hadoop/hive/ql/Driver.java
Lines 1829 (patched)
<https://reviews.apache.org/r/65304/#comment276095>

    Currently what happens when we get this exception?
    
    It seems to me that mechanism in HIVE-17626 works when execution itself fails, but in
this case, whether the cache entry is still valid or not can be inferred statically at planning
time, hence I am not sure whether it should be handled the same way? It seems we will have
certain overhead that might not be necessary.
    
    Can't we check the validity of the entry when we are replacing the plan by the scan on
the cached results, e.g., in SemanticAnalyzer?



ql/src/java/org/apache/hadoop/hive/ql/cache/results/QueryResultsCache.java
Lines 174 (patched)
<https://reviews.apache.org/r/65304/#comment276093>

    What happens if execution fails? Will the results still be cleaned properly?



ql/src/java/org/apache/hadoop/hive/ql/cache/results/QueryResultsCache.java
Lines 262 (patched)
<https://reviews.apache.org/r/65304/#comment276094>

    Code might have been simpler using Guava cache. This is just a note, maybe something that
can be considered for a follow-up.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
Lines 72 (patched)
<https://reviews.apache.org/r/65304/#comment276092>

    Leaf can be other node too, e.g., _DruidQuery_. Now reusing results is time-based, but
for those type of tables, we do not have guarantees (as with other external tables). Thus,
we should probably return true iff scan is an instance of HiveTableScan, false otherwise.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
Lines 77 (patched)
<https://reviews.apache.org/r/65304/#comment276089>

    _getProjects()_



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
Lines 87 (patched)
<https://reviews.apache.org/r/65304/#comment276088>

    It should be _getCondition()_.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
Lines 96 (patched)
<https://reviews.apache.org/r/65304/#comment276090>

    I think all the information should be in _getCondition()_.



ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
Lines 124 (patched)
<https://reviews.apache.org/r/65304/#comment276091>

    - There are some operators missing, e.g., semijoin.
    - I would create specific visit methods for all operators, then a fall-back visit method
on RelNode that returns false if it does not recognize the operator that we are visiting (as
a way to prevent any incorrect assesment, e.g., if new operators are added in the future).


- Jesús Camacho Rodríguez


On Jan. 25, 2018, 7:34 a.m., Jason Dere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65304/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2018, 7:34 a.m.)
> 
> 
> Review request for hive, Ashutosh Chauhan, Gopal V, and Jesús Camacho Rodríguez.
> 
> 
> Bugs: HIVE-18513
>     https://issues.apache.org/jira/browse/HIVE-18513
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> - For queries that result in MR/Tez/Spark jobs on the cluster, save the temporary query
results to a cache directory where they can be re-used.
> - Add QueryResultsCache to manage cached results. Currently cache invalidation is time-based,
update-based cache invalidation needs to be added later.
> - Driver/SemanticAnalyzer/Calcite planner changes to lookup queries in the cache and
use in place of the query plan.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0c2cf05 
>   common/src/java/org/apache/hadoop/hive/ql/log/PerfLogger.java 2767bca 
>   data/conf/hive-site.xml 01f83d1 
>   data/conf/llap/hive-site.xml cdda875 
>   data/conf/perf-reg/spark/hive-site.xml 497a61f 
>   data/conf/perf-reg/tez/hive-site.xml 012369f 
>   data/conf/rlist/hive-site.xml 9de00e5 
>   data/conf/spark/local/hive-site.xml fd0e6a0 
>   data/conf/spark/standalone/hive-site.xml 1e5bd65 
>   data/conf/spark/yarn-client/hive-site.xml a9a788b 
>   data/conf/tez/hive-site.xml 4519678 
>   itests/hive-blobstore/src/test/resources/hive-site.xml 038db0d 
>   itests/src/test/resources/testconfiguration.properties 1017249 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java 3f377f9 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 74595b0 
>   ql/src/java/org/apache/hadoop/hive/ql/cache/results/CacheUsage.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/cache/results/QueryResultsCache.java PRE-CREATION

>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 2e1fd37 
>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveCalciteUtil.java f0dd167

>   ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HiveRelOpMaterializationValidator.java
PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 372cfad 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 85a1f34 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/QBParseInfo.java ae2ec3d 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 28e1041 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/FetchWork.java 7243dc7 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 8af19d8 
>   ql/src/test/queries/clientpositive/results_cache_1.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/results_cache_2.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/results_cache_capacity.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/results_cache_lifetime.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/results_cache_temptable.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/results_cache_with_masking.q PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/results_cache_1.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/results_cache_1.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/results_cache_2.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/results_cache_capacity.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/results_cache_lifetime.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/results_cache_temptable.q.out PRE-CREATION 
>   ql/src/test/results/clientpositive/results_cache_with_masking.q.out PRE-CREATION 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 2a528cd 
> 
> 
> Diff: https://reviews.apache.org/r/65304/diff/3/
> 
> 
> Testing
> -------
> 
> qfile tests added.
> 
> 
> Thanks,
> 
> Jason Dere
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message