Re: Review Request 64490: HIVE-14498

2017-12-15 Thread Jesús Camacho Rodríguez

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

(Updated Dec. 16, 2017, 2:19 a.m.)


Review request for hive, Ashutosh Chauhan and Eugene Koifman.


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


Repository: hive-git


Description
---

HIVE-14498


Diffs (updated)
-

  common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 
711dfbdc1f05a25ffc64d297c6b1b25853d99a57 
  data/files/ssb/customer/0_0 PRE-CREATION 
  data/files/ssb/date/0_0 PRE-CREATION 
  data/files/ssb/lineorder/0_0 PRE-CREATION 
  data/files/ssb/part/0_0 PRE-CREATION 
  data/files/ssb/supplier/0_0 PRE-CREATION 
  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
  itests/hive-unit/src/main/java/org/hadoop/hive/jdbc/SSLTestUtils.java 
6cbcf8ca7cdea8b99736b50b38634c077895f5d8 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreEventListener.java
 e8031066c2dc9b1b40be570e9098ac0c55d997be 
  
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/parse/TestReplicationScenarios.java
 55acd1df3697f1742c826f1cd9648634811b915f 
  metastore/scripts/upgrade/derby/047-HIVE-14498.derby.sql PRE-CREATION 
  metastore/scripts/upgrade/derby/hive-schema-3.0.0.derby.sql 
f93d0d1d127156732a523fb45c3c2635d3ee530e 
  metastore/scripts/upgrade/derby/hive-txn-schema-3.0.0.derby.sql 
52713df30c66b34a3d5b815ca2814e7dca1e45a6 
  metastore/scripts/upgrade/derby/upgrade-2.3.0-to-3.0.0.derby.sql 
1f2647dfbf3263feda9afd98ab6767d7ea3d0557 
  metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql 
75891017584f93f94c55abcae10c512ad6bcb525 
  metastore/scripts/upgrade/mssql/032-HIVE-14498.mssql.sql PRE-CREATION 
  metastore/scripts/upgrade/mssql/hive-schema-3.0.0.mssql.sql 
26c82af74c58cb097df8fa4d36a3b641602e1047 
  metastore/scripts/upgrade/mssql/upgrade-2.3.0-to-3.0.0.mssql.sql 
864a5e5bd5c06810cab9d2f09d3b968845059a7a 
  metastore/scripts/upgrade/mysql/047-HIVE-14498.mysql.sql PRE-CREATION 
  metastore/scripts/upgrade/mysql/hive-schema-3.0.0.mysql.sql 
915af8bf4bb7864371f5b9ba8bfb8a71a064ec36 
  metastore/scripts/upgrade/mysql/hive-txn-schema-3.0.0.mysql.sql 
1df32c4b3548d385e8861b9312042cc25bdf84d7 
  metastore/scripts/upgrade/mysql/upgrade-2.3.0-to-3.0.0.mysql.sql 
caa059d893635e6afadab867760cb035b3a111d4 
  metastore/scripts/upgrade/oracle/047-HIVE-14498.oracle.sql PRE-CREATION 
  metastore/scripts/upgrade/oracle/hive-schema-3.0.0.oracle.sql 
65c72af87343d84fe6eb894c503bebfe54fe4618 
  metastore/scripts/upgrade/oracle/hive-txn-schema-3.0.0.oracle.sql 
12c24a5863e0b8382c643f0404700e0243585db0 
  metastore/scripts/upgrade/oracle/upgrade-2.3.0-to-3.0.0.oracle.sql 
33174c8a9a9bc768919ddda406324eda05aaf313 
  metastore/scripts/upgrade/postgres/046-HIVE-14498.postgres.sql PRE-CREATION 
  metastore/scripts/upgrade/postgres/hive-schema-3.0.0.postgres.sql 
415b5e0189bee95a104e3e3d9cd5f25187260a5b 
  metastore/scripts/upgrade/postgres/hive-txn-schema-3.0.0.postgres.sql 
1fa99aff5fcbbb96ed51b9c02850dfabcd6d3d76 
  metastore/scripts/upgrade/postgres/upgrade-2.3.0-to-3.0.0.postgres.sql 
01d359e5f4c632793ac8e3fb67aa6ea5492dac54 
  ql/src/java/org/apache/hadoop/hive/ql/Context.java 
6d48783d48581fb96ea1b5ded23ce0d549dc80a6 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
55ef8de9a5c7144931d0a6ff13224765ee737fea 
  
ql/src/java/org/apache/hadoop/hive/ql/hooks/MaterializedViewRegistryUpdateHook.java
 a57e4c888b204388b393bb173e2eac91c867137a 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
50bdce89a44a8dc87a97e394d00e5dadebbbd351 
  
ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveMaterializedViewsRegistry.java
 f8825a27caa1391c59ac7389fbc77af6590cc9bb 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 
0debff669a5d64d87bedd1c11a856cf561e46590 
  ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 
efd5f7af151b72dacd620ad6ce94a3a2f5885906 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 
28e3621d3264f4f704da0d775b396f7b7764fdb6 
  ql/src/java/org/apache/hadoop/hive/ql/plan/CreateViewDesc.java 
09aa82f1f0c1a90b08669b91615f26fb1f7cd649 
  ql/src/java/org/apache/hadoop/hive/ql/plan/ImportTableDesc.java 
33e30bf10fc048fdbe2bd4a78e5d6d94bd7b04d1 
  ql/src/java/org/apache/hadoop/hive/ql/stats/BasicStatsTask.java 
69b076a08a70fcea4f262ccbf9e063733ddd25f2 
  ql/src/test/org/apache/hadoop/hive/ql/metadata/TestHive.java 
bbd285d54aeae3073045b2cbbb5ac4c3f6cb6b2b 
  
ql/src/test/queries/clientnegative/materialized_view_no_transactional_rewrite.q 
PRE-CREATION 
  
ql/src/test/queries/clientnegative/materialized_view_no_transactional_rewrite_2.q
 PRE-CREATION 
  

[jira] [Created] (HIVE-18288) merge/concat not support on Acid table

2017-12-15 Thread Eugene Koifman (JIRA)
Eugene Koifman created HIVE-18288:
-

 Summary: merge/concat not support on Acid table
 Key: HIVE-18288
 URL: https://issues.apache.org/jira/browse/HIVE-18288
 Project: Hive
  Issue Type: Sub-task
  Components: Transactions
Reporter: Eugene Koifman
Assignee: Eugene Koifman


For example, mvn test -Dtest=TestCliDriver -Dqfile=orc_merge10.q

now ends up with 
{noformat}
2017-12-15T15:12:30,753 ERROR [7c3ff5b2-285c-44f2-8b13-5c3ccbd41b13 main] 
ql.Driver: FAILED: SemanticException 
org.apache.hadoop.hive.ql.parse.SemanticException: Concatenate/M\
erge can not be performed on transactional tables
org.apache.hadoop.hive.ql.parse.SemanticException: 
org.apache.hadoop.hive.ql.parse.SemanticException: Concatenate/Merge can not be 
performed on transactional tables
at 
org.apache.hadoop.hive.ql.parse.DDLSemanticAnalyzer.analyzeAlterTablePartMergeFiles(DDLSemanticAnalyzer.java:2172)
at 
org.apache.hadoop.hive.ql.parse.DDLSemanticAnalyzer.analyzeInternal(DDLSemanticAnalyzer.java:343)
{noformat}




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


Re: Review Request 64595: HIVE-18257 implement scheduling policy configuration instead of hardcoding fair scheduling

2017-12-15 Thread Sergey Shelukhin

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

(Updated Dec. 15, 2017, 11:21 p.m.)


Review request for hive, Harish Jaiprakash and Prasanth_J.


Changes
---

please ignore iteration 2


Repository: hive-git


Description
---

see jira


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 
f0481fcf25 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
a09b7961c2 
  ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 
98f5c58ab8 
  ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95 
  ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 2c92bb2056 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 cde34bcf42 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a 


Diff: https://reviews.apache.org/r/64595/diff/3/

Changes: https://reviews.apache.org/r/64595/diff/2-3/


Testing
---


Thanks,

