Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15956 )
Change subject: IMPALA-9764: TransactionKeepalive should set sleep interval based on Hive Configuration ...................................................................... Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/15956/1/fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java File fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java: http://gerrit.cloudera.org:8080/#/c/15956/1/fe/src/main/java/org/apache/impala/common/TransactionKeepalive.java@211 PS1, Line 211: HiveConf hiveConf = new HiveConf(TransactionKeepalive.class); Can we always assume that there will be a hive-site.xml on the machine? What will happen if there isn't any, will we use the default values? An alternative could be to ask HMS about the configs: https://github.com/apache/hive/blob/037eacea46371015a7f9894c5a9ccfb9708d5c56/standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift#L2298 I see that we already create a new HiveConfg in other parts of Impala, so I am ok with keeping this pattern in the is patch, just wanted to share my doubts about our approach. -- To view, visit http://gerrit.cloudera.org:8080/15956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I546dac324494f1c945e8943e267f8146d22a837d Gerrit-Change-Number: 15956 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Comment-Date: Tue, 19 May 2020 13:40:42 +0000 Gerrit-HasComments: Yes
