----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26661/#review56487 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveKVResultCache.java <https://reviews.apache.org/r/26661/#comment96841> I think it makes sense to make the class fully thread safe, for the sake of clarity and confidence. If there is no concurrent read, synchronization cost can be ignored. ql/src/java/org/apache/hadoop/hive/ql/exec/spark/HiveKVResultCache.java <https://reviews.apache.org/r/26661/#comment96842> Yeah. Let's make this thread safe for read also. Otherwise, it's error prone and hard to understand. We also need to make clear() thread safe, probably. - 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 > >
