----------------------------------------------------------- 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 > >
