Re: Review Request 69534: HIVE-20992: Split the property "hive.metastore.dbaccess.ssl.properties" into more coherent and user-friendly properties.

2018-12-14 Thread Vihang Karajgaonkar via Review Board 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69534/#review211343
---




standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Lines 462 (patched)


s/System/Java system



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 328 (original), 342 (patched)


This patch may introduce performance regression in the setConf method which 
is called for every new connection. MetastoreConf.getPassword is expensive 
since it needs to decrypt the truststore password. See HIVE-20740 which has 
more details. In most common cases, these configuration properties almost never 
change once they are set. But they are being read again and again at every new 
connection initialization time. I think we can improve this by caching the db 
and truststore password and reading it once when HMS starts. But I guess this 
could be separate from this JIRA.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 343 (patched)


may be rename this to configureSSLDeprecated



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 346 (patched)


This is a redundant log. If ssl is configured we have other logs below to 
tell us that. Note that logs printed in this method are going to be very 
frequent for each new HMS connection.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 354 (patched)


The exception message suggests taht it Disables SSL and continues, but 
actually, the exception is uncaught and it will terminate the connection 
request (and infact HMS coming up). I think you can remove "Disabling SSL and 
continuing." if thats not needed.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 356 (patched)


This comment is unclear. The getPassword method will get the encrypted 
password if configured right?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 367-369 (patched)


Can you define constants for javax.net.ssl* property keys at the class 
level?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 371 (patched)


Redundant log, please remove.



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 373 (patched)


Don't see the code where we disable and continue. Am I missing something?



standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Line 332 (original), 398 (patched)


I would suggest change this log to say "Configuring SSL using a deprecated 
key " + ConfVars.DBACCESS_SSL_PROPS.toString() + ". This may be removed in the 
future. See HIVE-20992 for more details."



standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java
Lines 1069 (patched)


suggest rename to testDeprecatedConfigIsOverriden()


- Vihang Karajgaonkar


On Dec. 14, 2018, 1:26 a.m., Morio Ramdenbourg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69534/
> ---
> 
> (Updated Dec. 14, 2018, 1:26 a.m.)
> 
> 
> Review request for hive, Adam Holley, Karthik Manamcheri, Peter Vary, and 
> Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-20992
> https://issues.apache.org/jira/browse/HIVE-20992
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> The following new properties were added:
> 
> 1. metastore.dbaccess.use.SSL (hive.metastore.dbaccess.use.SSL)
> 2. javax.net.ssl.trustStore
> 3. javax.net.ssl.trustStorePassword
> 4. javax.net.ssl.trustStoreType
> 
> This was in an effort to guide the user towards an easier SSL
> configuration experience. This is the minimum requirement to set up SSL
> encryption to the HMS backend store.
> 
> This also solves the issue of the truststore password being stored in
> plain text. It can now

Re: Review Request 69256: HIVE-20822

2018-12-14 Thread Jesús Camacho Rodríguez 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69256/
---

(Updated Dec. 14, 2018, 10:33 p.m.)


Review request for hive, Daniel Dai and Thejas Nair.


Bugs: HIVE-20822
https://issues.apache.org/jira/browse/HIVE-20822


Repository: hive-git


Description (updated)
---

HIVE-20822
HIVE-21046


Diffs (updated)
-

  itests/src/test/resources/testconfiguration.properties 
01cad2a79326fb726a57263442dc9591b5bd01b6 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveBetween.java
 ffc705f381d35f689276f9de1663e0a8197c98df 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/jdbc/HiveJdbcConverter.java
 382060243e6d253e7ca88d50b067f75ddf340ede 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/jdbc/JdbcHiveTableScan.java
 5b9a63527066eba1a71d9296fd483b108434bae3 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/HiveJdbcImplementor.java
 531e769b10dcda0169ff8b79d7eac53dc940dc71 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/JDBCAggregationPushDownRule.java
 8f96288aa72fd236bb1bcdd8f774a25848e7fd8b 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/JDBCExpandExpressionsRule.java
 PRE-CREATION 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/JDBCProjectPushDownRule.java
 5c03f87361e67ef18165c3c55903648c9408b214 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/JDBCSortPushDownRule.java
 33dc280d4d751d786ae91125fd440e01fa53deab 
  
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/jdbc/package-info.java
 08a17aa188d99c629fd30ed49b7fef930d7cc8a0 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