Sergey Shelukhin



Re: Review Request 64595: HIVE-18257 implement scheduling policy configuration instead of hardcoding fair scheduling

2017-12-15 Thread Sergey Shelukhin

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

(Updated Dec. 15, 2017, 11:20 p.m.)


Review request for hive, Harish Jaiprakash and Prasanth_J.


Repository: hive-git


Description
---

see jira


Diffs (updated)
-

  itests/src/test/resources/testconfiguration.properties 2bf64dcfbd 
  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 
f0481fcf25 
  ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorGroupKeyHelper.java 
02b0e5c0f6 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
a09b7961c2 
  ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 
98f5c58ab8 
  ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95 
  ql/src/test/queries/clientpositive/vector_reduce_groupby_duplicate_cols.q 
c82c960ce6 
  ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457 
  
ql/src/test/results/clientpositive/llap/vector_reduce_groupby_duplicate_cols.q.out
 afca3df14c 
  ql/src/test/results/clientpositive/vector_reduce_groupby_duplicate_cols.q.out 
eaa4031291 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 2c92bb2056 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java
 cde34bcf42 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a 
  
testutils/ptest2/src/main/java/org/apache/hive/ptest/execution/context/CloudExecutionContextProvider.java
 e80656385e 


Diff: https://reviews.apache.org/r/64595/diff/2/

Changes: https://reviews.apache.org/r/64595/diff/1-2/


Testing
---


Thanks,

Sergey Shelukhin



[jira] [Created] (HIVE-18287) Scratch dir permission check doesn't honor Ranger based privileges

2017-12-15 Thread Thejas M Nair (JIRA)
Thejas M Nair created HIVE-18287:


 Summary: Scratch dir permission check doesn't honor Ranger based 
privileges
 Key: HIVE-18287
 URL: https://issues.apache.org/jira/browse/HIVE-18287
 Project: Hive
  Issue Type: Bug
  Components: HiveServer2, Security
Affects Versions: 1.0.0, 2.4.0
Reporter: Kunal Rajguru


Hiveserver2 needs permission 733 or above on scratch directory to start 
successfully.
HS2 does not take into consideration the permission given to scratch dir via 
Ranger, it expects the permissions at HDFS level.
Even if we give full access to 'hive' user from Ranger , the start of HS2 
fails, it expects to have the permission from HDFS (#hdfs dfs -chmod 755 
/tmp/hive)

>> SessionState.java

{code:java}
private Path createRootHDFSDir(HiveConf conf) throws IOException { 
Path rootHDFSDirPath = new Path(HiveConf.getVar(conf, 
HiveConf.ConfVars.SCRATCHDIR)); 
FsPermission writableHDFSDirPermission = new FsPermission((short)00733); 
FileSystem fs = rootHDFSDirPath.getFileSystem(conf); 
if (!fs.exists(rootHDFSDirPath)) { 
Utilities.createDirsWithPermission(conf, rootHDFSDirPath, 
writableHDFSDirPermission, true); 
} 
FsPermission currentHDFSDirPermission = 
fs.getFileStatus(rootHDFSDirPath).getPermission(); 
if (rootHDFSDirPath != null && rootHDFSDirPath.toUri() != null) { 
String schema = rootHDFSDirPath.toUri().getScheme(); 
LOG.debug( 
"HDFS root scratch dir: " + rootHDFSDirPath + " with schema " + schema + ", 
permission: " + 
currentHDFSDirPermission); 
} else { 
LOG.debug( 
"HDFS root scratch dir: " + rootHDFSDirPath + ", permission: " + 
currentHDFSDirPermission); 
} 
// If the root HDFS scratch dir already exists, make sure it is writeable. 
if (!((currentHDFSDirPermission.toShort() & writableHDFSDirPermission 
.toShort()) == writableHDFSDirPermission.toShort())) { 
throw new RuntimeException("The root scratch dir: " + rootHDFSDirPath 
+ " on HDFS should be writable. Current permissions are: " + 
currentHDFSDirPermission); 
} 
{code}

>> Error message :

{code:java}
2017-08-23 09:56:13,965 WARN [main]: server.HiveServer2 
(HiveServer2.java:startHiveServer2(508)) - Error starting HiveServer2 on 
attempt 1, will retry in 60 seconds 
java.lang.RuntimeException: Error applying authorization policy on hive 
configuration: java.lang.RuntimeException: The root scratch dir: /tmp/hive on 
HDFS should be writable. Current permissions are: rwxr-x--- 
at org.apache.hive.service.cli.CLIService.init(CLIService.java:117) 
at org.apache.hive.service.CompositeService.init(CompositeService.java:59) 
at org.apache.hive.service.server.HiveServer2.init(HiveServer2.java:122) 
at 
org.apache.hive.service.server.HiveServer2.startHiveServer2(HiveServer2.java:474)
 
at org.apache.hive.service.server.HiveServer2.access$700(HiveServer2.java:87) 
at 
org.apache.hive.service.server.HiveServer2$StartOptionExecutor.execute(HiveServer2.java:720)
 
at org.apache.hive.service.server.HiveServer2.main(HiveServer2.java:593) 
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) 
at 
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) 
at 
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
 
at java.lang.reflect.Method.invoke(Method.java:498) 
at org.apache.hadoop.util.RunJar.run(RunJar.java:233) 
at org.apache.hadoop.util.RunJar.main(RunJar.java:148) 
Caused by: java.lang.RuntimeException: java.lang.RuntimeException: The root 
scratch dir: /tmp/hive on HDFS should be writable. Current permissions are: 
rwxr-x--- 
at org.apache.hadoop.hive.ql.session.SessionState.start(SessionState.java:547) 
at 
org.apache.hive.service.cli.CLIService.applyAuthorizationConfigPolicy(CLIService.java:130)
 
at org.apache.hive.service.cli.CLIService.init(CLIService.java:115) 
... 12 more 
Caused by: java.lang.RuntimeException: The root scratch dir: /tmp/hive on HDFS 
should be writable. Current permissions are: rwxr-x--- 
at 
org.apache.hadoop.hive.ql.session.SessionState.createRootHDFSDir(SessionState.java:648)
 
at 
org.apache.hadoop.hive.ql.session.SessionState.createSessionDirs(SessionState.java:580)
 
at org.apache.hadoop.hive.ql.session.SessionState.start(SessionState.java:533) 
... 14 more
{code}




--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


Re: Review Request 64555: HIVE-18230 create plan like plan, and replace plan commands for easy modification

2017-12-15 Thread Sergey Shelukhin

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

(Updated Dec. 15, 2017, 10:16 p.m.)


Review request for hive, Harish Jaiprakash and Prasanth_J.


Repository: hive-git


Description
---

see jira


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
55ef8de9a5c7144931d0a6ff13224765ee737fea 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
50bdce89a44a8dc87a97e394d00e5dadebbbd351 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
1dcfe9dd2834ada2f2bb75bc67e436017c21f71f 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
13789502c235be478659f9267b107e8580594bc8 
  ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
f1ca30109fc40b452f0e1423a586818bcb783796 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 
95c87259c5bdb91a4533d7897d5d32a7c4f56503 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java 
d70d52054dbef5ecbe5be55fd24df25c5381c8d7 
  ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java 
efdd05c5f270e26b84a0e022eac0445ff368969b 
  ql/src/test/queries/clientpositive/resourceplan.q 
fc924a2f95a849b6e14065cb62c2910db40d1b47 
  ql/src/test/results/clientpositive/llap/resourceplan.q.out 
7f3e784457f87ff88c5f746a9fc2b74ea28c2a6e 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 6e0da5781ed8d536852613bad1793d27dd17dfd4 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 fc254c6f5338ebcfe5a19bea847ff8141552f15f 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
 573ac0173dd51b81cb37902cc144983393ab1f51 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 2c92bb20561d09998e400b458036f8e4a960a469 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
 75fbfa23d25c589826a44be493eb84f3cd375e5e 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
 da518ab6e337cbd5befb9f7c1df1781335bd72f4 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 
1085ce566a5fba632ae2acb074ce538143bac6ed 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 24c59f2f1b0b017cff3b26f86c3f903b498d2553 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 1e4fe5d973542d6b0e3ae0760ca5c06ea9888089 


