> On April 28, 2020, 8:54 a.m., Peter Vary wrote: > > Thanks for the patch Karen! > > Few questions below. > > > > Thanks, > > Peter
Thanks for the review, Peter! Questions are addressed below. > On April 28, 2020, 8:54 a.m., Peter Vary wrote: > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > > Lines 2852 (patched) > > <https://reviews.apache.org/r/72444/diff/1/?file=2229081#file2229081line2852> > > > > Can we add a comment about valid values, and how to turn this off? Done > On April 28, 2020, 8:54 a.m., Peter Vary wrote: > > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java > > Lines 92 (patched) > > <https://reviews.apache.org/r/72444/diff/1/?file=2229082#file2229082line92> > > > > Can we check the valid values, and can we turn this function off? We can turn it off by setting to a negative number, which used to be checked at org.apache.hadoop.hive.metastore.txn.CompactionTxnHandler#isPastAbortedTimeThreshold and is now checked before the query to metastore is executed. Valid values are checked by org.apache.hadoop.hive.conf.Validator.TimeValidator. > On April 28, 2020, 8:54 a.m., Peter Vary wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > > Lines 113 (patched) > > <https://reviews.apache.org/r/72444/diff/1/?file=2229091#file2229091line114> > > > > Do we know for sure, that for null partition that this query is working > > for all of the supported databases? Group by partition was already present, I just removed a conditional clause and added 2 columns to the projection without touching partition handling. Do you think it's worth testing in each supported db even for these changes? > On April 28, 2020, 8:54 a.m., Peter Vary wrote: > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > > Lines 151 (patched) > > <https://reviews.apache.org/r/72444/diff/1/?file=2229091#file2229091line152> > > > > Why Instant? I usually use System.currentTimeMillis(). Also it might be > > worth to get it only once, and not for every compactionInfo Ok, will do. (I thought wrongly that System.currentTimeMillis was somehow timezone dependendent) - Karen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72444/#review220514 ----------------------------------------------------------- On April 28, 2020, 8:39 a.m., Karen Coppage wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72444/ > ----------------------------------------------------------- > > (Updated April 28, 2020, 8:39 a.m.) > > > Review request for hive, Laszlo Pinter and Peter Vary. > > > Bugs: HIVE-23280 > https://issues.apache.org/jira/browse/HIVE-23280 > > > Repository: hive-git > > > Description > ------- > > When a txn is aborted and the compaction threshold for number of aborted txns > is not reached then the aborted transaction can remain forever in the RDBMS > database. This could result in several serious performance degradations: > > getOpenTxns has to list this aborted txn forever > TXN_TO_WRITE_ID table is not cleaned > We should add a threshold, so after a given time the compaction is started > anyway. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java e3ddbf197b > ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java > 37a5862791 > > ql/src/test/org/apache/hadoop/hive/metastore/txn/TestCompactionTxnHandler.java > 15fcfc0e35 > ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java > 1151466f8c > > standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CompactionInfoStruct.java > 31b6ed450b > > standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php > 9fb7ff011a > > standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py > 4f317b3453 > > standalone-metastore/metastore-common/src/gen/thrift/gen-rb/hive_metastore_types.rb > e64ae0ead2 > standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift > 1e3d6e9b8b > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionInfo.java > 70d63ab18b > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java > 2344c2d5f6 > > standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java > 87130a519d > > > Diff: https://reviews.apache.org/r/72444/diff/1/ > > > Testing > ------- > > > Thanks, > > Karen Coppage > >