aef1d4ab2a7d5a640a552270bd307ac31e58739c 
  ql/src/test/queries/clientpositive/external_jdbc_table2.q 
28a21802fe5aaafb795acbfb75c191869d704300 
  ql/src/test/queries/clientpositive/external_jdbc_table_perf.q PRE-CREATION 
  ql/src/test/results/clientpositive/llap/external_jdbc_table2.q.out 
a1635772e1658fc39aa73669e8cb2819a82e7fe8 
  ql/src/test/results/clientpositive/llap/external_jdbc_table_perf.q.out 
PRE-CREATION 


Diff: https://reviews.apache.org/r/69256/diff/4/

Changes: https://reviews.apache.org/r/69256/diff/3-4/


Testing
---


Thanks,

Jesús Camacho Rodríguez

[jira] [Created] (HIVE-21046) Push IN clause with struct values to JDBC sources

2018-12-14 Thread Jesus Camacho Rodriguez (JIRA) 
Jesus Camacho Rodriguez created HIVE-21046:
--

 Summary: Push IN clause with struct values to JDBC sources
 Key: HIVE-21046
 URL: https://issues.apache.org/jira/browse/HIVE-21046
 Project: Hive
  Issue Type: Improvement
  Components: CBO
Reporter: Jesus Camacho Rodriguez
Assignee: Jesus Camacho Rodriguez


Since RDBMS may not support ROW operator, e.g., Derby, just rewrite into OR/AND 
clauses.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (HIVE-21045) Add connection pool info and rolling performance info to the metrics system

2018-12-14 Thread Karthik Manamcheri (JIRA) 
Karthik Manamcheri created HIVE-21045:
-

 Summary: Add connection pool info and rolling performance info to 
the metrics system
 Key: HIVE-21045
 URL: https://issues.apache.org/jira/browse/HIVE-21045
 Project: Hive
  Issue Type: Improvement
  Components: Standalone Metastore
Reporter: Karthik Manamcheri
Assignee: Karthik Manamcheri


There are two key metrics which I think we lack and which would be really great 
to help with scaling visibility in HMS.

*Average API duration for the past 'n' minutes*
We already compute and log the duration of API calls in the {{PerfLogger}}. We 
don't have any gauge on what the average duration of an API call is for the 
past some bucket of time. This will give us an insight into if there is load on 
the server which is increasing the average API response time.
 
*RDBMS Connection wait time*
We can use different connection pooling libraries such as bonecp or hikaricp. 
These pool managers expose statistics such as average time waiting to get a 
connection, number of connections active, etc. We should expose this as a 
metric so that we can track if the the connection pool size configured is too 
small and we are saturating!

These metrics would help catch problems with HMS resource contention before 
they actually have jobs failing.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (HIVE-21044) Add SLF4J reporter to the metrics system

2018-12-14 Thread Karthik Manamcheri (JIRA) 
Karthik Manamcheri created HIVE-21044:
-

 Summary: Add SLF4J reporter to the metrics system
 Key: HIVE-21044
 URL: https://issues.apache.org/jira/browse/HIVE-21044
 Project: Hive
  Issue Type: New Feature
  Components: HiveServer2, Standalone Metastore
Affects Versions: 3.1.0
Reporter: Karthik Manamcheri
Assignee: Karthik Manamcheri


Lets add SLF4J reporter as an option in Metrics reporting system. Currently we 
support JMX, JSON and Console reporting.

We will add a new option to {{hive.service.metrics.reporter}} called SLF4J. We 
can use the 
{{[Slf4jReporter|https://metrics.dropwizard.io/3.1.0/apidocs/com/codahale/metrics/Slf4jReporter.html]}}
 class.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)