Diff: https://reviews.apache.org/r/64555/diff/2/

Changes: https://reviews.apache.org/r/64555/diff/1-2/


Testing
---


Thanks,

Sergey Shelukhin



Re: Review Request 64555: HIVE-18230 create plan like plan, and replace plan commands for easy modification

2017-12-15 Thread Sergey Shelukhin


> On Dec. 15, 2017, 4:48 a.m., Harish Jaiprakash wrote:
> > standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
> > Lines 9869 (patched)
> > 
> >
> > Hmm... if people use replace plan frequently as a way of updating an 
> > existing plan then this list will grow long and we'll be making one call to 
> > db per rp. Would using a like query to get all the names and using a set to 
> > store the name make it faster?
> > 
> > Another way would be to use as version field used in conjunction with 
> > name to create a unique field. Since we want to implement version sometime 
> > this might be useful.

We can implement versioning in phase 2 :) 
As for the long list, note that the versioned name includes a timestamp, so in 
realistic use cases, we will only check once and there won't be a match.


- Sergey


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


On Dec. 12, 2017, 9:46 p.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64555/
> ---
> 
> (Updated Dec. 12, 2017, 9:46 p.m.)
> 
> 
> Review request for hive, Harish Jaiprakash and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> see jira
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  62c9172ef5d9ae74b158d1b4f1b8c5c0eca4e375 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 
> 55ef8de9a5c7144931d0a6ff13224765ee737fea 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 
> 50bdce89a44a8dc87a97e394d00e5dadebbbd351 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> a09b7961c2dbc26b4d2fa912d0be7037885f63e4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 
> 1dcfe9dd2834ada2f2bb75bc67e436017c21f71f 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 
> 13789502c235be478659f9267b107e8580594bc8 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 
> f1ca30109fc40b452f0e1423a586818bcb783796 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 
> 95c87259c5bdb91a4533d7897d5d32a7c4f56503 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java 
> d70d52054dbef5ecbe5be55fd24df25c5381c8d7 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateResourcePlanDesc.java 
> efdd05c5f270e26b84a0e022eac0445ff368969b 
>   ql/src/test/queries/clientpositive/resourceplan.q 
> fc924a2f95a849b6e14065cb62c2910db40d1b47 
>   ql/src/test/results/clientpositive/llap/resourceplan.q.out 
> 7f3e784457f87ff88c5f746a9fc2b74ea28c2a6e 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  6e0da5781ed8d536852613bad1793d27dd17dfd4 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  fc254c6f5338ebcfe5a19bea847ff8141552f15f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  573ac0173dd51b81cb37902cc144983393ab1f51 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  2c92bb20561d09998e400b458036f8e4a960a469 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  75fbfa23d25c589826a44be493eb84f3cd375e5e 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  da518ab6e337cbd5befb9f7c1df1781335bd72f4 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 
> 1085ce566a5fba632ae2acb074ce538143bac6ed 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  24c59f2f1b0b017cff3b26f86c3f903b498d2553 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1e4fe5d973542d6b0e3ae0760ca5c06ea9888089 
> 
> 
> Diff: https://reviews.apache.org/r/64555/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>



Re: Review Request 64433: HIVE-18203 change the way WM is enabled and allow dropping the last resource plan

2017-12-15 Thread Sergey Shelukhin

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

(Updated Dec. 15, 2017, 9:19 p.m.)


Review request for hive, Harish Jaiprakash and Prasanth_J.


Repository: hive-git


Description
---

see jira


Diffs (updated)
-

  
itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
 62c9172ef5 
  ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 55ef8de9a5 
  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 
3c1b8d080c 
  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 
f0481fcf25 
  ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 50bdce89a4 
  ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
a09b7961c2 
  ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 1dcfe9dd28 
  ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 95c87259c5 
  ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java 
d70d52054d 
  ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 
98f5c58ab8 
  ql/src/test/queries/clientpositive/resourceplan.q fc924a2f95 
  ql/src/test/results/clientpositive/llap/resourceplan.q.out 7f3e784457 
  service/src/java/org/apache/hive/service/server/HiveServer2.java 223be6aa32 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
 6e0da5781e 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
 fc254c6f53 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
 573ac0173d 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
 2c92bb2056 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
 75fbfa23d2 
  
standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
 da518ab6e3 
  standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
 24c59f2f1b 
  
standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
 1e4fe5d973 


Diff: https://reviews.apache.org/r/64433/diff/2/

Changes: https://reviews.apache.org/r/64433/diff/1-2/


Testing
---


Thanks,

Sergey Shelukhin



Re: Review Request 64433: HIVE-18203 change the way WM is enabled and allow dropping the last resource plan

2017-12-15 Thread Sergey Shelukhin


> On Dec. 11, 2017, 4:56 a.m., Harish Jaiprakash wrote:
> > ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
> > Lines 939 (patched)
> > 
> >
> > Yup, seem unintuitive to use enable in one and disable in another 
> > command. Is it better to add an extension to ALTER RESOURCE PLAN 
> > `plan_name` FORCE_DEACTIVATE;

I feel like this is error prone... I wanted to make it explicit to the admin 
that WM is being disabled, so they don't deactivate the plan just to edit it a 
little bit and throw everything into chaos.


- Sergey


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


On Dec. 8, 2017, 1:11 a.m., Sergey Shelukhin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64433/
> ---
> 
> (Updated Dec. 8, 2017, 1:11 a.m.)
> 
> 
> Review request for hive, Harish Jaiprakash and Prasanth_J.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> see jira
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  62c9172ef5 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 55ef8de9a5 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/TezSessionPoolManager.java 
> 8417ebb7d5 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 
> dbdbbf25db 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 50bdce89a4 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> a09b7961c2 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 1dcfe9dd28 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ResourcePlanParser.g 95c87259c5 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/AlterResourcePlanDesc.java 
> d70d52054d 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 
> c58e4507f2 
>   ql/src/test/queries/clientpositive/resourceplan.q 002b21c1b9 
>   ql/src/test/results/clientpositive/llap/resourceplan.q.out 093e5d58b6 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 223be6aa32 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  6e0da5781e 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  fc254c6f53 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  573ac0173d 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  2e80c9d3b1 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  75fbfa23d2 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  da518ab6e3 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 1085ce566a 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  24c59f2f1b 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1e4fe5d973 
> 
> 
> Diff: https://reviews.apache.org/r/64433/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Sergey Shelukhin
> 
>



Re: [DISCUSS] IMetaStoreClient and HMS Thrift API exception handling

2017-12-15 Thread Alan Gates
Sorry, I didn’t make clear which suggestion I was responding to, as there
are multiple suggestions in the thread.

​Adding new exceptions is fine, as long as it doesn’t break old clients
like Thejas mentions.  And clearly cleaning up NPEs is good.  Changing
which exceptions are thrown when a particular error is hit (e.g. throwing
InvalidObjectException when a table is created in a non-existent database
rather than the more reasonable NoSuchObject exception) I am less sure of,
as users have probably adapted to the existing behavior.

My earlier response was to the larger suggestions that would remove or
change methods, etc.

I agree we need to be thinking about a cleaner, leaner, and more rational
V2 of the API.  We could then put new functionality in V2 and maintain the
existing one for backwards compatibility.

The first question I’d ask on V2 is, should we stick with Thrift?  What if
we switched to REST?  Or Google RPC?  Or something else.  I’m not saying we
should switch, but it’s worth thinking through.  (Switching would come with
a high cost, since we pass all the objects around as thrift objects
internally, so it may not be worth it.  But that’s discussion we can have
when we start designing API V2…)

Alan.

On Fri, Dec 15, 2017 at 10:42 AM, Thejas Nair  wrote:

> Alan,
> The changes suggested by Peter was to add another checked exception, which
> is a subclass of TException. And TException is already being thrown by all
> thrift api calls. So it should not break any applications.
> The only concern I have is some issues if old client is used with new
> servers. We need to verifiy that the old client is able to deserialize a
> response with new exceptions.
>
>
> On Fri, Dec 15, 2017 at 7:33 AM, Alan Gates  wrote:
>
> > We do not want to break either the Thrift APIs or the IMetaStoreClient
> > ones.  I do not know if we have ever declared them to be public or not,
> but
> > they are de facto public in that everyone uses them.  Consider that these
> > are used by Impala, Presto, Qubole, Amazon's Glue, Spark, and probably
> > others we don't know about.  If we produce a new version that does not
> > maintain those APIs everyone will just ignore it and stay on the older
> > version (just like python 3).  I would even include changing exception
> > types in this.
> >
> > I would like to have a clean API with consistent error handling and that
> > does not include 37 different ways to fetch partitions (I exaggerate only
> > slightly), but we need to find a way to do it that does not force users
> to
> > rewrite their application to upgrade to the standalone metastore.
> >
> > Alan.
> >
> >
> > On Fri, Dec 15, 2017 at 2:20 AM, Peter Vary  wrote:
> >
> > > If the application using the old API does not handle the original
> > > exceptions differently than the TExceptions then it should work as
> > expected.
> > >
> > > > On Dec 14, 2017, at 10:50 PM, Thejas Nair 
> > wrote:
> > > >
> > > > This direction looks good to me.
> > > > If the new exceptions are inheriting from TException the applications
> > > would
> > > > still work. But would it still work if we old metastore client
> library
> > is
> > > > used with a newer version of metastore server running with these
> > changes
> > > ?
> > > >
> > > >
> > > > On Wed, Dec 13, 2017 at 4:02 AM, Peter Vary 
> > wrote:
> > > >
> > > >> Hi Team,
> > > >>
> > > >> Since the work begin to separate the HMS to a standalone project we
> > > >> thought that it would be useful the create extensive API tests for
> the
> > > >> public APIs of the new project.
> > > >> We started to create tests using the IMetaStoreClient interface
> > > >> implementations and found that not surprisingly the happy paths are
> > > working
> > > >> as expected, but there are some gaps in the exception handling. Most
> > of
> > > the
> > > >> enhancements affect the Thrift API as well.
> > > >> We went through the following methods:
> > > >> Functions
> > > >> Indexes
> > > >> Tables
> > > >> Databases
> > > >> We also plan to comb through at least the Partition related methods
> > too.
> > > >>
> > > >> The possible enhancements we found could be grouped to the following
> > > >> categories:
> > > >> Bugs: For example IMetaStoreClient.drop/alter/createFunction with
> > null
> > > >> function name might throw NullPointerException exception - this is
> > > clearly
> > > >> a bug which should be solved:
> > > >> Embedded MetaStore throws NullPointerException
> > > >> Remote MetaStore client throws TTransportException
> > > >> Sub-optimal error handling: For example
> IMetaStoreClient.alterFunction
> > > >> will not check if the new function is already exist, and tries to
> > > insert it
> > > >> anyway. After 10 tries it throws a MetaException where the exception
> > > text
> > > >> is "Update of object [...] failed : java.sql.
> > > >> SQLIntegrityConstraintViolationException [...]". 

[jira] [Created] (HIVE-18286) java.lang.ClassCastException: org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector cannot be cast to org.apache.hadoop.hive.ql.exec.vector.LongColumnVector

2017-12-15 Thread Eugene Koifman (JIRA)
Eugene Koifman created HIVE-18286:
-

 Summary: java.lang.ClassCastException: 
org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector cannot be cast to 
org.apache.hadoop.hive.ql.exec.vector.LongColumnVector
 Key: HIVE-18286
 URL: https://issues.apache.org/jira/browse/HIVE-18286
 Project: Hive
  Issue Type: Sub-task
  Components: Transactions
Affects Versions: 3.0.0
Reporter: Eugene Koifman
Assignee: Eugene Koifman


{noformat}
mvn test -Dtest=TestCliDriver -Dqfile=vector_outer_join3.q

create table small_alltypesorc1a as 
select * from alltypesorc 
where cint is not null and cstring1 is not null 
order by ctinyint, csmallint, cint, cbigint, cfloat, cdouble, cstring1, 
cstring2, ctimestamp1, ctimestamp2, cboolean1, cboolean2
 limit 5;
{noformat}

