> On Nov. 16, 2018, 3:02 a.m., Eugene Koifman wrote: > >
Addressed the comments; will upload changes in the patch. > On Nov. 16, 2018, 3:02 a.m., Eugene Koifman wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > > Lines 2685 (patched) > > <https://reviews.apache.org/r/69367/diff/1/?file=2108425#file2108425line2685> > > > > "And minor compaction will be disabled." - should make sure Initiator > > doesn't start minor and that Alter Table commands requesting Minor are > > no-op or throw so that these don't get into the compactor queue. We should > > also, perhaps think about how Initiator triggers Major compactions - are > > current config params adequate? Should do at least the 2nd part in a > > follow up jira, maybe both. Created https://jira.apache.org/jira/browse/HIVE-20933 > On Nov. 16, 2018, 3:02 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > > Lines 245 (patched) > > <https://reviews.apache.org/r/69367/diff/1/?file=2108430#file2108430line252> > > > > Ideally this should be prevented before it gets into the > > compction_queue. throwing here will cause failed compactions to accumulate > > in SHOW COMPACTIONS and prevent auto-scheduling of new ones. Created https://jira.apache.org/jira/browse/HIVE-20933 for this. > On Nov. 16, 2018, 3:02 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > > Lines 399 (patched) > > <https://reviews.apache.org/r/69367/diff/1/?file=2108430#file2108430line406> > > > > should this be in a finally{}? SessionState is threadLocal so it may > > get reused... or do we shutdown the session each time? Made the change. We do remove the threadlocal via SessionState.detachSession() in the finally block in DriverUtils.runOnDriver > On Nov. 16, 2018, 3:02 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > > Lines 513 (patched) > > <https://reviews.apache.org/r/69367/diff/1/?file=2108430#file2108430line521> > > > > why do you need partition key/values in the query? we are always > > reading a single partition. This is achieved by getAcidState() which takes > > partition dir as input (i.e. all the files it returns are within a given > > partition) As discussed, I'll throw an exeption from SplitGrouper if we somehow end up getting splits across partitions in there (unlikely, since the sql query we run adds a partition filter). - Vaibhav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69367/#review210589 ----------------------------------------------------------- On Nov. 16, 2018, 12:59 a.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69367/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2018, 12:59 a.m.) > > > Review request for hive and Eugene Koifman. > > > Bugs: HIVE-20699 > https://issues.apache.org/jira/browse/HIVE-20699 > > > Repository: hive-git > > > Description > ------- > > https://jira.apache.org/jira/browse/HIVE-20699 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 65264f323f > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestAcidOnTez.java > 40dd992455 > pom.xml 26b662e4c3 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java 7f8bd229a6 > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcSplit.java 4d55592b63 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > 92c74e1d06 > > > Diff: https://reviews.apache.org/r/69367/diff/1/ > > > Testing > ------- > > > Thanks, > > Vaibhav Gumashta > >
