----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26661/#review56449 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveBaseFunctionResultList.java <https://reviews.apache.org/r/26661/#comment96779> Nit: could we put -1L or skip it and suppress the warning instead of a random number for this? ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveKVResultCache.java <https://reviews.apache.org/r/26661/#comment96780> Do you mean one thread can access hasNext() and next(), while multiple threads can concurrently access add()? The doc here is a little unclear on this. ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveKVResultCache.java <https://reviews.apache.org/r/26661/#comment96782> I think the condition check should be also in the synchronized block, as it accesses protected resources: container.rowCount() and cursor. ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveKVResultCache.java <https://reviews.apache.org/r/26661/#comment96783> Same here, cursor and container are protected resources and they need to be in the synchnized block. ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestHiveKVResultCache.java <https://reviews.apache.org/r/26661/#comment96784> Same here. - Xuefu Zhang On Oct. 13, 2014, 9:33 p.m., Jimmy Xiang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26661/ > ----------------------------------------------------------- > > (Updated Oct. 13, 2014, 9:33 p.m.) > > > Review request for hive and Xuefu Zhang. > > > Bugs: HIVE-7873 > https://issues.apache.org/jira/browse/HIVE-7873 > > > Repository: hive-git > > > Description > ------- > > Re-enabled lazy HiveBaseFunctionResultList. A separate RowContainer is used > to work around the no-write-after-read limitation of RowContainer. The patch > also fixed a concurrency issue in HiveKVResultCache. Synchronized is used > instead of reentrant lock since I assume there won't be many threads to > access the cache. > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveBaseFunctionResultList.java > 0df2580 > ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveKVResultCache.java > a6b9037 > ql/src/test/org/apache/hadoop/hive/ql/exec/spark/TestHiveKVResultCache.java > 496a11f > > Diff: https://reviews.apache.org/r/26661/diff/ > > > Testing > ------- > > Unit test, some simple perf test. > > > Thanks, > > Jimmy Xiang > >
