----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72444/#review220514 -----------------------------------------------------------
Thanks for the patch Karen! Few questions below. Thanks, Peter common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Lines 2852 (patched) <https://reviews.apache.org/r/72444/#comment308989> Can we add a comment about valid values, and how to turn this off? ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java Lines 92 (patched) <https://reviews.apache.org/r/72444/#comment308990> Can we check the valid values, and can we turn this function off? ql/src/test/org/apache/hadoop/hive/ql/txn/compactor/TestInitiator.java Lines 262 (patched) <https://reviews.apache.org/r/72444/#comment308985> Could we add a check that check that we do not start compaction before the threshold? standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java Lines 113 (patched) <https://reviews.apache.org/r/72444/#comment308988> Do we know for sure, that for null partition that this query is working for all of the supported databases? standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java Lines 151 (patched) <https://reviews.apache.org/r/72444/#comment308986> Why Instant? I usually use System.currentTimeMillis(). Also it might be worth to get it only once, and not for every compactionInfo standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java Lines 153 (patched) <https://reviews.apache.org/r/72444/#comment308987> Maybe single statement return, like: return firstAbortedTxnTime + abortedTimeThreshold < System.currentTimeMillis() - Peter Vary On ápr. 28, 2020, 8:39 de, Karen Coppage wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72444/ > ----------------------------------------------------------- > > (Updated ápr. 28, 2020, 8:39 de) > > > 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 > >
