> On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > > Lines 196 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121174#file2121174line196> > > > > I don't understand the logic here. Since major compaction was done > > above, there should only be base/bucket0 and base/bucket1 so there is > > nothing for this query to group. Also, I would think SPLIT_GROUPING_MODE > > should be "query" here... if it's not, where is it set?
Sorry the comment is misleading (what I wanted to convey was what you wrote above) - removed. We set SPLIT_GROUPING_MODE inside CompactorMR, if COMPACTOR_CRUD_QUERY_BASED is set to true. I've modified runCompaction, runInitiator, runWorker methods to create a new HiveConf object and set COMPACTOR_CRUD_QUERY_BASED to true in that one. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > > Lines 255 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121174#file2121174line255> > > > > nit: this could just do ShowCompactions to see if anything got queued up I think you are right, this is redundant as I'm already checking for compaction queue above. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > > Lines 309 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121174#file2121174line309> > > > > How does (3,3,x) end up in bucket0? with bucketing_version=1 it should > > be (val mod num_buckets)=bucketId. Filed https://issues.apache.org/jira/browse/HIVE-21167. For the test cases in this jira, will use bucketing_version=2. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > > Lines 312 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121174#file2121174line312> > > > > And smimilarly, (4,4) is in bucket1... Filed https://issues.apache.org/jira/browse/HIVE-21167. For the test cases in this jira, will use bucketing_version=2. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > > Lines 315 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121174#file2121174line315> > > > > since you just ran a major compaction, there is only 1 file per bucket > > so would split grouper do anything? would there be > 1 split? Sorry, my comment was misleading - removed. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/HiveSplitGenerator.java > > Lines 189 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121176#file2121176line189> > > > > What is this for? It seems fragile since it forces some behavior on > > all tests. Do any newly added tests rely on this? This is a bug that I'm masking for purposes of test (similar to the bug reported here: https://issues.apache.org/jira/browse/HIVE-12957). Basically on my laptop Tez was not estimating taskResource (taskResource = getContext().getVertexTaskResource().getMemory();) correctly - coming up with a -ve value, which would cause it to throw Illegal Capacity exception. I think taskResource should never return -ve value. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java > > Line 1233 (original) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121178#file2121178line1233> > > > > Seems that now the class level JavaDoc is out of sync You mean the overall class doc for OrcRawRecordMerger needs an update? > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRecordUpdater.java > > Lines 637 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121179#file2121179line637> > > > > What throws the IAE? Above I see > > if (!reader.hasMetadataValue(OrcRecordUpdater.ACID_KEY_INDEX_NAME)) { > > > > shouldn't it bail out there if there is no index? I don't know why I was seeing this earlier - don't see it in my runs now. Removed. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRecordUpdater.java > > Lines 638 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121179#file2121179line638> > > > > is there a followup Jira for this? https://jira.apache.org/jira/browse/HIVE-21165 > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > > Lines 248 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121182#file2121182line248> > > > > What does this do for MM table? Hmmmm, bug - fixed it. > On Jan. 23, 2019, 1:23 a.m., Eugene Koifman wrote: > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFValidateAcidSortOrder.java > > Lines 91 (patched) > > <https://reviews.apache.org/r/69367/diff/7/?file=2121184#file2121184line91> > > > > when is it ok for 2 consecutive ROW_IDs to be equal? Throwing an exception now if comparison returs 0. - Vaibhav ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69367/#review212198 ----------------------------------------------------------- On Jan. 22, 2019, 7:04 a.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69367/ > ----------------------------------------------------------- > > (Updated Jan. 22, 2019, 7:04 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 b213609f39 > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestAcidOnTez.java > d6a41919bf > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java > PRE-CREATION > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java bbe7fb0697 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/HiveSplitGenerator.java > 15c14c9be5 > ql/src/java/org/apache/hadoop/hive/ql/exec/tez/SplitGrouper.java 7f8bd229a6 > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRawRecordMerger.java > fbb931cbcd > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcRecordUpdater.java > 6d4578e7a0 > ql/src/java/org/apache/hadoop/hive/ql/io/orc/OrcSplit.java 4d55592b63 > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java > 0e5b3e5473 > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorMR.java > dc05e1990e > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java > a0df82cb20 > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFValidateAcidSortOrder.java > PRE-CREATION > ql/src/test/results/clientpositive/show_functions.q.out 0fdcbda66f > > > Diff: https://reviews.apache.org/r/69367/diff/7/ > > > Testing > ------- > > > Thanks, > > Vaibhav Gumashta > >