{noformat}
2017-12-14T14:33:28,633  WARN [Thread-2754] mapred.LocalJobRunner: 
job_local113844877_0036
java.lang.Exception: java.io.IOException: java.lang.ClassCastException: 
org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector cannot be cast to 
org.apache.hadoop.hive.ql.exec.vector.LongColumnVector
at 
org.apache.hadoop.mapred.LocalJobRunner$Job.runTasks(LocalJobRunner.java:492) 
~[hadoop-mapreduce-client-common-3.0.0-beta1.jar:?]
at 
org.apache.hadoop.mapred.LocalJobRunner$Job.run(LocalJobRunner.java:552) 
[hadoop-mapreduce-client-common-3.0.0-beta1.jar:?]
Caused by: java.io.IOException: java.lang.ClassCastException: 
org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector cannot be cast to 
org.apache.hadoop.hive.ql.exec.vector.LongColumnVector
at 
org.apache.hadoop.hive.io.HiveIOExceptionHandlerChain.handleRecordReaderNextException(HiveIOExceptionHandlerChain.java:121)
 ~[hive-shims-common-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.io.HiveIOExceptionHandlerUtil.handleRecordReaderNextException(HiveIOExceptionHandlerUtil.java:77)
 ~[hive-shims-common-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.HiveContextAwareRecordReader.doNext(HiveContextAwareRecordReader.java:365)
 ~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.HiveRecordReader.doNext(HiveRecordReader.java:79) 
~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.HiveRecordReader.doNext(HiveRecordReader.java:33) 
~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.HiveContextAwareRecordReader.next(HiveContextAwareRecordReader.java:116)
 ~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.mapred.MapTask$TrackedRecordReader.moveToNext(MapTask.java:199)
 ~[hadoop-mapreduce-client-core-3.0.0-beta1.jar:?]
at 
org.apache.hadoop.mapred.MapTask$TrackedRecordReader.next(MapTask.java:185) 
~[hadoop-mapreduce-client-core-3.0.0-beta1.jar:?]
at org.apache.hadoop.mapred.MapRunner.run(MapRunner.java:52) 
~[hadoop-mapreduce-client-core-3.0.0-beta1.jar:?]
at org.apache.hadoop.mapred.MapTask.runOldMapper(MapTask.java:459) 
~[hadoop-mapreduce-client-core-3.0.0-beta1.jar:?]
at org.apache.hadoop.mapred.MapTask.run(MapTask.java:343) 
~[hadoop-mapreduce-client-core-3.0.0-beta1.jar:?]
at 
org.apache.hadoop.mapred.LocalJobRunner$Job$MapTaskRunnable.run(LocalJobRunner.java:271)
 ~[hadoop-mapreduce-client-common-3.0.0-beta1.jar:?]
at 
java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) 
~[?:1.8.0_25]
at java.util.concurrent.FutureTask.run(FutureTask.java:266) 
~[?:1.8.0_25]
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) 
~[?:1.8.0_25]
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) 
~[?:1.8.0_25]
at java.lang.Thread.run(Thread.java:745) ~[?:1.8.0_25]
Caused by: java.lang.ClassCastException: 
org.apache.hadoop.hive.ql.exec.vector.DoubleColumnVector cannot be cast to 
org.apache.hadoop.hive.ql.exec.vector.LongColumnVector
at 
org.apache.hadoop.hive.ql.io.orc.VectorizedOrcAcidRowBatchReader.findRecordsWithInvalidTransactionIds(VectorizedOrcAcidRowBatchReader.java:531)
 ~[hive-exec-3.0.0\
-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.orc.VectorizedOrcAcidRowBatchReader.next(VectorizedOrcAcidRowBatchReader.java:462)
 ~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.orc.VectorizedOrcAcidRowBatchReader.next(VectorizedOrcAcidRowBatchReader.java:62)
 ~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.HiveContextAwareRecordReader.doNext(HiveContextAwareRecordReader.java:360)
 ~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 
org.apache.hadoop.hive.ql.io.HiveRecordReader.doNext(HiveRecordReader.java:79) 
~[hive-exec-3.0.0-SNAPSHOT.jar:3.0.0-SNAPSHOT]
at 

Re: Review Request 64356: HIVE-18078 WM getSession needs some retry logic

2017-12-15 Thread Sergey Shelukhin

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

(Updated Dec. 15, 2017, 7:07 p.m.)


Review request for hive and Prasanth_J.


Repository: hive-git


Description
---

see jira


Diffs (updated)
-

  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/WorkloadManager.java 
f0481fcf25 
  ql/src/test/org/apache/hadoop/hive/ql/exec/tez/TestWorkloadManager.java 
98f5c58ab8 


Diff: https://reviews.apache.org/r/64356/diff/2/

Changes: https://reviews.apache.org/r/64356/diff/1-2/


Testing
---


Thanks,

Sergey Shelukhin



Re: [DISCUSS] IMetaStoreClient and HMS Thrift API exception handling

2017-12-15 Thread Thejas Nair
Alan,
The changes suggested by Peter was to add another checked exception, which
is a subclass of TException. And TException is already being thrown by all
thrift api calls. So it should not break any applications.
The only concern I have is some issues if old client is used with new
servers. We need to verifiy that the old client is able to deserialize a
response with new exceptions.


On Fri, Dec 15, 2017 at 7:33 AM, Alan Gates  wrote:

> We do not want to break either the Thrift APIs or the IMetaStoreClient
> ones.  I do not know if we have ever declared them to be public or not, but
> they are de facto public in that everyone uses them.  Consider that these
> are used by Impala, Presto, Qubole, Amazon's Glue, Spark, and probably
> others we don't know about.  If we produce a new version that does not
> maintain those APIs everyone will just ignore it and stay on the older
> version (just like python 3).  I would even include changing exception
> types in this.
>
> I would like to have a clean API with consistent error handling and that
> does not include 37 different ways to fetch partitions (I exaggerate only
> slightly), but we need to find a way to do it that does not force users to
> rewrite their application to upgrade to the standalone metastore.
>
> Alan.
>
>
> On Fri, Dec 15, 2017 at 2:20 AM, Peter Vary  wrote:
>
> > If the application using the old API does not handle the original
> > exceptions differently than the TExceptions then it should work as
> expected.
> >
> > > On Dec 14, 2017, at 10:50 PM, Thejas Nair 
> wrote:
> > >
> > > This direction looks good to me.
> > > If the new exceptions are inheriting from TException the applications
> > would
> > > still work. But would it still work if we old metastore client library
> is
> > > used with a newer version of metastore server running with these
> changes
> > ?
> > >
> > >
> > > On Wed, Dec 13, 2017 at 4:02 AM, Peter Vary 
> wrote:
> > >
> > >> Hi Team,
> > >>
> > >> Since the work begin to separate the HMS to a standalone project we
> > >> thought that it would be useful the create extensive API tests for the
> > >> public APIs of the new project.
> > >> We started to create tests using the IMetaStoreClient interface
> > >> implementations and found that not surprisingly the happy paths are
> > working
> > >> as expected, but there are some gaps in the exception handling. Most
> of
> > the
> > >> enhancements affect the Thrift API as well.
> > >> We went through the following methods:
> > >> Functions
> > >> Indexes
> > >> Tables
> > >> Databases
> > >> We also plan to comb through at least the Partition related methods
> too.
> > >>
> > >> The possible enhancements we found could be grouped to the following
> > >> categories:
> > >> Bugs: For example IMetaStoreClient.drop/alter/createFunction with
> null
> > >> function name might throw NullPointerException exception - this is
> > clearly
> > >> a bug which should be solved:
> > >> Embedded MetaStore throws NullPointerException
> > >> Remote MetaStore client throws TTransportException
> > >> Sub-optimal error handling: For example IMetaStoreClient.alterFunction
> > >> will not check if the new function is already exist, and tries to
> > insert it
> > >> anyway. After 10 tries it throws a MetaException where the exception
> > text
> > >> is "Update of object [...] failed : java.sql.
> > >> SQLIntegrityConstraintViolationException [...]". Fixing this could be
> > >> done without interface change, but following the logic of the other
> > methods
> > >> on the interface AlreadyExistsException should be thrown.
> > >> Inconsistent exception handling: Different methods will throw
> different
> > >> exceptions for similar errors. This makes the interface hard to
> > understand,
> > >> hard to document and maintain. For example:
> > >> Calling IMetaStoreClient.createTable with nonexistent database name
> will
> > >> throw InvalidObjectException
> > >> Calling IMetaStoreClient.createFunction with nonexistent database
> name
> > >> database will throw NoSuchObjectException
> > >> There are some cases when the Embedded MetaStore handles error
> > differently
> > >> than the Remote MetaStore. For example: IMetaStoreClient.dropTable
> with
> > >> "null" as a database:
> > >> Embedded MetaStore throws MetaException
> > >> Remote MetaStore client throws TProtocolException
> > >>
> > >> Proposed changes:
> > >> Fixing cases 1. and 2. is a simple bug fix - it could be done
> > independently
> > >> Fixing cases 3. and 4. will change how the IMetaStoreClient and HMS
> > Thrift
> > >> API works. For these we should review the IMetaStoreClient and HMS
> > Thrift
> > >> API interface exception handling, to create consistent and easy to
> > follow
> > >> rules for the possible exceptions. We propose to keep the current
> > >> exceptions, and only change when the given type of exceptions are
> > thrown.
> > >> If we stick to 

Re: Review Request 64631: HIVE-18273 add LLAP-level counters for WM

2017-12-15 Thread Sergey Shelukhin

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

(Updated Dec. 15, 2017, 6:36 p.m.)


Review request for hive and Prasanth_J.


Repository: hive-git


Description
---

see jira.


Diffs (updated)
-

  llap-common/src/java/org/apache/hadoop/hive/llap/counters/LlapWmCounters.java 
PRE-CREATION 
  
llap-server/src/java/org/apache/hadoop/hive/llap/counters/WmFragmentCounters.java
 PRE-CREATION 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/ContainerRunnerImpl.java
 f0a26eb258 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorService.java
 c0356af45a 
  
llap-server/src/java/org/apache/hadoop/hive/llap/daemon/impl/TaskRunnerCallable.java
 87a692f212 
  llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapIoImpl.java 
77c8ade5c9 
  
llap-server/src/java/org/apache/hadoop/hive/llap/io/api/impl/LlapRecordReader.java
 5f010bed8d 
  
llap-server/src/test/org/apache/hadoop/hive/llap/daemon/impl/TaskExecutorTestHelpers.java
 078420dc8a 
  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/LlapWmSummary.java 
PRE-CREATION 
  ql/src/java/org/apache/hadoop/hive/ql/exec/tez/monitoring/TezJobMonitor.java 
5ade1f3425 


Diff: https://reviews.apache.org/r/64631/diff/2/

Changes: https://reviews.apache.org/r/64631/diff/1-2/


Testing
---


Thanks,

Sergey Shelukhin



[jira] [Created] (HIVE-18285) StatsTask uses a cached ql.metadata.Table object

2017-12-15 Thread Eugene Koifman (JIRA)
Eugene Koifman created HIVE-18285:
-

 Summary: StatsTask uses a cached ql.metadata.Table object
 Key: HIVE-18285
 URL: https://issues.apache.org/jira/browse/HIVE-18285
 Project: Hive
  Issue Type: Bug
  Components: Metastore, Statistics
Reporter: Eugene Koifman
Assignee: Eugene Koifman


this then causes BasicStatsTask.aggregateStats(Hive) to call Hive.alterTable() 
with a stale Table object.  (It misses any changes made by any 
MetaStorePreEventListener)



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


Re: [DISCUSS] IMetaStoreClient and HMS Thrift API exception handling

2017-12-15 Thread Peter Vary
Hi Alan,

I think I understand your point. If we come up with the standalone MetaStore 
but the interface is completely different then the original one, then we will 
lose in adoption. I agree with your assessment.

Do you (or anyone) have any ideas, how can we rationalize the interface without 
disrupting the use cases you mentioned?
Sooner or later maintaining those 37 different ways to fetch partitions will be 
a huge burden, especially if we start adding new features like HIVE-17990, and 
hopefully others that come after that.
For example in Hadoop and Spark I see that there are "deprecated" methods which 
are getting removed after major releases. Do we have plans for this?

There are some boundaries which I did not feel yet, so I would like to know how 
the more experienced Hive developers feel about what constitutes a breaking 
change?
Is it a breaking change, if for a specific error the API was throwing 
InvalidObjectException, but after the change it will throw 
NoSuchObjectException -even if the method declaration does not need to be 
change for this?
Is it acceptable if we add a configuration value to the MetaStore to 
disable/enable throwing new exceptions? Like if "compatibility mode" is true, 
then the old exceptions are thrown, if it is false, then the new exceptions are 
thrown.
Note, that the complexity of the 2. version is only worth it, if we plan to 
adapt something like the Hadoop model, and sooner or later deprecate the old 
versions.

Thanks for joining the conversation, and highlighting important constraints!
Peter



> On Dec 15, 2017, at 4:33 PM, Alan Gates  wrote:
> 
> We do not want to break either the Thrift APIs or the IMetaStoreClient
> ones.  I do not know if we have ever declared them to be public or not, but
> they are de facto public in that everyone uses them.  Consider that these
> are used by Impala, Presto, Qubole, Amazon's Glue, Spark, and probably
> others we don't know about.  If we produce a new version that does not
> maintain those APIs everyone will just ignore it and stay on the older
> version (just like python 3).  I would even include changing exception
> types in this.
> 
> I would like to have a clean API with consistent error handling and that
> does not include 37 different ways to fetch partitions (I exaggerate only
> slightly), but we need to find a way to do it that does not force users to
> rewrite their application to upgrade to the standalone metastore.
> 
> Alan.
> 
> 
> On Fri, Dec 15, 2017 at 2:20 AM, Peter Vary  wrote:
> 
>> If the application using the old API does not handle the original
>> exceptions differently than the TExceptions then it should work as expected.
>> 
>>> On Dec 14, 2017, at 10:50 PM, Thejas Nair  wrote:
>>> 
>>> This direction looks good to me.
>>> If the new exceptions are inheriting from TException the applications
>> would
>>> still work. But would it still work if we old metastore client library is
>>> used with a newer version of metastore server running with these changes
>> ?
>>> 
>>> 
>>> On Wed, Dec 13, 2017 at 4:02 AM, Peter Vary  wrote:
>>> 
 Hi Team,
 
 Since the work begin to separate the HMS to a standalone project we
 thought that it would be useful the create extensive API tests for the
 public APIs of the new project.
 We started to create tests using the IMetaStoreClient interface
 implementations and found that not surprisingly the happy paths are
>> working
 as expected, but there are some gaps in the exception handling. Most of
>> the
 enhancements affect the Thrift API as well.
 We went through the following methods:
 Functions
 Indexes
 Tables
 Databases
 We also plan to comb through at least the Partition related methods too.
 
 The possible enhancements we found could be grouped to the following
 categories:
 Bugs: For example IMetaStoreClient.drop/alter/createFunction with null
 function name might throw NullPointerException exception - this is
>> clearly
 a bug which should be solved:
 Embedded MetaStore throws NullPointerException
 Remote MetaStore client throws TTransportException
 Sub-optimal error handling: For example IMetaStoreClient.alterFunction
 will not check if the new function is already exist, and tries to
>> insert it
 anyway. After 10 tries it throws a MetaException where the exception
>> text
 is "Update of object [...] failed : java.sql.
 SQLIntegrityConstraintViolationException [...]". Fixing this could be
 done without interface change, but following the logic of the other
>> methods
 on the interface AlreadyExistsException should be thrown.
 Inconsistent exception handling: Different methods will throw different
 exceptions for similar errors. This makes the interface hard to
>> understand,
 hard to document and maintain. For example:
 Calling 

Re: Adding Hive Metastore functions to add and alter partitions for multiple tables

2017-12-15 Thread Andrew Sherman
Thanks Kaijie.

One concern is that the new functions effectively expand the size of the
transactions and the work that must be undone if they fail. So the question
is whether the benefit is large enough to justify adding complexity.

If no-one else has comments then you should probably think about having
people look at the code.

-Andrew

On Wed, Dec 13, 2017 at 6:54 PM, 秦凯捷  wrote:

> Hi Andrew,
>
> Thanks for you response. For your comments:
>
> -Functionality:
> Support adding and altering multiple partitions for multiple tables in one
> SQL and API request as one transaction.
>
> - what happens in the case of a failure when part way through the
> operations.
> For altering and adding partitions, all the objectstore changes for
> partitions will be operated in one transaction. So the transaction will be
> roll-back in case of failure.
> For adding partitions, there may be additional steps to add directories on
> filesystem for newly added partitions. They will be deleted in case of
> failure, just like what AddPartitions is doing now.
>
> - what impact on the system there will be if an operation takes a long time
> Alter partitions for multiple tables actually has no big difference than
> current altering partitions for one table. They will both take a long time
> if someone is trying to alter too many partitions or for too many tables.
> Transaction timeout will strike down the operation.
> We are doing performance test on our system to see how long it takes for
> multiple scenarios but after all, this should not be a blocker.
>
> Thanks,
> Kaijie
>
> 秦凯捷
> Tel: +86-13810485829
> E-mail: daniel...@gmail.com
>
>
>
> On Thu, Dec 14, 2017 at 3:38 AM, Andrew Sherman 
> wrote:
>
> > Hi Kaijie,
> >
> > I think this is an area that other the Hive community is interested in.
> So
> > please do go ahead and describe your functionality.
> > I think that it is important to describe
> > - what happens in the case of a failure when part way through the
> > operations.
> > - what impact on the system there will be if an operation takes a long
> time
> >
> > Thanks
> >
> > -Andrew
> >
> > On Tue, Dec 12, 2017 at 1:31 AM, 秦凯捷  wrote:
> >
> > > Hi dev,
> > >
> > > I'm wondering if Hive community have ever considered support adding and
> > > altering multiple partitions for multiple tables?
> > >
> > > I'm using Hive Metastore to manage the metadata for Presto querying.
> Our
> > > business requires that we should publish some partitions of data for
> > > multiple tables at the same time in an atomic transaction to keep the
> > data
> > > consistency. Currently Hive Metastore only supports adding and altering
> > > multiple tables for one table.
> > >
> > > I drafted AddPartitionsForTables and AlterPartitionsForTables function
> to
> > > achieve this based on existing AddPartition and AlterPartition logic
> and
> > we
> > > are testing it on our system.
> > > I'm wondering if community have considered these functionality. I would
> > > like to contribute the functionality if you have interest.
> > >
> > > Thank you!
> > > -Kaijie
> > >
> > >
> > > Tel: +86-13810485829
> > > E-mail: daniel...@gmail.com
> > >
> >
>


[jira] [Created] (HIVE-18284) NPE when inserting data with 'distribute by'

2017-12-15 Thread Aki Tanaka (JIRA)
Aki Tanaka created HIVE-18284:
-

 Summary: NPE when inserting data with 'distribute by'
 Key: HIVE-18284
 URL: https://issues.apache.org/jira/browse/HIVE-18284
 Project: Hive
  Issue Type: Bug
  Components: Query Processor
Affects Versions: 2.3.2, 2.3.1
Reporter: Aki Tanaka


A Null Pointer Exception occurs when inserting data with 'distribute by' 
clause. The following snippet query reproduces this issue:


{code:java}
create table table1 (col1 string, datekey int);
insert into table1 values ('ROW1', 1), ('ROW2', 2), ('ROW3', 1);
create table table2 (col1 string) partitioned by (datekey int);

set hive.exec.dynamic.partition.mode=nonstrict;
insert into table table2
PARTITION(datekey)
select col1,
datekey
from table1
distribute by datekey ;
{code}

I could run the insert query without the error if I remove Distribute By  or 
use Cluster By clause.
It seems that the issue happens because Distribute By does not guarantee 
clustering or sorting properties on the distributed keys.

FileSinkOperator removes the previous fsp. FileSinkOperator will remove the 
previous fsp which might be re-used when we use Distribute By.
https://github.com/apache/hive/blob/branch-2.3/ql/src/java/org/apache/hadoop/hive/ql/exec/FileSinkOperator.java#L972

The following stack trace is logged.

{code:java}
Vertex failed, vertexName=Reducer 2, vertexId=vertex_1513111717879_0056_1_01, 
diagnostics=[Task failed, taskId=task_1513111717879_0056_1_01_00, 
diagnostics=[TaskAttempt 0 failed, info=[Error: Error while running task ( 
failure ) : 
attempt_1513111717879_0056_1_01_00_0:java.lang.RuntimeException: 
org.apache.hadoop.hive.ql.metadata.HiveException: Hive Runtime Error while 
processing row (tag=0) {"key":{},"value":{"_col0":"ROW3","_col1":1}}
at 
org.apache.hadoop.hive.ql.exec.tez.TezProcessor.initializeAndRunProcessor(TezProcessor.java:211)
at 
org.apache.hadoop.hive.ql.exec.tez.TezProcessor.run(TezProcessor.java:168)
at 
org.apache.tez.runtime.LogicalIOProcessorRuntimeTask.run(LogicalIOProcessorRuntimeTask.java:370)
at 
org.apache.tez.runtime.task.TaskRunner2Callable$1.run(TaskRunner2Callable.java:73)
at 
org.apache.tez.runtime.task.TaskRunner2Callable$1.run(TaskRunner2Callable.java:61)
at java.security.AccessController.doPrivileged(Native Method)
at javax.security.auth.Subject.doAs(Subject.java:422)
at 
org.apache.hadoop.security.UserGroupInformation.doAs(UserGroupInformation.java:1698)
at 
org.apache.tez.runtime.task.TaskRunner2Callable.callInternal(TaskRunner2Callable.java:61)
at 
org.apache.tez.runtime.task.TaskRunner2Callable.callInternal(TaskRunner2Callable.java:37)
at org.apache.tez.common.CallableWithNdc.call(CallableWithNdc.java:36)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at 
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at 
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
at java.lang.Thread.run(Thread.java:748)
Caused by: org.apache.hadoop.hive.ql.metadata.HiveException: Hive Runtime Error 
while processing row (tag=0) {"key":{},"value":{"_col0":"ROW3","_col1":1}}
at 
org.apache.hadoop.hive.ql.exec.tez.ReduceRecordSource$GroupIterator.next(ReduceRecordSource.java:365)
at 
org.apache.hadoop.hive.ql.exec.tez.ReduceRecordSource.pushRecord(ReduceRecordSource.java:250)
at 
org.apache.hadoop.hive.ql.exec.tez.ReduceRecordProcessor.run(ReduceRecordProcessor.java:317)
at 
org.apache.hadoop.hive.ql.exec.tez.TezProcessor.initializeAndRunProcessor(TezProcessor.java:185)
... 14 more
Caused by: java.lang.NullPointerException
at 
org.apache.hadoop.hive.ql.exec.FileSinkOperator.process(FileSinkOperator.java:762)
at org.apache.hadoop.hive.ql.exec.Operator.forward(Operator.java:897)
at 
org.apache.hadoop.hive.ql.exec.SelectOperator.process(SelectOperator.java:95)
at 
org.apache.hadoop.hive.ql.exec.tez.ReduceRecordSource$GroupIterator.next(ReduceRecordSource.java:356)
... 17 more
{code}





--
This message was sent by Atlassian JIRA
(v6.4.14#64029)


Re: [DISCUSS] IMetaStoreClient and HMS Thrift API exception handling

2017-12-15 Thread Alan Gates
We do not want to break either the Thrift APIs or the IMetaStoreClient
ones.  I do not know if we have ever declared them to be public or not, but
they are de facto public in that everyone uses them.  Consider that these
are used by Impala, Presto, Qubole, Amazon's Glue, Spark, and probably
others we don't know about.  If we produce a new version that does not
maintain those APIs everyone will just ignore it and stay on the older
version (just like python 3).  I would even include changing exception
types in this.

I would like to have a clean API with consistent error handling and that
does not include 37 different ways to fetch partitions (I exaggerate only
slightly), but we need to find a way to do it that does not force users to
rewrite their application to upgrade to the standalone metastore.

Alan.


On Fri, Dec 15, 2017 at 2:20 AM, Peter Vary  wrote:

> If the application using the old API does not handle the original
> exceptions differently than the TExceptions then it should work as expected.
>
> > On Dec 14, 2017, at 10:50 PM, Thejas Nair  wrote:
> >
> > This direction looks good to me.
> > If the new exceptions are inheriting from TException the applications
> would
> > still work. But would it still work if we old metastore client library is
> > used with a newer version of metastore server running with these changes
> ?
> >
> >
> > On Wed, Dec 13, 2017 at 4:02 AM, Peter Vary  wrote:
> >
> >> Hi Team,
> >>
> >> Since the work begin to separate the HMS to a standalone project we
> >> thought that it would be useful the create extensive API tests for the
> >> public APIs of the new project.
> >> We started to create tests using the IMetaStoreClient interface
> >> implementations and found that not surprisingly the happy paths are
> working
> >> as expected, but there are some gaps in the exception handling. Most of
> the
> >> enhancements affect the Thrift API as well.
> >> We went through the following methods:
> >> Functions
> >> Indexes
> >> Tables
> >> Databases
> >> We also plan to comb through at least the Partition related methods too.
> >>
> >> The possible enhancements we found could be grouped to the following
> >> categories:
> >> Bugs: For example IMetaStoreClient.drop/alter/createFunction with null
> >> function name might throw NullPointerException exception - this is
> clearly
> >> a bug which should be solved:
> >> Embedded MetaStore throws NullPointerException
> >> Remote MetaStore client throws TTransportException
> >> Sub-optimal error handling: For example IMetaStoreClient.alterFunction
> >> will not check if the new function is already exist, and tries to
> insert it
> >> anyway. After 10 tries it throws a MetaException where the exception
> text
> >> is "Update of object [...] failed : java.sql.
> >> SQLIntegrityConstraintViolationException [...]". Fixing this could be
> >> done without interface change, but following the logic of the other
> methods
> >> on the interface AlreadyExistsException should be thrown.
> >> Inconsistent exception handling: Different methods will throw different
> >> exceptions for similar errors. This makes the interface hard to
> understand,
> >> hard to document and maintain. For example:
> >> Calling IMetaStoreClient.createTable with nonexistent database name will
> >> throw InvalidObjectException
> >> Calling IMetaStoreClient.createFunction with nonexistent database name
> >> database will throw NoSuchObjectException
> >> There are some cases when the Embedded MetaStore handles error
> differently
> >> than the Remote MetaStore. For example: IMetaStoreClient.dropTable with
> >> "null" as a database:
> >> Embedded MetaStore throws MetaException
> >> Remote MetaStore client throws TProtocolException
> >>
> >> Proposed changes:
> >> Fixing cases 1. and 2. is a simple bug fix - it could be done
> independently
> >> Fixing cases 3. and 4. will change how the IMetaStoreClient and HMS
> Thrift
> >> API works. For these we should review the IMetaStoreClient and HMS
> Thrift
> >> API interface exception handling, to create consistent and easy to
> follow
> >> rules for the possible exceptions. We propose to keep the current
> >> exceptions, and only change when the given type of exceptions are
> thrown.
> >> If we stick to this then the interface will be binary backward
> compatible
> >> since currently every method defines TException as a throwable and every
> >> exception is inherited from TException. I think we allowed to change
> this
> >> since 3.0.0 is a major release.
> >>
> >> Do we agree with the general direction of these changes?
> >>
> >> Thanks,
> >> Peter
> >>
> >>
>
>


Re: apache hive builds

2017-12-15 Thread Adam Szita
This should be fixed now. Some conclusions can be seen in at:
https://issues.apache.org/jira/browse/HIVE-18263?focusedCommentId=16292354=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16292354

On 11 December 2017 at 18:48, Adam Szita  wrote:

> Hi all,
>
> There are two things related to this:
> 1, We have recently applied a fix on the ptest component so we did a
> restart this morning. We also re-triggered all pending jobs at that time.
> More details: https://issues.apache.org/jira/browse/HIVE-18212
> 2, It seems like that for some time (even before Yetus integration was
> added) ptest was suffering from very slow executions from time to time.
> Today we looked in this as well in depth. I have found that some executor
> slaves are getting shut down in the middle of the testing. I already have a
> fix ready and will shortly upload it at:
> https://issues.apache.org/jira/browse/HIVE-18263
>
> Thanks,
> Adam
>
>
> On 11 December 2017 at 10:06, Anishek Agarwal  wrote:
>
>> Hello
>>
>> The apache builds seem to be taking very long and most of them just being
>> either manually shut or getting killed
>> Builds after https://builds.apache.org/job/PreCommit-HIVE-Build/8160/
>> seem
>> to be showing the problem. Problems are surfacing after TestExecDriver is
>> being run. Is there someone  here with better infrastructure understanding
>> to help figure out what is happening.
>>
>> Thanks
>> Anishek
>>
>
>


Re: [DISCUSS] IMetaStoreClient and HMS Thrift API exception handling

2017-12-15 Thread Peter Vary
If the application using the old API does not handle the original exceptions 
differently than the TExceptions then it should work as expected.

> On Dec 14, 2017, at 10:50 PM, Thejas Nair  wrote:
> 
> This direction looks good to me.
> If the new exceptions are inheriting from TException the applications would
> still work. But would it still work if we old metastore client library is
> used with a newer version of metastore server running with these changes ?
> 
> 
> On Wed, Dec 13, 2017 at 4:02 AM, Peter Vary  wrote:
> 
>> Hi Team,
>> 
>> Since the work begin to separate the HMS to a standalone project we
>> thought that it would be useful the create extensive API tests for the
>> public APIs of the new project.
>> We started to create tests using the IMetaStoreClient interface
>> implementations and found that not surprisingly the happy paths are working
>> as expected, but there are some gaps in the exception handling. Most of the
>> enhancements affect the Thrift API as well.
>> We went through the following methods:
>> Functions
>> Indexes
>> Tables
>> Databases
>> We also plan to comb through at least the Partition related methods too.
>> 
>> The possible enhancements we found could be grouped to the following
>> categories:
>> Bugs: For example IMetaStoreClient.drop/alter/createFunction with null
>> function name might throw NullPointerException exception - this is clearly
>> a bug which should be solved:
>> Embedded MetaStore throws NullPointerException
>> Remote MetaStore client throws TTransportException
>> Sub-optimal error handling: For example IMetaStoreClient.alterFunction
>> will not check if the new function is already exist, and tries to insert it
>> anyway. After 10 tries it throws a MetaException where the exception text
>> is "Update of object [...] failed : java.sql.
>> SQLIntegrityConstraintViolationException [...]". Fixing this could be
>> done without interface change, but following the logic of the other methods
>> on the interface AlreadyExistsException should be thrown.
>> Inconsistent exception handling: Different methods will throw different
>> exceptions for similar errors. This makes the interface hard to understand,
>> hard to document and maintain. For example:
>> Calling IMetaStoreClient.createTable with nonexistent database name will
>> throw InvalidObjectException
>> Calling IMetaStoreClient.createFunction with nonexistent database name
>> database will throw NoSuchObjectException
>> There are some cases when the Embedded MetaStore handles error differently
>> than the Remote MetaStore. For example: IMetaStoreClient.dropTable with
>> "null" as a database:
>> Embedded MetaStore throws MetaException
>> Remote MetaStore client throws TProtocolException
>> 
>> Proposed changes:
>> Fixing cases 1. and 2. is a simple bug fix - it could be done independently
>> Fixing cases 3. and 4. will change how the IMetaStoreClient and HMS Thrift
>> API works. For these we should review the IMetaStoreClient and HMS Thrift
>> API interface exception handling, to create consistent and easy to follow
>> rules for the possible exceptions. We propose to keep the current
>> exceptions, and only change when the given type of exceptions are thrown.
>> If we stick to this then the interface will be binary backward compatible
>> since currently every method defines TException as a throwable and every
>> exception is inherited from TException. I think we allowed to change this
>> since 3.0.0 is a major release.
>> 
>> Do we agree with the general direction of these changes?
>> 
>> Thanks,
>> Peter
>> 
>> 



Re: [DISCUSS] IMetaStoreClient and HMS Thrift API exception handling

2017-12-15 Thread Peter Vary
It makes perfect sense to me to clean up the API when we release the standalone 
MetaStore.
+1 for the API cleanup in general!

> On Dec 15, 2017, at 1:53 AM, Alexander Kolbasov  wrote:
> 
> +1 for API cleanup in general!
> 
> On Thu, Dec 14, 2017 at 4:25 PM, Sergey Shelukhin 
> wrote:
> 
>> If we break the APIs, can we also do the API cleanup where we remove
>> duplicate ones and change everything to use req/resp pattern?
>> 
>> On 17/12/14, 13:50, "Thejas Nair"  wrote:
>> 
>>> This direction looks good to me.
>>> If the new exceptions are inheriting from TException the applications
>>> would
>>> still work. But would it still work if we old metastore client library is
>>> used with a newer version of metastore server running with these changes ?
>>> 
>>> 
>>> On Wed, Dec 13, 2017 at 4:02 AM, Peter Vary  wrote:
>>> 
 Hi Team,
 
 Since the work begin to separate the HMS to a standalone project we
 thought that it would be useful the create extensive API tests for the
 public APIs of the new project.
 We started to create tests using the IMetaStoreClient interface
 implementations and found that not surprisingly the happy paths are
 working
 as expected, but there are some gaps in the exception handling. Most of
 the
 enhancements affect the Thrift API as well.
 We went through the following methods:
 Functions
 Indexes
 Tables
 Databases
 We also plan to comb through at least the Partition related methods too.
 
 The possible enhancements we found could be grouped to the following
 categories:
 Bugs: For example IMetaStoreClient.drop/alter/createFunction with null
 function name might throw NullPointerException exception - this is
 clearly
 a bug which should be solved:
 Embedded MetaStore throws NullPointerException
 Remote MetaStore client throws TTransportException
 Sub-optimal error handling: For example IMetaStoreClient.alterFunction
 will not check if the new function is already exist, and tries to
 insert it
 anyway. After 10 tries it throws a MetaException where the exception
 text
 is "Update of object [...] failed : java.sql.
 SQLIntegrityConstraintViolationException [...]". Fixing this could be
 done without interface change, but following the logic of the other
 methods
 on the interface AlreadyExistsException should be thrown.
 Inconsistent exception handling: Different methods will throw different
 exceptions for similar errors. This makes the interface hard to
 understand,
 hard to document and maintain. For example:
 Calling IMetaStoreClient.createTable with nonexistent database name will
 throw InvalidObjectException
 Calling IMetaStoreClient.createFunction with nonexistent database name
 database will throw NoSuchObjectException
 There are some cases when the Embedded MetaStore handles error
 differently
 than the Remote MetaStore. For example: IMetaStoreClient.dropTable with
 "null" as a database:
 Embedded MetaStore throws MetaException
 Remote MetaStore client throws TProtocolException
 
 Proposed changes:
 Fixing cases 1. and 2. is a simple bug fix - it could be done
 independently
 Fixing cases 3. and 4. will change how the IMetaStoreClient and HMS
 Thrift
 API works. For these we should review the IMetaStoreClient and HMS
 Thrift
 API interface exception handling, to create consistent and easy to
 follow
 rules for the possible exceptions. We propose to keep the current
 exceptions, and only change when the given type of exceptions are
 thrown.
 If we stick to this then the interface will be binary backward
 compatible
 since currently every method defines TException as a throwable and every
 exception is inherited from TException. I think we allowed to change
 this
 since 3.0.0 is a major release.
 
 Do we agree with the general direction of these changes?
 
 Thanks,
 Peter
 
 
>> 
>>