Re: Review Request 71267: HIVE-22087: Transform Database object on getDatabase() to return location based on client capabilities.

[Thejas Nair]

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




standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift
Lines 1938 (patched)
<https://reviews.apache.org/r/71267/#comment304501>

To future proof that, it would have better better to have a 
GetDatabaseResponse as well
similar to get_catalog.
That can be a smaller follo wup patch


- Thejas Nair


On Aug. 14, 2019, 6:26 a.m., Naveen Gangam wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71267/
> ---
> 
> (Updated Aug. 14, 2019, 6:26 a.m.)
> 
> 
> Review request for hive, Daniel Dai, Jason Dere, and Thejas Nair.
> 
> 
> Bugs: HIVE-22087
> https://issues.apache.org/jira/browse/HIVE-22087
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> 1) getDatabase() calls should be transformed to return a Database object that 
> can vary in location depending on the client capabilities. If client has 
> ACID*WRITE* capabilities, location is unaltered. If the client does not have 
> such capabilities, the database will return an location from the external 
> warehouse directory.
> 2) When a non-ACID MANAGED table is translated to EXTERNAL table, its 
> location should be altered to point to an external warehouse directory and 
> not to the managed warehouse.
> 3) Some new test cases.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreTransformer.java
>  e50b577ff7 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/AlterPartitionsRequest.java
>  6453c93d79 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/CreateTableRequest.java
>  5d42a80373 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/FindSchemasByColsResp.java
>  4024751ed3 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetDatabaseRequest.java
>  PRE-CREATION 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsFilterSpec.java
>  fcba6ebb4d 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsProjectionSpec.java
>  d94cbb1bcc 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsRequest.java
>  dd4bf8339a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/GetPartitionsResponse.java
>  ddfa59fb1c 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/RenamePartitionRequest.java
>  de467c298f 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/SchemaVersion.java
>  09fcd476e9 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/ThriftHiveMetastore.java
>  6b117291a6 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMFullResourcePlan.java
>  080111d85b 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetAllResourcePlanResponse.java
>  d0174005ca 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMGetTriggersForResourePlanResponse.java
>  e5425909d4 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/WMValidateResourcePlanResponse.java
>  b12c2284a2 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/ThriftHiveMetastore.php
>  4623e9ab5f 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-php/metastore/Types.php
>  0d45371b88 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore-remote
>  647c762acd 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ThriftHiveMetastore.py
>  5107d0f99a 
>   
> standalone-metastore/metastore-common/src/gen/thrift/gen-py/hive_metastore/ttypes.py
>  08c0730e1c 
>   
> standalone-me

Re: Avoiding merge commits

[Thejas Nair]

+1


On Mon, Mar 11, 2019 at 8:22 AM Owen O'Malley 
wrote:

> I'm +1 for avoiding merge commits on master and release branches.
>
> .. Owen
>
>
> On Mon, Mar 11, 2019 at 7:47 AM Ashutosh Chauhan 
> wrote:
>
> > Hi,
> > With advent of gitbox/github integration, folks have started using merge
> > commits (inadvertently I believe by merging via Github). This causes
> issues
> > downstream where in my branch, I can't cherry-pick and rebase, but rather
> > get merge history. So, I propose we ban merge commits.
> > Too radical?
> >
> > Thanks,
> > Ashutosh
> >
>

Re: Force push of branch-2.3

[Thejas Nair]

+1


On Wed, Mar 27, 2019 at 9:45 AM Owen O'Malley 
wrote:

> All,
>The branch-2.3 currently does not have either the rel/release-2.3.3 or
> rel/release-2.3.4 tag on it. There is only one commit after the 2.3.4 tag
> that back ports HIVE-20126. If no one objects, I’d like to force push
> branch-2.3 to include the 2.3.4 tag (with HIVE-20126 rebased on top). We
> can’t fix the branch to include the 2.3.3 tag without disconnecting the
> 2.3.4 tag.
>
> Let me know if anyone has any concerns.
>
> Thanks!
>Owen

Re: [ANNOUNCE] New committer: Mahesh Behera

[Thejas Nair]

Congrats Mahesh!
Very well deserved!

On Fri, Nov 16, 2018 at 6:24 PM Ashutosh Chauhan  wrote:
>
>  Apache Hive's Project Management Committee (PMC) has invited Mahesh
> Behera to become a committer, and we are pleased to announce that he has
> accepted.
> Mahesh, welcome, thank you for your contributions, and we look forward to
> your further interactions with the community!
>
> Thanks,
> Ashutosh Chauhan (on behalf of the Apache Hive PMC)

Re: HIVE-20420 Provide a fallback authorizer when no other authorizer is in use

[Thejas Nair]

This was for CVE-2018-11777.
You can find more details in description of CVE-2018-11777

On Wed, Nov 14, 2018 at 3:40 AM Oleksiy S  wrote:
>
> Guys, could you help with this new feature? HIVE-20420
>
> I see no docs, no use cases, just nothing. Thanks.
>
> --
> Oleksiy

Re: [VOTE] Apache Hive 2.3.4 Release Candidate 0

[Thejas Nair]

+1 Verified signatures, build src tar

On Wed, Oct 31, 2018 at 2:45 PM Daniel Dai  wrote:
>
> Apache Hive 2.3.4 Release Candidate 0 is available here:
>
> http://people.apache.org/~daijy/apache-hive-2.3.4-rc-0/
>
> Maven artifacts are available here:
>
> https://repository.apache.org/content/repositories/orgapachehive-1093
>
> Source tag for RCN is at:
>
> https://github.com/apache/hive/tree/release-2.3.4-rc0
>
> Voting will conclude in 72 hours.
>
> Hive PMC Members: Please test and vote.
>
> Thanks.
>
>

Re: [VOTE] Apache Hive 3.1.1 Release Candidate 0

[Thejas Nair]

 * Verified signatures and checksums.
 * Reviewed git rc tag contents
 * Build src tar.gz
 * Untarred bin.tar.gz and ran queries

+1 to the release


On Wed, Oct 24, 2018 at 4:59 PM Daniel Dai  wrote:
>
> Apache Hive 3.1.1 Release Candidate 0 is available here:
>
> http://people.apache.org/~daijy/apache-hive-3.1.1-rc-0/
>
> Maven artifacts are available here:
>
> https://repository.apache.org/content/repositories/orgapachehive-1092
>
> Source tag for RCN is at:
>
> https://github.com/apache/hive/tree/release-3.1.1-rc0
>
> Voting will conclude in 72 hours.
>
> Hive PMC Members: Please test and vote.
>
> Thanks.
>

Re: Apache Hive 3.1. release preparation

[Thejas Nair]

Hi Vihang,
I don't think we need to serialize these releases at this time. The
metastore is still being released as part of hive as well at this
time.
Maybe as part of the Hive 3.1 release process, ie after branching,
during stabilization/voting you can verify that metastore in
standalone mode is working in that branch ? (Earlier we start the
verification, the better).
We could use still use the same version tag and have the releases go
in parallel, or a different sequence.
Not having to serialize these releases would help with smaller release cycles.

Thanks,
Thejas



On Tue, Jun 26, 2018 at 9:59 AM, Vihang Karajgaonkar
 wrote:
> +1 for releasing Hive 3.1. I can be the RM for the metastore 3.1 release
> while Vineet takes care of the Hive release. The metastore will be released
> from branch-3.1 after it is cut out. I propose to make a metastore 3.1
> release first and then release Hive 3.1 shortly afterwards so that it
> depends on metastore 3.1. Any thoughts?
>
> On Mon, Jun 25, 2018 at 3:16 PM, Vihang Karajgaonkar 
> wrote:
>
>> I think it would be useful to do a metastore 3.1 release as well along
>> with the release. In order to do that we should deploy metastore in
>> standalone mode, make sure it works as expected and also document how to
>> install and use metastore as a standalone module.
>>
>> On Mon, Jun 25, 2018 at 3:00 PM, Vineet Garg 
>> wrote:
>>
>>> Hello folks,
>>>
>>> It has been more than one month since Hive 3.0 release. Plenty of bug
>>> fixes and minor features have been pushed in to branch-3 and therefore I
>>> believe it is time for us to release Hive 3.1.
>>> I plan to cut off branch 3.1 off branch-3 tomorrow at end of day.  Once
>>> the branch is cut please do not commit anything in there. If you absolutely
>>> must please check in with me first. I plan to prepare a RC within a week of
>>> cutting the branch.
>>>
>>> Thanks,
>>> Vineet
>>>
>>>
>>

Re: Ptest timeouts

[Thejas Nair]

+ Vihang


On Tue, Jun 19, 2018 at 1:55 AM, Prasanth Jayachandran
 wrote:
> Precommit tests have started to timeout again. Could it be because of disk 
> space issue? Should we need restart again? Sometimes >90% disk usage may also 
> result in unhealthy node where no containers can be launched.
>
> Thanks
> Prasanth

Re: Cleaning up old version in dist

[Thejas Nair]

+1

On Thu, Jun 7, 2018 at 11:13 AM, Alan Gates  wrote:
> Apache asks that we keep at most 2 current versions in dist, to minimize
> the space we take up on distribution mirrors.  Since we are running
> multiple lines and a have a couple of separately releasable modules we'll
> have more than 2 versions there.  But we have old versions of Hive 2 (2.1,
> 2.2) and of the storage-api (2.4, 2.5).  I think we should remove these.
> That will leave us with the most up to date versions of Hive 1, 2, 3, the
> storage api, and the standalone metastore.  Note that this does not affect
> their availability in maven central or the apache archive.
>
> Alan.

Re: Metastore thrift api permission

[Thejas Nair]

This covers metastore api authorization -
https://cwiki.apache.org/confluence/display/Hive/Storage+Based+Authorization+in+the+Metastore+Server


On Tue, May 29, 2018 at 8:29 AM, 侯宗田  wrote:
> Hello,everyone:
> I am using HMS thrift api to get metadata of hive table, it is ok in my local 
> host. But I am wondering how is the metastore access permission is being 
> controlled. Is it the metadata can be accessed by thrift client just by the 
> HMS server IP and port or it need extra authentication?

Re: Review Request 67102: HIVE-19440: Make StorageBasedAuthorizer work with information schema

[Thejas Nair]

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


Ship it!




Ship It!

- Thejas Nair


On May 15, 2018, 9:04 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67102/
> ---
> 
> (Updated May 15, 2018, 9:04 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19440
> 
> 
> Diffs
> -
> 
>   
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/storagehandler/DummyHCatAuthProvider.java
>  a53028f 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestHDFSPermissionPolicyProvider.java
>  PRE-CREATION 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/security/DummyHiveMetastoreAuthorizationProvider.java
>  31e795c 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql d9606d8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java a1f549a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HDFSPermissionPolicyProvider.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java
>  8a7c06d 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveMetastoreAuthorizationProvider.java
>  0dab334 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PolicyProviderContainer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  9b2e6cd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
>  b66d188 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java
>  48798d8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java
>  02ed7aa 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentAuthorizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  3eb0914 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 661beb5 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  92d2e3f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  6af2aa5 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  09f9bb1 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  264fdb9 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  ce7d286 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  b223920 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveObjectPrivilegeBuilder.java
>  d802e1a 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MDBPrivilege.java
>  3d8fa21 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MGlobalPrivilege.java
>  5b496e0 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MPartitionColumnPrivilege.java
>  ab50a92 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MPartitionPrivilege.java
>  3193bc1 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MTableColumnPrivilege.java
>  ad7322f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MTablePrivilege.java
>  6460400 
>   standalone-metastore/src/main/resources/package.jdo 2d2cb19 
>   standalone-metastore/src/main/sql/derby/hive-schema-3.1.0.derby.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/derby/upgrade-3.0.0-to-3.1.0.derby.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/mssql/hive-schema-3.1.0.mssql.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/mssql/upgrade-3.0.0-to-3.1.0.mssql.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/mysql/hive-schema-3.1.0.mysql.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/mysql/upgrade-3.0.0-to-3.1.0.mysql.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/oracle/hive-schema-3.1.0.oracle.sql 
> PRE-CREATI

Re: Review Request 67102: HIVE-19440: Make StorageBasedAuthorizer work with information schema

[Thejas Nair]

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




service/src/java/org/apache/hive/service/server/HiveServer2.java
Lines 992 (patched)
<https://reviews.apache.org/r/67102/#comment285140>

how about using AuthorizationPreEventListener.class.getName() or so ?


- Thejas Nair


On May 15, 2018, 9:04 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67102/
> ---
> 
> (Updated May 15, 2018, 9:04 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19440
> 
> 
> Diffs
> -
> 
>   
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/storagehandler/DummyHCatAuthProvider.java
>  a53028f 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestHDFSPermissionPolicyProvider.java
>  PRE-CREATION 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/security/DummyHiveMetastoreAuthorizationProvider.java
>  31e795c 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql d9606d8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java a1f549a 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HDFSPermissionPolicyProvider.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java
>  8a7c06d 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveMetastoreAuthorizationProvider.java
>  0dab334 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PolicyProviderContainer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  9b2e6cd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
>  b66d188 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java
>  48798d8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java
>  02ed7aa 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentAuthorizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  3eb0914 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 661beb5 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  92d2e3f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  6af2aa5 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  09f9bb1 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  264fdb9 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  ce7d286 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  b223920 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveObjectPrivilegeBuilder.java
>  d802e1a 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MDBPrivilege.java
>  3d8fa21 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MGlobalPrivilege.java
>  5b496e0 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MPartitionColumnPrivilege.java
>  ab50a92 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MPartitionPrivilege.java
>  3193bc1 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MTableColumnPrivilege.java
>  ad7322f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MTablePrivilege.java
>  6460400 
>   standalone-metastore/src/main/resources/package.jdo 2d2cb19 
>   standalone-metastore/src/main/sql/derby/hive-schema-3.1.0.derby.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/derby/upgrade-3.0.0-to-3.1.0.derby.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/mssql/hive-schema-3.1.0.mssql.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/mssql/upgrade-3.0.0-to-3.1.0.mssql.sql 
> PRE-CREATION 
>   standalone-metastore/src/main/sql/mysql/hive-schema-3.1.0.mysql.sql 
> PRE-C

Re: [VOTE] Stricter commit guidelines

[Thejas Nair]

+1

On Mon, May 14, 2018 at 10:44 PM, Jesus Camacho Rodriguez
 wrote:
> After work has been done to ignore most of the tests that were failing 
> consistently/intermittently [1], I wanted to start this vote to gather 
> support from the community to be stricter wrt committing patches to Hive. The 
> committers guide [2] already specifies that a +1 should be obtained before 
> committing, but there is another clause that allows committing under the 
> presence of flaky tests (clause 4). Flaky tests are as good as having no 
> tests, hence I propose to remove clause 4 and enforce the +1 from testing 
> infra before committing.
>
>
>
> As I see it, by enforcing that we always get a +1 from the testing infra 
> before committing, 1) we will have a more stable project, and 2) we will have 
> another incentive as a community to create a more robust testing infra, e.g., 
> replacing flaky tests for similar unit tests that are not flaky, trying to 
> decrease running time for tests, etc.
>
>
>
> Please, share your thoughts about this.
>
>
>
> Here is my +1.
>
>
>
> Thanks,
>
> Jesús
>
>
>
> [1] 
> http://mail-archives.apache.org/mod_mbox/hive-dev/201805.mbox/%3C63023673-AEE5-41A9-BA52-5A5DFB2078B6%40apache.org%3E
>
> [2] 
> https://cwiki.apache.org/confluence/display/Hive/HowToCommit#HowToCommit-PreCommitruns,andcommittingpatches
>
>
>

Re: Review Request 67102: HIVE-19440: Make StorageBasedAuthorizer work with information schema

[Thejas Nair]

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




ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PolicyProviderContainer.java
Lines 51 (patched)
<https://reviews.apache.org/r/67102/#comment285113>

naming convention would start with lower case - 
authorizationProviderPosition



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentAuthorizer.java
Lines 74 (patched)
<https://reviews.apache.org/r/67102/#comment285114>

can you add a comment about this fallback path for SBA ?



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
Lines 65 (patched)
<https://reviews.apache.org/r/67102/#comment285136>

HIVE_AUTHORIZATION_ENABLED can be false , while  metastore authorization 
(SBA) is enabled .

SBA doesn't use this flag. 
https://cwiki.apache.org/confluence/display/Hive/Storage+Based+Authorization+in+the+Metastore+Server#StorageBasedAuthorizationintheMetastoreServer-ConfigurationParametersforMetastoreSecurity


- Thejas Nair


On May 11, 2018, 9:55 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67102/
> ---
> 
> (Updated May 11, 2018, 9:55 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19440
> 
> 
> Diffs
> -
> 
>   
> hcatalog/core/src/main/java/org/apache/hive/hcatalog/storagehandler/DummyHCatAuthProvider.java
>  a53028f 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestHDFSPermissionPolicyProvider.java
>  PRE-CREATION 
>   
> itests/util/src/main/java/org/apache/hadoop/hive/ql/security/DummyHiveMetastoreAuthorizationProvider.java
>  31e795c 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql d9606d8 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 4611ce9 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HDFSPermissionPolicyProvider.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProviderBase.java
>  8a7c06d 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveMetastoreAuthorizationProvider.java
>  0dab334 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PolicyProviderContainer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  9b2e6cd 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java
>  b66d188 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveV1Authorizer.java
>  48798d8 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java
>  02ed7aa 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentAuthorizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  3eb0914 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java cf3edbf 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  3978b88 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  6af2aa5 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  09f9bb1 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  b43334b 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  ce7d286 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  b223920 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/client/builder/HiveObjectPrivilegeBuilder.java
>  d802e1a 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MDBPrivilege.java
>  3d8fa21 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MGlobalPrivilege.java
>  5b496e0 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MPartitionColumnPrivilege.java
>  ab50a92 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/model/MPartitionPrivilege.java
>  3193bc1 
>   
> standalone-metastore/src/main/ja

Re: Apache Hive 3.0.0 release preparation

[Thejas Nair]

Hi Vihang,
That is expected if you have 2.4.0 release that happens little after
(or around same time as)  a 3.0 release.
This is more likely to happen with last minute bug fixes.



On Mon, May 7, 2018 at 11:58 AM, Vihang Karajgaonkar
<vih...@cloudera.com> wrote:
> How do we handle the cases where a patch needs to go in branch-2? I think
> the reason of the commits are going in branch-3 is because you don't want
> have a intermediate version where a patch is missing. Eg: patch is fixed in
> Hive 2.4.0 and Hive 3.1.0 but not Hive 3.0.0. Is that normal?
>
> On Mon, May 7, 2018 at 11:34 AM, Vineet Garg <vg...@hortonworks.com> wrote:
>
>> Hello all,
>>
>> It’s been more than a month since we have cutoff branch-3. There are tests
>> which are still failing consistently(https://issues.
>> apache.org/jira/browse/HIVE-19142).  At this point we should work on
>> fixing tests and stabilize the branch. So please refrain from committing
>> anything but test fixes to branch-3.
>> If you have a patch beside test fix which you would like to get into
>> branch-3 please talk to me before committing.
>>
>> Also if you are assigned a JIRA for fixing test in branch-3 please fix it
>> as soon as you can.
>>
>> Thanks,
>> Vineet Garg
>>
>> On Apr 13, 2018, at 12:36 PM, Vihang Karajgaonkar <vih...@cloudera.com
>> <mailto:vih...@cloudera.com>> wrote:
>>
>> Hi Vineet,
>>
>> I created a profile on ptest-server so that tests can be run on branch-3.
>> It is the same as branch-2 patches. You will need to include branch-3 in
>> the patch name. Eg. HIVE-1234.01-branch-3.patch
>>
>> -Vihang
>>
>>
>>
>> On Mon, Apr 9, 2018 at 4:35 PM, Vineet Garg <vg...@hortonworks.com> vg...@hortonworks.com>> wrote:
>>
>> I have created an umbrella jira to investigate and fix test failures for
>> hive 3.0.0. LINK : https://issues.apache.org/jira/browse/HIVE-19142.
>> Please link any other existing jira related to test failure with this
>> umbrella jira.
>>
>> Also, how do we run tests on branch-3? Is there some setup to be done?
>>
>> -Vineet
>>
>> On Apr 9, 2018, at 4:26 AM, Zoltan Haindrich <zhaindr...@hortonworks.com<
>> mailto:zhaindr...@hortonworks.com><
>> mailto:zhaindr...@hortonworks.com>> wrote:
>>
>> Hello
>>
>> A few weeks earlier I've tried to hunt down this problem...
>> so...to my best knowledge the cause of this seems to be the following:
>>
>> * in some cases the "cleanup" after a failed query may somehow leave some
>> threads behind...
>> * these threads have reference to the "customized" session classloader -
>> this makes the threads more memory hungry
>> * after a while these threads/classloaders eat up the heap...
>>
>> I've opened HIVE-18522 for this thread issue
>>
>> I think this problem is not new ...and it might have been present earlier
>> as well...the only thing what changed is that there were a few more new
>> features which have added new udfs/etc which made the memory cost of a
>> session more heavier..
>> ...and as a sidenote: I'm not convinced that this issue will arise in a
>> proper hs2 setup - as it might be easily connected to the fact that these
>> tests are using the cli driver to execute the tests.
>>
>>
>> cheers,
>> Zoltan
>>
>> On 7 Apr 2018 7:15 p.m., Ashutosh Chauhan <hashut...@apache.org> ashut...@apache.org>> ashut...@apache.org<mailto:ashut...@apache.org>>> wrote:
>> We need to investigate and find out root cause of these failures. If its
>> determined that its a corner case and fix is non-trivial then we may
>> release note it under known issues. But ideally we should fix these
>> failures.
>> Cutting a branch should make it easier since branch is expected to receive
>> lot less commits as compared to master so it should be faster to stabilize
>> branch.
>>
>> On Fri, Apr 6, 2018 at 10:49 AM, Eugene Koifman <ekoif...@hortonworks.com<
>> mailto:ekoif...@hortonworks.com><
>> mailto:ekoif...@hortonworks.com>>
>> wrote:
>>
>> Cutting the branch before the tests are stabilized would mean we have to
>> fix them in 2 places.
>>
>> On 4/6/18, 10:05 AM, "Thejas Nair" <thejas.n...@gmail.com> thejas.n...@gmail.com>> thejas.n...@gmail.com<mailto:thejas.n...@gmail.com>>> wrote:
>>
>>   That needs to be cleaned up. There are far too many right now, its
>>   just not handful of flaky tests.
>>
>>
>

Re: Review Request 66571: HIVE-19161: Add authorizations to information schema

[Thejas Nair]

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




jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java
Lines 146 (patched)
<https://reviews.apache.org/r/66571/#comment283779>

indentation seems off here.



ql/src/java/org/apache/hadoop/hive/ql/metadata/JarUtils.java
Lines 55 (patched)
<https://reviews.apache.org/r/66571/#comment283780>

the accumulo specific reference shold be removed from this class



ql/src/java/org/apache/hadoop/hive/ql/metadata/JarUtils.java
Lines 143 (patched)
<https://reviews.apache.org/r/66571/#comment283781>

how about using java8 style and skip finally block -
try (ZipFile zip = new ZipFile(jar)) { 
  
    
    }


- Thejas Nair


On April 28, 2018, 1:09 a.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66571/
> ---
> 
> (Updated April 28, 2018, 1:09 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19161
> 
> 
> Diffs
> -
> 
>   
> accumulo-handler/src/java/org/apache/hadoop/hive/accumulo/HiveAccumuloHelper.java
>  9fccb49 
>   accumulo-handler/src/java/org/apache/hadoop/hive/accumulo/Utils.java 
> 3a2facf 
>   
> accumulo-handler/src/java/org/apache/hadoop/hive/accumulo/serde/CompositeAccumuloRowIdFactory.java
>  d8b9aa3 
>   
> accumulo-handler/src/java/org/apache/hadoop/hive/accumulo/serde/DefaultAccumuloRowIdFactory.java
>  bae2930 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java f40c606 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  8ecbaad 
>   itests/hive-unit/pom.xml 3ae7f2f 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestRestrictedList.java 
> 79fdb68 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/server/TestInformationSchemaWithPrivilege.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/JdbcStorageHandler.java
>  df55272 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/DatabaseAccessorFactory.java
>  6d3c8d9 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/GenericJdbcDatabaseAccessor.java
>  772bc5d 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/JdbcRecordIterator.java
>  638e2b0 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/MsSqlDatabaseAccessor.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/OracleDatabaseAccessor.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/PostgresDatabaseAccessor.java
>  PRE-CREATION 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql 339 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d59bf1f 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/JarUtils.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProvider.java
>  60d9dc1 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveResourceACLsImpl.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 60b63d4 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentGroups.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/resourceplan.q.out 9850276 
>   ql/src/test/results/clientpositive/show_functions.q.out 4df555b 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java e373628 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  397a081 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  1c8d223 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  aee416d 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  184ecb6 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  2c9f2e5 
>   
> standalone-metastore/src/main/java/org

Re: Proposal: Apply SQL based authorization functions in the metastore.

[Thejas Nair]

Hi Elliot,

One scenario where Storage based authorization doesn't work is the
case of object stores such as S3. In those scenarios, the
tool/platform that is accessing the data won't have any restrictions
on data access either. I am not sure how the data access would be
secured in such cases, even if metastore access is controlled.

Overall, the metastore api is a much lower level API, and as a result
it is difficult to enforce higher level restrictions at that level.
(More on that below).

I agree that O/JDBC via HS2 is not something distributed tools can use
(at least with standard API).
I think the ideal way to enforce security is having tools/platforms
read via a 'table server' (and not give them direct file system
access).
At Hortonworks, we have been using this to provide security for Spark,
by allowing it to read in parallel from LLAP deamons -
https://www.slideshare.net/Hadoop_Summit/security-updates-more-seamless-access-controls-with-apache-spark-and-apache-ranger
https://github.com/hortonworks-spark/spark-llap/wiki/1.-Goal-and-features
(You can replace Ranger with SQL auth as well in above examples).

The next phase of that work would likely make use Apache Arrow for the
data exchange (there are some hive jiras created recently around it).

I had considered having the authorization at metastore level, but
realized that is not the right place to enforce the RDBMS/SQL style
policies. Here are some notes I wrote while back about it -
http://hadoop-pig-hive-thejas.blogspot.com/2014/03/hive-sql-standard-authorization-why-not.html

Quoting from there -
The advantage of doing it at the metastore api level would have been
that pig and MR would also be covered under this authorization model.
But this works only if the SQL actions always needs some metastore api
calls, and access control on these calls it needs to make can be used
to enforce the SQL level authorization.

Take for example INSERT privilege in SQL, you can grant INSERT without
granting SELECT privilege. But when processing insert queries for the
user, we need to be able to do a getTable() and read the schema of the
table. But if you look at it from metastore api perspective, you
should not be able to do a getTable() without having SELECT privileges
on the table.
Similar issues happen with DELETE and UPDATE privileges, which you can
grant without SELECT.

Another example is URIs in the SQL statement, you don't need to make
any metatore api calls before access URIs. So URI access control can't
be implemented using metastore api calls.

Another use case is anything that you want to allow the ADMIN to do
but the action does not involve specific metastore api calls that can
be used to control the action.

Thanks,
Thejas


On Fri, Apr 20, 2018 at 6:30 AM, Elliot West  wrote:
> Hello,
>
> I’d like to propose that SQL based authorization (or something similar) be
> applied and enforced also in the metastore service as part of the initiative
> to extract HMS as an independent project. While any such implementation
> cannot be ’system complete’ like HiveServer2 (HS2) (HMS has no scope to
> intercept operations applied to table data, only metadata), it would be a
> significant step forward for controlling the operations that can be actioned
> by the many non-HS2 clients in the Hive ecosystem.
>
> I believe this is a good time to consider this option as there is currently
> much discussion in the Hive community on the future directions of HMS and
> greater recognition that HMS is now seen as general data platform
> infrastructure and not simply an internal Hive component.
>
> Further details are below. I’d be grateful for any feedback, thoughts, and
> suggestions on how this could move forward.
>
> Problem
> At this time, Hive’s SQL based authorization feature is the recommended
> approach for controlling which operations may be performed on what by whom.
> This feature is applied in the HS2 component. However, a large number of
> platforms that integrate with Hive do not do so via HS2, instead talking to
> the metastore service directly and so bypassing authorization. They can
> perform destructive operations such as a table drop even though the
> permissions declared in the metastore may explicitly forbid it as they are
> able to circumvent the authorization logic in HS2.
>
> In short, there seems to be a lack of encapsulation with authorization in
> the metastore; HMS owns the metadata, is responsible for performing actions
> on metadata, for maintaining permissions on what actions are permissible by
> whom, and yet has no means to use the information it has to protect the data
> it owns.
>
> Workarounds
> Common workarounds to this deficiency include falling back to storage based
> authorization or running read only metastore instances. However, both of
> these approaches have significant drawbacks:
>
> File based auth does not function when using object stores such as S3 and so
> is not usable in cloud deployments of Hive - a 

Re: ptest queue

[Thejas Nair]

Option 3 seems reasonable. I believe that used to be the state a while
back (maybe 12 months back or so).
When 2nd ptest for same jira runs, it checks if the latest patch has
already been run.


On Wed, Apr 25, 2018 at 7:37 AM, Peter Vary  wrote:
> I would vote for version 3. It would solve the big patch problem, and removes 
> the unnecessary test runs too.
>
> Thanks,
> Peter
>
>> On Apr 25, 2018, at 11:01 AM, Adam Szita  wrote:
>>
>> Hi all,
>>
>> I had a patch (HIVE-19077) committed with the original aim being the
>> prevention of wasting resources when running ptest on the same patch
>> multiple times:
>> It is supposed to manage scenarios where a developer uploads
>> HIVE-XYZ.1.patch, that gets queued in jenkins, then before execution
>> HIVE-XYZ.2.patch (for the same jira) is uploaded and that gets queued also.
>> When the first patch starts to execute ptest will see that patch2 is the
>> latest patch and will use that. After some time the second queued job will
>> also run on this very same patch.
>> This is just pointless and causes long queues to progress slowly.
>>
>> My idea was to remove these duplicates from the queue where I'd only keep
>> the latest queued element if I see more queued entries for the same jira
>> number. It's like when you go grocery shopping and you're already in line
>> at cashier but you realise you also need e.g. milk. You go grab it and join
>> the END of the queue. So I believe it's a fair punishment for losing one's
>> spot in the queue for making amends on their patch.
>>
>> That said Deepak made me realise that for big patches this will be very
>> cumbersome due to the need of constant rebasing to avoid conflicts on patch
>> application.
>> I have three proposals now:
>>
>> 1: Leave this as it currently is (with HIVE-19077 committed) - *only the
>> latest queued job will run of the same jira*
>> pros: no wasting resources to run the same patches more times, 'scheduling'
>> is fair: if you amend you're patch you may loose your original spot in the
>> queue
>> cons: big patches that are prone to conflicts will be hard to get executed
>> in ptest, devs will have to wait more time for their ptest results if they
>> amend their patches
>>
>> 2: *Add a safety switch* to this queue checking feature (currently proposed
>> in HIVE-19077), deduplication can be switch off on request
>> pros: same as 1st, + ability to have more control on this mechanism i.e.
>> turn it off for big/urgent patches
>> cons: big patches that use the swich might still waste resources, also devs
>> might use safety switch inappropriately for their own evil benefit :)
>>
>> 3: Deduplication the other way around - *only the first queued job will run
>> of the same jira*, ptest server will keep record of patch names and won't
>> execute a patch with a seen name and jira number again
>> pros: same patches will not be executed more times accidentally, big
>> patches won't be a problem either, devs will get their ptest result back
>> earlier even if more jobs are triggered for same jira/patch name
>> cons: scheduling is less fair: devs can reserve their spots in the queue
>>
>>
>> (0: restore original: I'm strongly against this, ptest queue is already too
>> big as it is, we have to at least try and decrease its size by
>> deduplicating jiras in it)
>>
>> I'm personally fine with any of the 1,2,3 methods listed above, with my
>> favourites being 2 and 3.
>> Let me know which one you think is the right path to go down on.
>>
>> Thanks,
>> Adam
>>
>> On 20 April 2018 at 20:14, Eugene Koifman  wrote:
>>
>>> Would it be possible to add patch name validation when it gets added to
>>> the queue?
>>> Currently I think it fails when the bot gets to the patch if it’s not
>>> named correctly.
>>> More  common for branch patches
>>>
>>> On 4/20/18, 8:20 AM, "Zoltan Haindrich"  wrote:
>>>
>>>Hello,
>>>
>>>Some time ago the ptest queue worked the following way:
>>>
>>>* for some reason ATTACHMENT_ID was not set by the upstream jira
>>> scanner
>>>tool; this  triggered a feature in Jenkins: if for the same ticket
>>>mutliple patches were uploaded; they didn't triggered new runs
>>> (because
>>>the parameters were the same)
>>>* this have become fixed at some point...around that time I started
>>>getting multiple ptest executions for the same ticket - because I've
>>>fixed a minor typo after submitting the first version of my patch...
>>>* currently we also have a jenkins queue reader inside the ptest
>>>job...which checks if the ticket is in the queue right now; and if is
>>>it, it just exits...this logic kinda restores the earlier behaviour;
>>>with the exception that if I upload a patch every day and the queue is
>>>longer that 1day (like now); I will never get a ptest run :D
>>>* ...now here I come! I've just removed my patch from yesterday;
>>> because
>>>I want a ptest run 

Re: Review Request 66571: HIVE-19161: Add authorizations to information schema

[Thejas Nair]

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 4203 (patched)
<https://reviews.apache.org/r/66571/#comment282155>

needs rename to "hive." here as well



metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Lines 1085 (patched)
<https://reviews.apache.org/r/66571/#comment282157>

I think its easier to reason about the correctness of this one if you put 
another set of paranthesis around the group match condition-

OR ((array_contains(current_groups(), P.`PRINCIPAL_NAME`) OR 
P.`PRINCIPAL_NAME` = 'public') AND P.`PRINCIPAL_TYPE`='GROUP'))

Otherwise, it seems like it might get reduced to something like -

P.`PRINCIPAL_TYPE`='USER' OR (array) AND P.`PRINCIPAL_TYPE`='GROUP'



metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Lines 1120 (patched)
<https://reviews.apache.org/r/66571/#comment282156>

similar parenthesis would be useful here



metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Lines 1152 (patched)
<https://reviews.apache.org/r/66571/#comment282158>

similar parenthesis would be useful here



metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Lines 1309 (patched)
<https://reviews.apache.org/r/66571/#comment282159>

similar parenthesis would be useful here



metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Lines 1345 (patched)
<https://reviews.apache.org/r/66571/#comment282161>

similar parenthesis would be useful here



metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Lines 1382 (patched)
<https://reviews.apache.org/r/66571/#comment282160>

similar parenthesis would be useful here



service/src/java/org/apache/hive/service/server/HiveServer2.java
Lines 141 (patched)
<https://reviews.apache.org/r/66571/#comment282164>

typo, rename variable to -
zooKeeperClientForPrivilegeSynchronizer

or shorter -
zKClientForPrivSync



standalone-metastore/src/main/thrift/hive_metastore.thrift
Lines 1997 (patched)
<https://reviews.apache.org/r/66571/#comment282177>

Update the description ? 
// Revokes all privileges for the object and adds the newly granted 
privileges for it.


- Thejas Nair


On April 13, 2018, 10:30 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66571/
> ---
> 
> (Updated April 13, 2018, 10:30 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19161
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0627c35 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  801de7a 
>   itests/hive-unit/pom.xml f473d25 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/server/TestInformationSchemaWithPrivilege.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/DatabaseAccessorFactory.java
>  7dc690f 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/GenericJdbcDatabaseAccessor.java
>  178c97d 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/JdbcRecordIterator.java
>  638e2b0 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/MsSqlDatabaseAccessor.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/OracleDatabaseAccessor.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/PostgresDatabaseAccessor.java
>  PRE-CREATION 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql 339 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d59bf1f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProvider.java
>  60d9dc1 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveResourceACLsImpl.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 6003ced 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentGroups.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/server/HiveServer2.ja

Re: Review Request 66571: HIVE-19161: Add authorizations to information schema

[Thejas Nair]

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




jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/OracleDatabaseAccessor.java
Lines 23 (patched)
<https://reviews.apache.org/r/66571/#comment282154>

Adding a prefix to it like "dummy_rownum_col_rn1938392" might help with 
debugging later!


- Thejas Nair


On April 13, 2018, 10:30 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66571/
> ---
> 
> (Updated April 13, 2018, 10:30 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19161
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0627c35 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  801de7a 
>   itests/hive-unit/pom.xml f473d25 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/server/TestInformationSchemaWithPrivilege.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/DatabaseAccessorFactory.java
>  7dc690f 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/GenericJdbcDatabaseAccessor.java
>  178c97d 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/JdbcRecordIterator.java
>  638e2b0 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/MsSqlDatabaseAccessor.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/OracleDatabaseAccessor.java
>  PRE-CREATION 
>   
> jdbc-handler/src/main/java/org/apache/hive/storage/jdbc/dao/PostgresDatabaseAccessor.java
>  PRE-CREATION 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql 339 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d59bf1f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/HiveAuthorizationProvider.java
>  60d9dc1 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveResourceACLsImpl.java
>  PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 6003ced 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentGroups.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 6308c5c 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  450da4f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  ebbf465 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  b2c40c2 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  2056930 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  f6c46ee 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856d 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  304f567 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  85c6727 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java
>  7d37262 
> 
> 
> Diff: https://reviews.apache.org/r/66571/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Re: Review Request 66571: HIVE-19161: Add authorizations to information schema

[Thejas Nair]

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2916 (patched)
<https://reviews.apache.org/r/66571/#comment282104>

The syncrhonizer could be eventually moved to metastore as well. 
I think its better to name both synchronizer variables as "hive." instead 
of "hive.server2."


- Thejas Nair


On April 11, 2018, 10:33 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66571/
> ---
> 
> (Updated April 11, 2018, 10:33 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19161
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0627c35 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  801de7a 
>   itests/hive-unit/pom.xml f473d25 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/server/TestInformationSchemaWithPrivilege.java
>  PRE-CREATION 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql 339 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d59bf1f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveResourceACLs.java
>  53e221f 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 6003ced 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentGroups.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 6308c5c 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  450da4f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  ebbf465 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  b2c40c2 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  2056930 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  f6c46ee 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856d 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  304f567 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  85c6727 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java
>  7d37262 
> 
> 
> Diff: https://reviews.apache.org/r/66571/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Re: Review Request 66571: HIVE-19161: Add authorizations to information schema

[Thejas Nair]

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Lines 2914 (patched)
<https://reviews.apache.org/r/66571/#comment281994>

It would be good to add these to "hive.conf.restricted.list" values



metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Lines 1306 (patched)
<https://reviews.apache.org/r/66571/#comment281992>

this is not easy to read where the parenthesis start and end, can you 
reformat with some indendation to make it easier to read ?



ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveResourceACLs.java
Lines 61 (patched)
<https://reviews.apache.org/r/66571/#comment282003>

EnumMap could be used for efficiency here



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
Lines 51 (patched)
<https://reviews.apache.org/r/66571/#comment281993>

the if/else can be replaced with -

enabled = new BooleanWritable(HiveConf.getBoolVar(hiveConf, 
ConfVars.HIVE_SERVER2_RESTRICT_INFORMATION_SCHEMA)



service/src/java/org/apache/hive/service/server/HiveServer2.java
Lines 896 (patched)
<https://reviews.apache.org/r/66571/#comment282004>

does it have to also have dynamic sevice discovery enabled ?
We could assume that if HIVE_SERVER2_PRIVILEGE_SYNCHRONIZER=true and 
namespace is set, zookeeper is setup appropriately.



service/src/java/org/apache/hive/service/server/HiveServer2.java
Lines 905 (patched)
<https://reviews.apache.org/r/66571/#comment282005>

call variable authorizer (not authorizor) ?


- Thejas Nair


On April 11, 2018, 10:33 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66571/
> ---
> 
> (Updated April 11, 2018, 10:33 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19161
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0627c35 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  801de7a 
>   itests/hive-unit/pom.xml f473d25 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/server/TestInformationSchemaWithPrivilege.java
>  PRE-CREATION 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql 339 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d59bf1f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveResourceACLs.java
>  53e221f 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 6003ced 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentGroups.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 6308c5c 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  450da4f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  ebbf465 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  b2c40c2 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  2056930 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  f6c46ee 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856d 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  304f567 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  85c6727 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java
>  7d37262 
> 
> 
> Diff: https://reviews.apache.org/r/66571/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Re: Review Request 66571: HIVE-19161: Add authorizations to information schema

[Thejas Nair]

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




metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql
Line 1079 (original), 1079 (patched)
<https://reviews.apache.org/r/66571/#comment281965>

some use cases of sys is quoted while others are not



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentGroups.java
Lines 38 (patched)
<https://reviews.apache.org/r/66571/#comment281967>

I think something on lines of following better describes it - 
"Returns all groups the current user belongs to"



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 6066 (patched)
<https://reviews.apache.org/r/66571/#comment281983>

This revokes all privileges for the db/table, not just the ones in 
revokePrivileges call.
In that case, I am wondering if it should be passed list 
instead of PrivilegeBag ?
That would make it more clear about the purpose.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
Lines 6108 (patched)
<https://reviews.apache.org/r/66571/#comment281981>

can you add a comment for this block (ie optimization done in that) ?


- Thejas Nair


On April 11, 2018, 10:33 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66571/
> ---
> 
> (Updated April 11, 2018, 10:33 p.m.)
> 
> 
> Review request for hive.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-19161
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 0627c35 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  801de7a 
>   itests/hive-unit/pom.xml f473d25 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/server/TestInformationSchemaWithPrivilege.java
>  PRE-CREATION 
>   metastore/scripts/upgrade/hive/hive-schema-3.0.0.hive.sql 339 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java d59bf1f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/PrivilegeSynchonizer.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveResourceACLs.java
>  53e221f 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 6003ced 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFCurrentGroups.java
>  PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFRestrictInformationSchema.java
>  PRE-CREATION 
>   service/src/java/org/apache/hive/service/server/HiveServer2.java 6308c5c 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  450da4f 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
>  ebbf465 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java
>  b2c40c2 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  2056930 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  f6c46ee 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856d 
>   standalone-metastore/src/main/thrift/hive_metastore.thrift 7450439 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  304f567 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  85c6727 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClientPreCatalog.java
>  7d37262 
> 
> 
> Diff: https://reviews.apache.org/r/66571/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Re: Review Request 66503: HIVE-19126: CachedStore: Use memory estimation to limit cache size during prewarm

[Thejas Nair]

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


Ship it!




Ship It!

- Thejas Nair


On April 11, 2018, 10:53 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66503/
> ---
> 
> (Updated April 11, 2018, 10:53 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-19126
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Diffs
> -
> 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/IncrementalObjectSizeEstimator.java
>  6f4ec6f1ea 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcFileEstimateErrors.java
>  2f7fa24558 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java
>  0bbaf7e459 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856de87 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
>  89b400697b 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  f007261daf 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/SizeValidator.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66503/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: contributor role

[Thejas Nair]

Hi Antal,
Are you referring to contributor role in Jira ?
I think you should be able to assign jiras to yourself without that.
ie, anyone with jira account should be equivalent to a contributor.
Are you facing any issues with that ?

Thanks,
Thejas


On Tue, Apr 10, 2018 at 9:52 AM, Antal Sinkovits
 wrote:
> Hi Team,
>
> Could you please assign contributor role to me. My jira username is
> asinkovits.
> Thanks.
>
> Regards,
> Antal

Re: Review Request 66503: HIVE-19126: CachedStore: Use memory estimation to limit cache size during prewarm

[Thejas Nair]

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


Ship it!




Ship It!

- Thejas Nair


On April 10, 2018, 12:25 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66503/
> ---
> 
> (Updated April 10, 2018, 12:25 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-19126
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Diffs
> -
> 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/IncrementalObjectSizeEstimator.java
>  6f4ec6f1ea 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcFileEstimateErrors.java
>  2f7fa24558 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java
>  0bbaf7e459 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856de87 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
>  89b400697b 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  940a1bf276 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/SizeValidator.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66503/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 66503: HIVE-19126: CachedStore: Use memory estimation to limit cache size during prewarm

[Thejas Nair]

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Line 308 (original), 305 (patched)
<https://reviews.apache.org/r/66503/#comment281644>

(It would be good to point out that this is based on an estimation, not the 
exact memory footprint).

The maximum memory in bytes that the cached objects can use. Memory used is 
calculated based on estimated size of tables and partitions in the cache.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 165 (patched)
<https://reviews.apache.org/r/66503/#comment281645>

I see LLAP uses HiveConf.getSizeVar(conf, ConfVars.LLAP_IO_MEMORY_MAX_SIZE);
 to get a type of size
 can you please check if MetastoreConf.getLongVar(conf, 
ConfVars.CACHED_RAW_STORE_MAX_CACHE_MEMORY) does the conversion to bytes ?


- Thejas Nair


On April 9, 2018, 10:42 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66503/
> ---
> 
> (Updated April 9, 2018, 10:42 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-19126
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Diffs
> -
> 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/IncrementalObjectSizeEstimator.java
>  6f4ec6f1ea 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcFileEstimateErrors.java
>  2f7fa24558 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java
>  0bbaf7e459 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856de87 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
>  89b400697b 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  940a1bf276 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/SizeValidator.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/66503/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 66503: HIVE-19126: CachedStore: Use memory estimation to limit cache size during prewarm

[Thejas Nair]

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 217 (original), 222 (patched)
<https://reviews.apache.org/r/66503/#comment281590>

If the catalogs.add(rawStore.getCatalog(catName)) is moved to another line, 
safer to use {} for a  block under for loop.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
Lines 118 (patched)
<https://reviews.apache.org/r/66503/#comment281591>

I think we can just keep track of size of partitions in it (and perhaps 
tables) since that should be taking almost all the memory.

I am just worried that SharedCache is more complicated structure, and we 
might hit bugs around such structures in estimation. There are whole lot of 
other basic classes that we don't estimate size of.



standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
Lines 1786 (patched)
<https://reviews.apache.org/r/66503/#comment281593>

In master the Validator impls have been moved to separate classes (recent 
commit I suppose). Can yo u also please add SizeValidator as a seperate class ?


- Thejas Nair


On April 9, 2018, 9:57 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66503/
> ---
> 
> (Updated April 9, 2018, 9:57 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-19126
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-19126
> 
> 
> Diffs
> -
> 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/IncrementalObjectSizeEstimator.java
>  6f4ec6f1ea 
>   
> llap-server/src/java/org/apache/hadoop/hive/llap/io/metadata/OrcFileEstimateErrors.java
>  2f7fa24558 
>   
> llap-server/src/test/org/apache/hadoop/hive/llap/cache/TestIncrementalObjectSizeEstimator.java
>  0bbaf7e459 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  c47856de87 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
>  89b400697b 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  995137f967 
> 
> 
> Diff: https://reviews.apache.org/r/66503/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Apache Hive 3.0.0 release preparation

[Thejas Nair]

That needs to be cleaned up. There are far too many right now, its
just not handful of flaky tests.


On Fri, Apr 6, 2018 at 2:48 AM, Peter Vary  wrote:
> Hi Team,
>
> I am new to the Hive release process and it is not clear to me how the 
> failing tests are handled. Do we plan to fix the failing tests before 
> release? Or it is accepted to cut a new major release with known test issues.
>
> Thanks,
> Peter
>
>> On Apr 5, 2018, at 8:25 PM, Vineet Garg  wrote:
>>
>> Hello,
>>
>> I plan to cut off branch for Hive 3.0.0 on Monday (9 April) since bunch of 
>> folks have big patches pending.
>>
>> Regards,
>> Vineet G
>>
>>> On Apr 2, 2018, at 3:14 PM, Vineet Garg  wrote:
>>>
>>> Hello,
>>>
>>> We have enough votes to prepare a release candidate for Hive 3.0.0. I am 
>>> going to cutoff a branch in a day or two. I’ll send an email as soon as I 
>>> have the branch ready.
>>> Meanwhile there are approximately 69 JIRAs which are currently opened with 
>>> fix version 3.0.0. I’ll appreciate if their respective owners would update 
>>> the JIRA if it is a blocker. Otherwise I’ll update them to defer the fix 
>>> version to next release.
>>>
>>> Regards,
>>> Vineet G
>>>
>>
>

Re: Documenting standalone metastore

[Thejas Nair]

Sounds good to me.
We have done something similar in past for HiveServer1 vs HiveServer2,
ReplicationV1 vs V2 etc .


On Mon, Apr 2, 2018 at 10:42 AM, Alan Gates  wrote:
> I've started looking at what will be required to document the new
> standalone metastore.  I started by reviewing the existing admin guide for
> the metastore at
> https://cwiki.apache.org/confluence/display/Hive/AdminManual+MetastoreAdmin
>
> Much of this will need to change for the standalone metastore, as we'll
> need to add in information for the standalone case, many configuration
> values and tool names have changed (though the old ones still work for
> backwards compatibility).  I am concerned that if I put all of the changes
> in this document it will make it hard to use for people administering Hive
> 1 and 2, which is everyone right now and will continue to be a majority for
> at least the next year.
>
> So I propose to add a new page for Hive 3.0, and clearly edit the current
> page to say it only applies to Hive 1 and 2 and put in a pointer to the
> Hive 3 page.  Much of the content of that page will mirror the current one,
> which is painful from a maintenance viewpoint; however, it will produce a
> more usable set of documentation for our users.
>
> Seem reasonable?  I wanted to check before proceeding as this is not how we
> have usually done our documentation.
>
> Alan.

Re: Review Request 66185: JDBC: Provide an option to simplify beeline usage by supporting default and named URL for beeline

[Thejas Nair]

> On March 29, 2018, 11:32 p.m., Vihang Karajgaonkar wrote:
> > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParser.java
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/66185/diff/2/?file=1989804#file1989804line79>
> >
> > Most of the code in this file is duplicated from 
> > UserHS2ConnectionFileParser. This logic would work for what you are trying 
> > to achieve but I think we are doing this in much more complex way than 
> > necessary. Here is my suggestion:
> > 
> > Based on my understanding you have following requirements:
> > 1. User will provide a named url like eg blue using beeline -c blue
> > 2. If the user doesn't provide a named url a default url is used.
> > 
> > You can do this with a small change in the existing code and without 
> > the need to merge the two connection files. In the example above, if the 
> > user doesn't specify -c option above but the beeline-hs2-connection.xml 
> > exists the code is same as what we have currently. (We can obviously 
> > support beeline-site.xml name as well with a trivial change)
> > 
> > if the user specifies beeline -c blue instead, in 
> > UserHS2ConnectionFileParser.getConnectionProperties() instead of looking 
> > for keys starting for beeline.hs2.connection. you can modify the code to 
> > look for keys starting from beeline.hs2.connection.blue. In that case you 
> > reuse most of the existing code and don't need the additional merging 
> > logic. The result is you have only one user connection file and one 
> > hive-site.xml which can be overriden by user connection file.
> > 
> > The only compromise in the above design is that you cannot provide one 
> > long url string like you want to do in beeline-site.xml. If this is not a 
> > hard requirement, I think we will have a much cleaner implementation with 
> > less confusion between having two connection xml files which behave almost 
> > the same way. If its a hard requirement, I guess you can still do by 
> > defining a new key like you are doing currently for beeline-site.xml
> > 
> > Is there a use-case which will not work in my suggestion above? May be 
> > I am missing something very obvious. Would appreciate if you could help me 
> > undestand. Thanks!
> 
> Thejas Nair wrote:
> Vihang, here are the requirements -
> 
> * Beeline can be used in hosts that are not part of the hadoop cluster. 
> hive-site.xml might not be available there. Also, editing the large 
> hive-site.xml (if user is not in the cluster) is going to be complicated for 
> users. So we don't want to rely on hive-site.xml.
> * Having the complete url as a config param in beeline-site.xml makes 
> easy to manage. You don't have to edit multiple parameters to add a new named 
> URL to it. Ambari provides full HS2 jdbc URL that is easy to copy and then 
> add to this file (when not managed by Ambari).
> * If the host is part of a hadoop cluster, then beeline-site.xml can be 
> added by Ambari (or equivalent). However, there are client environment 
> related parameters that user might want to set in addition to that (SSL 
> truststore for example). In that case you don't want the user to modify 
> beeline-site.xml directly as it would get overwritten. So having a second 
> user override .xml file helps users to manage those settings seperately.
> 
> Vihang Karajgaonkar wrote:
> Thanks for the clarifying the details. Would appreciate if can you also 
> clarify the below questions too.
> How do you handle the case where there are multiple HS2 instances on the 
> cluster? Eg. one with Hive 2.3.x and one with Hive 3.0.0?
> 
> Regarding the third point:
> Seems like the order of precendence is beeline-site.xml (first of 
> $HOME/.beeline/beeline-site.xml, $HIVE_CONF_DIR/beeline-site.xml and 
> /etc/hive/conf/beeline-site.xml) which gets overridden by 
> beeline-hs2-connection.xml (first of $HOME/.beeline/beeline-site.xml, 
> $HIVE_CONF_DIR/beeline-site.xml, /etc/hive/conf/beeline-site.xml) which gets 
> merged into hive-site.xml if available.
> I agree with the usecase where user might want to override auto-generated 
> file. Do you think it makes sense to have two different xml file names to do 
> that? Isn't it confusing? Why not make it such that user-specific file in 
> $HOME/.beeline/beeline-site.xml overrides the autogenerated 
> $HIVE_CONF_DIR/beeline-site.xml or /etc/hive/conf/beeline-site.xml? if we do 
> that we can get rid of one additional filename altogether. I am okay if you 
> want to get rid of beeline-hs2-connection.xml if you follow the above 

Re: [VOTE] Apache Hive 2.3.3 Release Candidate 0

[Thejas Nair]

+1
Verified signature & checksum of src and bin tar.gz. Build source and
ran commands from binary.


On Thu, Mar 29, 2018 at 4:37 PM, Alan Gates  wrote:
> +1.  Did a build in an empty repo, ran rat, checked the key signature and
> sha256, checked for any binary objects in the output.
>
> Alan.
>
> On Thu, Mar 29, 2018 at 12:26 AM, Daniel Dai  wrote:
>
>> Apache Hive 2.3.3 Release Candidate 0 is available here:
>>
>> http://people.apache.org/~daijy/apache-hive-2.3.3-rc-0/
>>
>> Maven artifacts are available here:
>>
>> https://repository.apache.org/content/repositories/orgapachehive-1084/
>>
>> Source tag for RCN is at:
>> https://github.com/apache/hive/tree/release-2.3.3-rc0
>>
>> Voting will conclude in 72 hours.
>>
>> Hive PMC Members: Please test and vote.
>>
>> Thanks.
>>

Re: Review Request 66185: JDBC: Provide an option to simplify beeline usage by supporting default and named URL for beeline

[Thejas Nair]

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




beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
Lines 189 (patched)
<https://reviews.apache.org/r/66185/#comment280885>

this might end up matching substrings of new properties.
use delimiters in the search to avoid that ?

Alternatively, should we just apply the configs from jdbcConnectionParams 
at the end ? If the application is in sequence by the jdbc driver, then we 
don't have to bother overriding it, the last values get picked up. (if jdbc 
driver already does that)


- Thejas Nair


On March 28, 2018, 8:27 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66185/
> ---
> 
> (Updated March 28, 2018, 8:27 p.m.)
> 
> 
> Review request for hive, Thejas Nair and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-18963
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 402fae 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineConfFileParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineHS2ConnectionFileParseException.java
>  acddf82a67 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParser.java 
> PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java
>  b769e8581f 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
>  f635b40633 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java
>  2801ebee09 
>   beeline/src/main/resources/BeeLine.properties 6fca953836 
>   
> beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java
>  1d17887417 
>   beeline/src/test/resources/test-hs2-named-connection-config.xml 
> PRE-CREATION 
>   
> itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/BeelineWithHS2ConnectionFileTestBase.java
>  3da31ad8a9 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 6d7787da7d 
> 
> 
> Diff: https://reviews.apache.org/r/66185/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 66185: JDBC: Provide an option to simplify beeline usage by supporting default and named URL for beeline

[Thejas Nair]

> On March 29, 2018, 11:32 p.m., Vihang Karajgaonkar wrote:
> > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParser.java
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/66185/diff/2/?file=1989804#file1989804line79>
> >
> > Most of the code in this file is duplicated from 
> > UserHS2ConnectionFileParser. This logic would work for what you are trying 
> > to achieve but I think we are doing this in much more complex way than 
> > necessary. Here is my suggestion:
> > 
> > Based on my understanding you have following requirements:
> > 1. User will provide a named url like eg blue using beeline -c blue
> > 2. If the user doesn't provide a named url a default url is used.
> > 
> > You can do this with a small change in the existing code and without 
> > the need to merge the two connection files. In the example above, if the 
> > user doesn't specify -c option above but the beeline-hs2-connection.xml 
> > exists the code is same as what we have currently. (We can obviously 
> > support beeline-site.xml name as well with a trivial change)
> > 
> > if the user specifies beeline -c blue instead, in 
> > UserHS2ConnectionFileParser.getConnectionProperties() instead of looking 
> > for keys starting for beeline.hs2.connection. you can modify the code to 
> > look for keys starting from beeline.hs2.connection.blue. In that case you 
> > reuse most of the existing code and don't need the additional merging 
> > logic. The result is you have only one user connection file and one 
> > hive-site.xml which can be overriden by user connection file.
> > 
> > The only compromise in the above design is that you cannot provide one 
> > long url string like you want to do in beeline-site.xml. If this is not a 
> > hard requirement, I think we will have a much cleaner implementation with 
> > less confusion between having two connection xml files which behave almost 
> > the same way. If its a hard requirement, I guess you can still do by 
> > defining a new key like you are doing currently for beeline-site.xml
> > 
> > Is there a use-case which will not work in my suggestion above? May be 
> > I am missing something very obvious. Would appreciate if you could help me 
> > undestand. Thanks!

Vihang, here are the requirements -

* Beeline can be used in hosts that are not part of the hadoop cluster. 
hive-site.xml might not be available there. Also, editing the large 
hive-site.xml (if user is not in the cluster) is going to be complicated for 
users. So we don't want to rely on hive-site.xml.
* Having the complete url as a config param in beeline-site.xml makes easy to 
manage. You don't have to edit multiple parameters to add a new named URL to 
it. Ambari provides full HS2 jdbc URL that is easy to copy and then add to this 
file (when not managed by Ambari).
* If the host is part of a hadoop cluster, then beeline-site.xml can be added 
by Ambari (or equivalent). However, there are client environment related 
parameters that user might want to set in addition to that (SSL truststore for 
example). In that case you don't want the user to modify beeline-site.xml 
directly as it would get overwritten. So having a second user override .xml 
file helps users to manage those settings seperately.


- Thejas


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


On March 28, 2018, 8:27 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66185/
> ---
> 
> (Updated March 28, 2018, 8:27 p.m.)
> 
> 
> Review request for hive, Thejas Nair and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-18963
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 402fae 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineConfFileParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineHS2ConnectionFileParseException.java
>  acddf82a67 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2conne

Re: Review Request 66185: JDBC: Provide an option to simplify beeline usage by supporting default and named URL for beeline

[Thejas Nair]

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




beeline/src/java/org/apache/hive/beeline/BeeLine.java
Lines 1110 (patched)
<https://reviews.apache.org/r/66185/#comment280866>

it would be useful to print the URL as part of this error message.



beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineConfFileParseException.java
Lines 21 (patched)
<https://reviews.apache.org/r/66185/#comment280867>

why is this needed ? This is not marked as serializable.



beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParseException.java
Lines 21 (patched)
<https://reviews.apache.org/r/66185/#comment280868>

looks unnecessary



beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
Lines 148 (patched)
<https://reviews.apache.org/r/66185/#comment280869>

javadoc describing what this is supposed to do would be useful



beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
Lines 156-157 (patched)
<https://reviews.apache.org/r/66185/#comment280870>

String host = getMergedProperty(userConnectionProperties, 
jdbcConnectionParams, HS2ConnectionFileParser.HOST_PROPERTY_KEY, null /*default 
value*/);

would be enable code re-use between this and defaultDb value extraction.


- Thejas Nair


On March 28, 2018, 8:27 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66185/
> ---
> 
> (Updated March 28, 2018, 8:27 p.m.)
> 
> 
> Review request for hive, Thejas Nair and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-18963
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 402fae 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineConfFileParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineHS2ConnectionFileParseException.java
>  acddf82a67 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParser.java 
> PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java
>  b769e8581f 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
>  f635b40633 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java
>  2801ebee09 
>   beeline/src/main/resources/BeeLine.properties 6fca953836 
>   
> beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java
>  1d17887417 
>   beeline/src/test/resources/test-hs2-named-connection-config.xml 
> PRE-CREATION 
>   
> itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/BeelineWithHS2ConnectionFileTestBase.java
>  3da31ad8a9 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 6d7787da7d 
> 
> 
> Diff: https://reviews.apache.org/r/66185/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 66185: JDBC: Provide an option to simplify beeline usage by supporting default and named URL for beeline

[Thejas Nair]

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




beeline/src/java/org/apache/hive/beeline/BeeLine.java
Lines 1110 (patched)
<https://reviews.apache.org/r/66185/#comment280866>

it would be useful to print the URL as part of this error message.



beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineConfFileParseException.java
Lines 21 (patched)
<https://reviews.apache.org/r/66185/#comment280867>

why is this needed ? This is not marked as serializable.



beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParseException.java
Lines 21 (patched)
<https://reviews.apache.org/r/66185/#comment280868>

looks unnecessary



beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
Lines 148 (patched)
<https://reviews.apache.org/r/66185/#comment280869>

javadoc describing what this is supposed to do would be useful



beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
Lines 156-157 (patched)
<https://reviews.apache.org/r/66185/#comment280870>

String host = getMergedProperty(userConnectionProperties, 
jdbcConnectionParams, HS2ConnectionFileParser.HOST_PROPERTY_KEY, null /*default 
value*/);

would be enable code re-use between this and defaultDb value extraction.


- Thejas Nair


On March 28, 2018, 8:27 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66185/
> ---
> 
> (Updated March 28, 2018, 8:27 p.m.)
> 
> 
> Review request for hive, Thejas Nair and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-18963
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 402fae 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineConfFileParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineHS2ConnectionFileParseException.java
>  acddf82a67 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParseException.java
>  PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineSiteParser.java 
> PRE-CREATION 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java
>  b769e8581f 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
>  f635b40633 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java
>  2801ebee09 
>   beeline/src/main/resources/BeeLine.properties 6fca953836 
>   
> beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java
>  1d17887417 
>   beeline/src/test/resources/test-hs2-named-connection-config.xml 
> PRE-CREATION 
>   
> itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/BeelineWithHS2ConnectionFileTestBase.java
>  3da31ad8a9 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java 6d7787da7d 
> 
> 
> Diff: https://reviews.apache.org/r/66185/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: hive dist area questions

[Thejas Nair]

They are related to old CVEs. They provided a way to fix the security
issue without having to upgrade Hive. We don't need them in the
release mirrors as they are very old and and we have had several
releases with the fixes.
Owen and I discussed offline, and they been removed from main download
location and but are still available in the archive.


On Tue, Mar 27, 2018 at 4:15 PM, Owen O'Malley  wrote:
> Thejas,
>
> While uploading the new storage api 2.5.0 release into the dist area, I
> realized that we had some non-releases in there. Thejas aded "ldap-fix" in
> May 2015 and Sushanth added "hive-parent-auth-hook" in Jan 2016. Are they
> releases? Are they out of date?
>
> I also went through the dist area and replaced our old md5 to sha256, so
> that we meet the new requirements for checksums.
>
> Thanks,
>Owen

Re: Review Request 66185: JDBC: Provide an option to simplify beeline usage by supporting default and named URL for beeline

[Thejas Nair]

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




beeline/src/java/org/apache/hive/beeline/BeeLine.java
Lines 321 (patched)
<https://reviews.apache.org/r/66185/#comment280113>

following pattern of other options "named JDBC URL" might be better here.



beeline/src/java/org/apache/hive/beeline/BeeLine.java
Lines 322 (patched)
<https://reviews.apache.org/r/66185/#comment280115>

A comma after "the named JDBC URL to connect to" would make it easier to 
read.

Also, maybe start with upper case "The" ? It looks awkward starting with 
lower case. Maybe we should change the other ones as well in this/another patch.



beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java
Lines 51 (patched)
<https://reviews.apache.org/r/66185/#comment280183>

I was thinking it would be useful to have 'named' url params, and a 
different setting for mapping default to a name.
So that users can easily modify and put that file in a location they 
prefer, but still easily understand their current setting.

For example, the config would have - 

jdbc:/..
jdbc:/..
HS2_LLAP

The user can change default between HS2_TEZ and HS2_LLAP without having to 
lose the descriptive name.



beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
Lines 130 (patched)
<https://reviews.apache.org/r/66185/#comment280180>

It would be useful to print the file name along with error message.
Maybe, instead of looking up all urls, you can pass in the url name (if 
any) to the method in UserHS2ConnectionFileParser ?
That way all the entries in config don't need to be traversed, you can just 
construct the expected config name and look for that.



beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java
Lines 127 (patched)
<https://reviews.apache.org/r/66185/#comment280177>

You can use Configuration.getPropsWithPrefix (since we are using 
Configuration anyway!)



beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java
Line 58 (original), 58 (patched)
<https://reviews.apache.org/r/66185/#comment280184>

A nit -
Adding following method would keep this simpler/easier to read -

getParsedUrlFromConfigFile(String filename) {
 return getParsedUrlFromConfigFile(filename, null);
}
Or perhaps another method for the named url case

getNamedUrlFromConfigFile


- Thejas Nair


On March 20, 2018, 10:54 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66185/
> ---
> 
> (Updated March 20, 2018, 10:54 p.m.)
> 
> 
> Review request for hive, Thejas Nair and Vihang Karajgaonkar.
> 
> 
> Bugs: HIVE-18963
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-18963
> 
> 
> Diffs
> -
> 
>   beeline/src/java/org/apache/hive/beeline/BeeLine.java 402fae 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java
>  b769e8581f 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java
>  f635b40633 
>   
> beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java
>  2801ebee09 
>   beeline/src/main/resources/BeeLine.properties 6fca953836 
>   
> beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java
>  1d17887417 
>   beeline/src/test/resources/test-hs2-named-connection-config.xml 
> PRE-CREATION 
>   
> itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/BeelineWithHS2ConnectionFileTestBase.java
>  3da31ad8a9 
> 
> 
> Diff: https://reviews.apache.org/r/66185/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Hive 3.0 release

[Thejas Nair]

Hi Igor,
For many of the steps in release management, you need to be a Hive committer.
Thanks,
Thejas


On Tue, Mar 20, 2018 at 10:59 PM, Кривенко Ігор  wrote:
> Are there any requirements for being RM ?
> May i be Release manager for 3.0?
>
> Thanks,
> Igor Kryvenko
>
> 2018-03-21 1:46 GMT+02:00 Ashutosh Chauhan :
>
>> Hi all,
>>
>> It's almost a year since we branched off branch-2 Since then there are more
>> than 1700 commits which master has received. There are lots of new features
>> hidden in these commits. I think its about time to make a release so that
>> users can get their hands on them.
>>
>> I propose that we cut a branch for 3.0 by this month end and then target
>> release from it in following weeks.
>> Of course, we want to continue supporting branch-2 as well and make dot
>> releases from it.
>>
>> Thoughts?
>>
>> Thanks,
>> Ashutosh
>>
>> PS: Any volunteers for RM ?
>>

Re: Hive 3.0 release

[Thejas Nair]

+1


On Tue, Mar 20, 2018 at 4:46 PM, Ashutosh Chauhan  wrote:
> Hi all,
>
> It's almost a year since we branched off branch-2 Since then there are more
> than 1700 commits which master has received. There are lots of new features
> hidden in these commits. I think its about time to make a release so that
> users can get their hands on them.
>
> I propose that we cut a branch for 3.0 by this month end and then target
> release from it in following weeks.
> Of course, we want to continue supporting branch-2 as well and make dot
> releases from it.
>
> Thoughts?
>
> Thanks,
> Ashutosh
>
> PS: Any volunteers for RM ?

Re: information_schema in metastore

[Thejas Nair]

INFORMATION_SCHEMA is part of the SQL92 standard. This is a feature
supported and widely used in most mature RDBMs. The idea is to enable
that for Hive as well.

https://issues.apache.org/jira/browse/HIVE-1010

On Fri, Mar 2, 2018 at 10:59 AM, Vihang Karajgaonkar
 wrote:
> I recently noticed the work done in HIVE-16946
>  related to information
> schema. Based on what I understand so far, it uses JdbcSerDe to create hive
> tables similar to metastore db tables. It looks like an interesting idea. Can
> someone share how is it used and what was the motivation to add it?
>
> Thanks,
> Vihang

Re: Review Request 65634: HIVE-18264: CachedStore: Store cached partitions/col stats within the table cache

[Thejas Nair]

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 292 (original), 200 (patched)
<https://reviews.apache.org/r/65634/#comment278193>

It would be more accurate to say "Processed " rather than "Cached " since 
the count includes tables that were skipped.


- Thejas Nair


On Feb. 13, 2018, 12:08 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65634/
> ---
> 
> (Updated Feb. 13, 2018, 12:08 p.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-18264
> https://issues.apache.org/jira/browse/HIVE-18264
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-18264
> 
> 
> Diffs
> -
> 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  78b26374f2 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java
>  d58ed677f3 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/RawStore.java
>  e4e7d4239d 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java
>  f0f650ddcf 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
>  80aa3bcdb4 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
>  32ea17495f 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  9100c73beb 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  86e72d8d76 
>   
> standalone-metastore/src/test/java/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  bd61df654a 
> 
> 
> Diff: https://reviews.apache.org/r/65634/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: reverting test-breaking changes

[Thejas Nair]

+1
I agree, this makes sense. The number of failures keeps increasing.
A 24 hour heads up in either case before revert would be good.


On Thu, Feb 22, 2018 at 2:45 AM, Peter Vary  wrote:

> I agree with Zoltan. The continuously braking tests make it very hard to
> spot real issues.
> Any thoughts on doing it automatically?
>
> > On Feb 22, 2018, at 10:47 AM, Zoltan Haindrich  wrote:
> >
> > *
> >
> > Hello,
> >
> > *
> > *
> >
> > **
> >
> > In the last couple weeks the number of broken tests have started to go
> up...and even tho I run bisect/etc from time to time ; sometimes people
> don’t react to my comments/tickets/etc.
> >
> > Because keeping this many failing tests makes it easier for a new one to
> slip in...I think reverting the patch introducing the test failures would
> also help in some case.
> >
> > I think it would help a lot to prevent further test breaks to revert the
> patch if any of the following conditions is met:
> >
> > *
> > *
> >
> > C1) if the notification/comment about the fact that the patch indeed
> broken a test somehow have been unanswered for at least 24 hours.
> >
> > C2) if the patch is in for 7 days; but the test failure is still not
> addressed (note that in this case there might be a conversation about
> fixing it...but in this case ; to enable other people to work in a cleaner
> environment is more important than a single patch - and if it can't be
> fixed in 7 days...well it might not get fixed in a month).
> >
> > *
> > *
> >
> > I would like to also note that I've seen a few tickets which have been
> picked up by people who were not involved in creating the original change -
> and although the intention was good, they might miss the context of the
> original patch and may "fix" the tests in the wrong way: accept a q.out
> which is inappropriate or ignore the test...
> >
> > *
> > *
> >
> > would it be ok to implement this from now on? because it makes my
> efforts practically useless if people are not reacting…
> >
> > *
> > *
> >
> > note: just to be on the same page - this is only about running a single
> test which falls on its own - I feel that flaky tests are an entirely
> different topic.
> >
> > *
> > *
> >
> > cheers,
> >
> > Zoltan
> >
> > **
> > *
>
>

Re: Yetus JDK version

[Thejas Nair]

+ Peter, Adam (The Yetus experts)


On Thu, Feb 1, 2018 at 9:49 AM, Alan Gates  wrote:

> Ok, looking briefly at it, it looks like if we changed
> testutils/…/TestScripts.java line 76 to set javaHome to 1.8 instead of 1.7
> that we’ll be running ptest with 1.8.  I’m not familiar with ptest, but I’m
> guessing that someone would need to make this change and then re-deploy
> ptest in our test infrastructure.  Is there anything else we need to do?
> We clearly already have 1.8 installed on the test machines because the code
> compiles with 1.8, but I don’t know what the path is, etc.
>
> Alan.
>
> On Tue, Jan 30, 2018 at 4:58 PM, Alan Gates  wrote:
>
> > I put code in the latest patch for HIVE-17983 that executes one of the
> > compiled classes as part of the maven build.  (It does this to
> > automatically generate the config template.)  This works locally and in
> the
> > ptest build.  But in the Yetus tests it fails with:
> >
> > Exception in thread "main" java.lang.UnsupportedClassVersionError:
> > org/apache/hadoop/hive/metastore/conf/ConfTemplatePrinter : Unsupported
> > major.minor version 52.0
> >
> > This means that it is compiling with JDK 1.8 but running it with 1.7.
> How
> > do we switch the Yetus build so it runs maven with the correct JDK
> version?
> >
> > Alan.
> >
>

Re: Review Request 65271: JDBC: Provide a way for JDBC users to pass cookie info via connection string

[Thejas Nair]

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




jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java
Line 91 (original), 96 (patched)
<https://reviews.apache.org/r/65271/#comment276342>

A nit - 
I think using "+=" is more readable for appends -

cookieHeaderKeyValues +=
   ";" + entry.getKey() + "=" + entry.getValue();


- Thejas Nair


On Jan. 31, 2018, 10:50 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65271/
> ---
> 
> (Updated Jan. 31, 2018, 10:50 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-18447
> https://issues.apache.org/jira/browse/HIVE-18447
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-18447
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/thrift/TestThriftHttpCLIServiceFeatures.java
>  93b10fb4b4 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cb2f09cbf2 
>   jdbc/src/java/org/apache/hive/jdbc/HttpBasicAuthInterceptor.java 5d2ddb5c21 
>   jdbc/src/java/org/apache/hive/jdbc/HttpKerberosRequestInterceptor.java 
> 37862be804 
>   jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java 
> cf1a11ecb6 
>   jdbc/src/java/org/apache/hive/jdbc/HttpTokenAuthInterceptor.java 59a91dd14c 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java f7f3854b86 
> 
> 
> Diff: https://reviews.apache.org/r/65271/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Branch for "Per Table Write ID" implementation

[Thejas Nair]

+1
Makes sense to split the changes into multiple smaller patches that are
easier to review, and creating this branch would help with that.



On Tue, Jan 9, 2018 at 10:55 PM, Sankar Hariappan <
shariap...@hortonworks.com> wrote:

> Hi all,
>
> "Hive Replication” feature is advancing to support ACID tables (HIVE-18320<
> https://issues.apache.org/jira/browse/HIVE-18320>).
> “Per Table Write ID” is an important requirement to support replication
> for ACID tables especially for the use case of “Analytics workload
> off-loading for scalability”. Details are available in the design document
> attached in the JIRA.
>
> Per table Write ID implementation have several changes.
>
>   1.  Add metadata tables to allocate and manage write ID. Also, map it
> against global transaction.
>   2.  Handle snapshot isolation for ACID/MM table reads by using
> ValidWriteIDList instead of ValidTxnList.
>   3.  Modify ORC/Hive row readers to use ValidWriteIDList instead of
> ValidTxnList to read valid delta/base directories.
>   4.  Update ValidCompactorTxnList to use table Write Ids.
>   5.  Upgrade from existing Hive versions by migrating the ACID/MM tables
> to use Write ID instead of global transaction ID.
>   6.  Correct the UT test scripts to use ValidWriteIDList instead of
> ValidTxnList for snapshot isolation tests.
>   7.  Rename the method/variable names of several classes to use WriteId
> instead of TxnId.
>
> As part of HIVE-18192,
> I have implemented first 3 changes in the list which makes ACID read/write
> to work with Write ID change. But, this feature will be incomplete without
> rest of the changes.
>
> Hence, I would like to create a branch (branch-per-table-writeid) from
> master to commit this feature with multiple patches. This branch is
> expected to be short-lived for 2 to 3 weeks.
>
> Request feedback from the community.
>
> Best regards
> Sankar
>
>

Re: Review Request 64925: HIVE-18349: Misc metastore changes for debuggability

[Thejas Nair]

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 1635 (patched)
<https://reviews.apache.org/r/64925/#comment273644>

looks like this success boolean is not tracking success of commit.
Exception is already being thrown from above stack.


- Thejas Nair


On Jan. 3, 2018, 8:09 p.m., Prasanth_J wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64925/
> ---
> 
> (Updated Jan. 3, 2018, 8:09 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-18349
> https://issues.apache.org/jira/browse/HIVE-18349
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-18349: Misc metastore changes for debuggability
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  f1b58c526db15cfdf9098af603549643c0f4dd13 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  2e43dc85eae2490683a52444c9456e201697a3b3 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/datasource/BoneCPDataSourceProvider.java
>  4ff2bb77d387bd2182f8cdd248303edd15dd12bd 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/datasource/HikariCPDataSourceProvider.java
>  6ffc24a27a452f9a063831a2a1d39a7b1be5d6b5 
> 
> 
> Diff: https://reviews.apache.org/r/64925/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Prasanth_J
> 
>

Re: Review Request 64925: HIVE-18349: Misc metastore changes for debuggability

[Thejas Nair]

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




standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Lines 882 (patched)
<https://reviews.apache.org/r/64925/#comment273643>

passing table info as argument seems like the better option, since 
endFunction can be called from methods that don't have table info.
(similar to the way startTableFunction is used from drop_table_..)


- Thejas Nair


On Jan. 3, 2018, 8:09 p.m., Prasanth_J wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64925/
> ---
> 
> (Updated Jan. 3, 2018, 8:09 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-18349
> https://issues.apache.org/jira/browse/HIVE-18349
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-18349: Misc metastore changes for debuggability
> 
> 
> Diffs
> -
> 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
>  f1b58c526db15cfdf9098af603549643c0f4dd13 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java
>  2e43dc85eae2490683a52444c9456e201697a3b3 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/datasource/BoneCPDataSourceProvider.java
>  4ff2bb77d387bd2182f8cdd248303edd15dd12bd 
>   
> standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/datasource/HikariCPDataSourceProvider.java
>  6ffc24a27a452f9a063831a2a1d39a7b1be5d6b5 
> 
> 
> Diff: https://reviews.apache.org/r/64925/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Prasanth_J
> 
>

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

[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 <alanfga...@gmail.com> 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 <pv...@cloudera.com> 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 <thejas.n...@gmail.com>
> 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 <pv...@cloudera.com>
> 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
> > >>

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

[Thejas Nair]

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: adding a label that would trigger HiveQA?

[Thejas Nair]

+1
It would be great if label could be added automatically and it gets removed
after HiveQA run


On Fri, Nov 10, 2017 at 5:05 PM, Sergey Shelukhin 
wrote:

> Resubmitting the same patch for HiveQA as patches are constantly getting
> dropped is getting old.
> I wonder if we should have a label that would trigger HiveQA and only be
> removed at the end, when posting results to the JIRA?
> We could either add it in addition to the current filter or trigger
> mechanism or, if JIRA allows that, make it the only selection criteria and
> add it automatically when clicking submit patch (+remove it on cancel
> patch and when HiveQA has finished).
>
> Comments/suggestions?
> I can try to modify the scripts some day.
>
>

Re: Integrating Yetus with Precommit job

[Thejas Nair]

+1
Yes, I think this can help us catch many issues early on, it will be very
useful!


On Mon, Nov 6, 2017 at 7:43 AM, Adam Szita  wrote:

> Hi all,
>
> As a next step of test subsystem improvements we would like to have the
> Yetus check integrated with the ptest framework. This means that whenever a
> new patch is uploaded - along with the already existing Precommit test run
> - Hive's Yetus patch check script would be triggered also. This script runs
> checkstyle, findbugs, ASF license check, etc with and without the submitted
> patch applied and reports the diffs (i.e. how many checkstyle problems does
> the patch introduce).
>
> It would be executed parallel to the ptest test execution and report back
> the results as a (another) jira comment to the issue in question.
> In the last days I've been working on this (HIVE-16748) and a patch is
> ready to make this happen. A sample Yetus result comment is available at
> https://issues.apache.org/jira/browse/HIVE-16748?
> focusedCommentId=16218616=com.atlassian.jira.
> plugin.system.issuetabpanels:comment-tabpanel#comment-16218616
>
> We think this would be a useful tool for us developers and would like to go
> ahead with this change, but we're also curious about your input in this
> matter. Please let us know what you think about this change.
>
> Thanks,
> Adam
>

Re: Review Request 63458: HIVE-16917: HiveServer2 guard rails - Limit concurrent connections from user

[Thejas Nair]

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


Ship it!




Ship It!

- Thejas Nair


On Nov. 2, 2017, 6:25 p.m., Prasanth_J wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63458/
> ---
> 
> (Updated Nov. 2, 2017, 6:25 p.m.)
> 
> 
> Review request for hive, Sergey Shelukhin and Thejas Nair.
> 
> 
> Bugs: HIVE-16917
> https://issues.apache.org/jira/browse/HIVE-16917
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16917: HiveServer2 guard rails - Limit concurrent connections from user
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a3c853a 
>   
> itests/hive-unit/src/test/java/org/apache/hive/service/cli/session/TestHiveSessionImpl.java
>  ebcf4a8 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionBase.java 
> 9436a25 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 
> 0206fe3 
>   
> service/src/java/org/apache/hive/service/cli/session/HiveSessionImplwithUGI.java
>  8975aee 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 
> 9b2ae57 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> 6354c8c 
>   
> service/src/test/org/apache/hive/service/cli/TestCLIServiceConnectionLimits.java
>  PRE-CREATION 
>   
> service/src/test/org/apache/hive/service/cli/session/TestPluggableHiveSessionImpl.java
>  47f95c5 
> 
> 
> Diff: https://reviews.apache.org/r/63458/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Prasanth_J
> 
>

Re: Review Request 63458: HIVE-16917: HiveServer2 guard rails - Limit concurrent connections from user

[Thejas Nair]

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




service/src/java/org/apache/hive/service/cli/session/SessionManager.java
Lines 477 (patched)
<https://reviews.apache.org/r/63458/#comment267171>

Can you also log the key on which limit has been reached ? (username and/or 
ipaddress).
It would help with troubleshooting from the server logs.



service/src/java/org/apache/hive/service/cli/session/SessionManager.java
Lines 508 (patched)
<https://reviews.apache.org/r/63458/#comment267174>

safer to check - if( connectionCount >= limit )


- Thejas Nair


On Nov. 2, 2017, 8:08 a.m., Prasanth_J wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63458/
> ---
> 
> (Updated Nov. 2, 2017, 8:08 a.m.)
> 
> 
> Review request for hive, Sergey Shelukhin and Thejas Nair.
> 
> 
> Bugs: HIVE-16917
> https://issues.apache.org/jira/browse/HIVE-16917
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> HIVE-16917: HiveServer2 guard rails - Limit concurrent connections from user
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a3c853a 
>   service/src/java/org/apache/hive/service/cli/session/SessionManager.java 
> 9b2ae57 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 
> 6354c8c 
>   
> service/src/test/org/apache/hive/service/cli/TestCLIServiceConnectionLimits.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63458/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Prasanth_J
> 
>

Re: Client compatibility for MetaStoreFilterHook

[Thejas Nair]

Hi Alex,
Sorry about the delay in responding. I have replied in the jira.

Thanks,
Thejas



On Mon, Oct 30, 2017 at 5:41 PM, Alexander Kolbasov 
wrote:

> Looking at https://issues.apache.org/jira/browse/HIVE-9350 (Add ability
> for
> HiveAuthorizer implementations to filter out results of 'show tables',
> 'show databases') I see that it changed methods of MetaStoreFilterHook to
> throw exceptions which id didn't throw before. This means that any clients
> using it require recompilation (and potential code changes to handle
> exceptions) which is a compatibility issue. This was done in 1.2.0 - IN am
> wondering how such compatibility issues were handled in Hive 1 and Hive 2
> branches?
>
> Thanks for clarification,
>
> - Alex
>

Re: Review Request 62695: HIVE-17649: Export/Import: Move export data write to a task

[Thejas Nair]

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


Ship it!




Ship It!

- Thejas Nair


On Oct. 2, 2017, 10:55 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62695/
> ---
> 
> (Updated Oct. 2, 2017, 10:55 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-17649
> https://issues.apache.org/jira/browse/HIVE-17649
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17649
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExportTask.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java fe9b624 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java f9991d9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ExportSemanticAnalyzer.java 
> b8c6ea9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/PartitionExport.java 
> 7e72f23 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/TableExport.java 
> ed43272 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExportWork.java PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_export.q.out 19c8115 
>   ql/src/test/results/clientnegative/exim_12_nonnative_export.q.out 5da4daa 
> 
> 
> Diff: https://reviews.apache.org/r/62695/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 62695: HIVE-17649: Export/Import: Move export data write to a task

[Thejas Nair]

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




ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/TableExport.java
Line 62 (original), 62 (patched)
<https://reviews.apache.org/r/62695/#comment263781>

can you also remove this member variable?


- Thejas Nair


On Oct. 2, 2017, 8:27 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62695/
> ---
> 
> (Updated Oct. 2, 2017, 8:27 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-17649
> https://issues.apache.org/jira/browse/HIVE-17649
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17649
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExportTask.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java fe9b624 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java f9991d9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ExportSemanticAnalyzer.java 
> b8c6ea9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/TableExport.java 
> ed43272 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExportWork.java PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_uri_export.q.out 19c8115 
>   ql/src/test/results/clientnegative/exim_12_nonnative_export.q.out 5da4daa 
> 
> 
> Diff: https://reviews.apache.org/r/62695/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: [Announce] New committer: Sankar Hariappan

[Thejas Nair]

Congrats Sankar!

On Fri, Sep 29, 2017 at 11:36 AM, Peter Vary  wrote:

> Congratulations Sankar!
>
> > On Sep 29, 2017, at 7:56 PM, Ashutosh Chauhan 
> wrote:
> >
> > The Project Management Committee (PMC) for Apache Hive has invited Sankar
> > Harriapan to become a committer and we are pleased to announce that he
> has
> > accepted.
> >
> > Welcome, Sankar!
> >
> > Thanks,
> > Ashutosh
>
>

Re: [Announce] New committer: Anishek Agarwal

[Thejas Nair]

Congrats Anishek!

On Fri, Sep 29, 2017 at 11:36 AM, Peter Vary  wrote:

> Congratulations Anishek!
>
> > On Sep 29, 2017, at 7:55 PM, Ashutosh Chauhan 
> wrote:
> >
> > The Project Management Committee (PMC) for Apache Hive has invited
> Anishek
> > Agarwal to become a committer and we are pleased to announce that he has
> > accepted.
> >
> > Welcome, Anishek!
> >
> > Thanks,
> > Ashutosh
>
>

Re: Review Request 62695: HIVE-17649: Export/Import: Move export data write to a task

[Thejas Nair]

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




ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/TableExport.java
Line 80 (original), 82 (patched)
<https://reviews.apache.org/r/62695/#comment263534>

return void here ?
And remove AuthEntities member variable as well ?



ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/TableExport.java
Line 159 (original), 161 (patched)
<https://reviews.apache.org/r/62695/#comment263533>

the authEntitites computation can be skipped with this change. right ?


- Thejas Nair


On Sept. 29, 2017, 5:31 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62695/
> ---
> 
> (Updated Sept. 29, 2017, 5:31 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Bugs: HIVE-17649
> https://issues.apache.org/jira/browse/HIVE-17649
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-17649
> 
> 
> Diffs
> -
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/ExportTask.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java fe9b624 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/repl/ReplDumpTask.java f9991d9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ExportSemanticAnalyzer.java 
> b8c6ea9 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/repl/dump/TableExport.java 
> ed43272 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/ExportWork.java PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62695/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




ql/src/java/org/apache/hadoop/hive/ql/Driver.java
Line 718 (original)
<https://reviews.apache.org/r/62373/#comment262475>

KILL_QUERY shoudl also not need any txn to be started. Why remove it from 
here ?



ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
Line 3041 (original), 3041 (patched)
<https://reviews.apache.org/r/62373/#comment262476>

Why the change in log message ? the use of "{}" is preferred as it doesn't 
involve string concatenation if that log level isn't enabled.



ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java
Line 2582 (original), 2583 (patched)
<https://reviews.apache.org/r/62373/#comment262477>

Is there a problem if we call the authorization method even if hs2Hostname 
is null ?
It seems safer secuirty practise to make the authorization call even if the 
hs2Hostname is null for some reason.



service/src/java/org/apache/hive/service/cli/CLIService.java
Lines 614 (patched)
<https://reviews.apache.org/r/62373/#comment262478>

any reason for this change ? QueryState seems to provide a good abstraction 
for queryId



service/src/java/org/apache/hive/service/cli/operation/OperationManager.java
Line 63 (original)
<https://reviews.apache.org/r/62373/#comment262479>

why revert this approach of tracking the mapping using the hashmap ? That 
looks like more optimized route and cleaner as well.


- Thejas Nair


On Sept. 23, 2017, 7:53 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 23, 2017, 7:53 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniLlap.java 
> 28fa7a5783 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java bfae8b9e41 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 
> 8d6003ad06 
>   metastore/src/gen/thrift/gen-py/__init__.py e69de29bb2 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 6616cba7d1 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java ad5b3a3098 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/jav

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




service/src/java/org/apache/hive/service/server/KillQueryImpl.java
Lines 39 (patched)
<https://reviews.apache.org/r/62373/#comment262184>

the tool or user might not know which HS2 instance has query id, it is 
possible that this one doesn't have it. The current code can result in an NPE.
It would be better to just log that information and return.


- Thejas Nair


On Sept. 20, 2017, 9:24 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 20, 2017, 9:24 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniLlap.java 
> 28fa7a5783 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   itests/src/test/resources/testconfiguration.properties d472bb3f9e 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java bfae8b9e41 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 
> 8d6003ad06 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4e7c80f184 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 714ea1f3fd 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/KillQuery.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/NullKillQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 97c8124293 
>   
> ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java
>  1a89eb1263 
>   ql/src/test/queries/clientnegative/authorization_kill_query.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/kill_query.q PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_kill_query.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/kill_query.q.out PRE-CREATION 
>   service-rpc/if/TCLIService.thrift 976ca9b6b3 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService.h 5fd423da6e 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService.cpp 3597d44f2d 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_constants.cpp 874a81bf6b 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_server.skeleton.cpp 
> 5d7caf9783 
>   service-rpc/src/gen/thr

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java
Lines 42 (patched)
<https://reviews.apache.org/r/62373/#comment262126>

I think it would be cleaner if new instance of ZooKeeperHiveClientHelper 
doesn't have to be created.
The lifetime of these objects is only until the 'public' methods of this 
api is invoked, so it would be good to have configureConnParams and 
getDirectParamsList as static methods.

Maybe break up preConfigureConnParams into two methods (getZkClient and 
getServerHosts);
ie 
void configureConnParams(JdbcConnectionParams connParams) throws 
ZooKeeperHiveClientException {
try {
  CuratorFramework zooKeeperClient = getZkClient(connParams);
List serverHosts = getServerHosts(zooKeeperClient);
..
..


- Thejas Nair


On Sept. 20, 2017, 12:31 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 20, 2017, 12:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscovery.java 
> b153679dc8 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   itests/src/test/resources/testconfiguration.properties d472bb3f9e 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java bfae8b9e41 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 
> 8d6003ad06 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4e7c80f184 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 8aa2d90b76 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/KillQuery.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/NullKillQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 8b64407d53 
>   
> ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java
>  1a89eb1263 
>   ql/src/test/queries/clientnegative/authorization_kill_query.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/kill_query.q PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_kill_query.q.out 
> PRE-CREATION 
>   ql/src/test

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




service/src/java/org/apache/hive/service/cli/operation/OperationManager.java
Line 218 (original), 222 (patched)
<https://reviews.apache.org/r/62373/#comment262124>

it would be cleaner to just call removeOperation instead of following - 

handleToOperation.remove(operationHandle, operation);
  queryIdToOperation.remove(operation.getQueryState().getQueryId(), 
operation);
  if (operation instanceof SQLOperation) {
removeSafeQueryInfo(operationHandle);
  }
  return operation;
}



jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java
Lines 122 (patched)
<https://reviews.apache.org/r/62373/#comment262125>

can you remove these whitespaces that show up as red


- Thejas Nair


On Sept. 20, 2017, 12:31 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 20, 2017, 12:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscovery.java 
> b153679dc8 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   itests/src/test/resources/testconfiguration.properties d472bb3f9e 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java bfae8b9e41 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 
> 8d6003ad06 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4e7c80f184 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 8aa2d90b76 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/KillQuery.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/NullKillQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 8b64407d53 
>   
> ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java
>  1a89eb1263 
>   ql/src/test/queries/clientnegative/authorization_kill_query.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/kill_query.q PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_kill_query.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/kill_query.q.out PRE-CREATI

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java
Lines 40 (patched)
<https://reviews.apache.org/r/62373/#comment262119>

this is used only in preConfigureConnParams, doesn't need to be a class 
variable



jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java
Lines 41 (patched)
<https://reviews.apache.org/r/62373/#comment262120>

this is used only in preConfigureConnParams, doesn't need to be a class 
variable



jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java
Line 73 (original), 73 (patched)
<https://reviews.apache.org/r/62373/#comment262118>

maybe call it updateParamsWithZKServerNode ?



jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java
Lines 131 (patched)
<https://reviews.apache.org/r/62373/#comment262117>

applyFromServerNode can call 
directConnParams.setCurrentHostZnodePath(serverNode)  instead of it being 
called from outside right before it, as it is part of the setup for a specific 
zk node that got picked.


- Thejas Nair


On Sept. 20, 2017, 12:31 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 20, 2017, 12:31 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscovery.java 
> b153679dc8 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   itests/src/test/resources/testconfiguration.properties d472bb3f9e 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java bfae8b9e41 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 
> 8d6003ad06 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4e7c80f184 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 8aa2d90b76 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/KillQuery.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/NullKillQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 8b64407d53 
>   
> ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java
>  1a89eb1263 
>   ql/src/test/queries/clientnegativ

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
Lines 130 (patched)
<https://reviews.apache.org/r/62373/#comment262109>

unused. remove ?



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
Lines 132 (patched)
<https://reviews.apache.org/r/62373/#comment262110>

unused. remove ?



jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java
Lines 633 (patched)
<https://reviews.apache.org/r/62373/#comment262111>

Add a comment that this is internal api to be used for testing ?
You can use @VisibleForTesting annotation.


- Thejas Nair


On Sept. 19, 2017, 9:28 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 19, 2017, 9:28 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscovery.java 
> b153679dc8 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   itests/src/test/resources/testconfiguration.properties d472bb3f9e 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java bfae8b9e41 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 
> 8d6003ad06 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4e7c80f184 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 8aa2d90b76 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/KillQuery.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/NullKillQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 8b64407d53 
>   
> ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java
>  1a89eb1263 
>   ql/src/test/queries/clientnegative/authorization_kill_query.q PRE-CREATION 
>   ql/src/test/queries/clientpositive/kill_query.q PRE-CREATION 
>   ql/src/test/results/clientnegative/authorization_kill_query.q.out 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/llap/kill_query.q.out PRE-CREATION 
>   service-rpc/if/TCLIService.thrift 976ca9b6b3 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService.h 5fd423da6e 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService.cpp 3597d

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
Lines 1452 (patched)
<https://reviews.apache.org/r/62373/#comment262105>

the fail, assert etc from this thread would not cause the test to actually 
fail.
Only fail/asserts etc in main test method thread would have an impact.
See 
https://stackoverflow.com/questions/2596493/junit-assert-in-thread-throws-exception

Maybe just capture it and validate using uncaughtexceptionhandler ?



itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
Lines 1460 (patched)
<https://reviews.apache.org/r/62373/#comment262103>

lets call it "killing the query" and name the thread tKill



itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
Lines 1465 (patched)
<https://reviews.apache.org/r/62373/#comment262104>

time mentioned in the comment is incorrect. maybe just remove the comment, 
the next line is quite obvious.



itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
Lines 1473 (patched)
<https://reviews.apache.org/r/62373/#comment262106>

let the exception propogate to uncaughtexceptionhandler ?


- Thejas Nair


On Sept. 19, 2017, 9:28 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 19, 2017, 9:28 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscovery.java 
> b153679dc8 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   itests/src/test/resources/testconfiguration.properties d472bb3f9e 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   jdbc/src/java/org/apache/hive/jdbc/Utils.java bfae8b9e41 
>   jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java 
> 8d6003ad06 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4e7c80f184 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 8aa2d90b76 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/KillQuery.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/NullKillQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/Session

Re: Review Request 62373: [HIVE-17483] HS2 kill command to kill queries using query id

[Thejas Nair]

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




itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
Lines 1425 (patched)
<https://reviews.apache.org/r/62373/#comment261944>

remove ?



itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java
Lines 1439 (patched)
<https://reviews.apache.org/r/62373/#comment261945>

parallel statement execution within same connection can have issues. Better 
to use 2nd connection for kill query



jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
Lines 181 (patched)
<https://reviews.apache.org/r/62373/#comment261947>

Can we re-use methods used by regular HiveConnection constructor code path 
to get the list of URLS ?

The duplication of code to read from ZK again here can be a maintainence 
problem.



service/src/java/org/apache/hive/service/cli/operation/OperationManager.java
Lines 386 (patched)
<https://reviews.apache.org/r/62373/#comment261946>

maintaining a hashmap of queryid to operation will help with performance, 
and also the case of killing from same connection.


- Thejas Nair


On Sept. 18, 2017, 4:57 a.m., Teddy Choi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62373/
> ---
> 
> (Updated Sept. 18, 2017, 4:57 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-17483
> https://issues.apache.org/jira/browse/HIVE-17483
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> For administrators, it is important to be able to kill queries if required. 
> Currently, there is no clean way to do it.
> It would help to have a "kill query " command that can be run using 
> odbc/jdbc against a HiveServer2 instance, to kill a query with that queryid 
> running in that instance.
> Authorization will have to be done to ensure that the user that is invoking 
> the API is allowed to perform this action.
> In case of SQL std authorization, this would require admin role.
> 
> 
> Diffs
> -
> 
>   itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java 
> 1108934df2 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniHS2.java 
> 4a9af80fdc 
>   
> itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestServiceDiscoveryWithMiniHS2.java
>  e8051e40f2 
>   itests/src/test/resources/testconfiguration.properties d472bb3f9e 
>   jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java 1311d2d88c 
>   jdbc/src/java/org/apache/hive/jdbc/HiveStatement.java c6bd41feb7 
>   ql/src/java/org/apache/hadoop/hive/ql/Driver.java 4e7c80f184 
>   ql/src/java/org/apache/hadoop/hive/ql/QueryState.java fa7c32386b 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 8aa2d90b76 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/Entity.java 131c1e1bb5 
>   ql/src/java/org/apache/hadoop/hive/ql/hooks/WriteEntity.java da8c1e2305 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/DDLSemanticAnalyzer.java 
> 251decac9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g b5792ac485 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 429e0d995a 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 003e09fd13 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java 
> 553dd64b5f 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/DDLWork.java 2b9e897a54 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/HiveOperation.java e1f1f53c3c 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/KillQueryDesc.java PRE-CREATION 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/AuthorizationUtils.java
>  04e5565506 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HiveOperationType.java
>  3af97ea02f 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/HivePrivilegeObject.java
>  41983f1b4c 
>   
> ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/Operation2Privilege.java
>  da99972e0c 
>   ql/src/java/org/apache/hadoop/hive/ql/session/KillQuery.java PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/NullKillQuery.java 
> PRE-CREATION 
>   ql/src/java/org/apache/hadoop/hive/ql/session/SessionState.java 8b64407d53 
>   
> ql/src/test/org/apache/hadoop/hive/ql/parse/TestSQL11ReservedKeyWordsNegative.java
>  1a89eb1263 
>   ql/src/test/queries/clientnegative/authorization_kill_query.q PRE-CREATION 
>   ql/src/te

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

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




metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java
Line 266 (original)
<https://reviews.apache.org/r/59205/#comment248479>

don't we need this removePartitionColStatsFromCache call ?


- Thejas Nair


On May 16, 2017, 8:10 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 16, 2017, 8:10 a.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 1c37b6e 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  91a3a38 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java d296851 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> b28983f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c1af690 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/StatObjectConverter.java 
> fcf6f27 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> 668499b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 1cc838f 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> 0c7d8bb 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/ColumnStatsMergerFactory.java
>  da6cd46 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/hbase/stats/merge/DateColumnStatsMerger.java
>  PRE-CREATION 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  f613c30 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1720e37 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

> On May 15, 2017, 8:19 a.m., Thejas Nair wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
> > Line 281 (original), 284 (patched)
> > <https://reviews.apache.org/r/59205/diff/2-5/?file=1717962#file1717962line285>
> >
> > This needs to be TimeUnit.MILLISECONDS, as you want it converted to 
> > MILLISECONDS

the signature is - getTimeVar(Configuration conf, ConfVars var, TimeUnit 
outUnit)


- Thejas


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


On May 15, 2017, 7:59 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 15, 2017, 7:59 a.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d6a80ae 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  91a3a38 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b897ffa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> ed19f42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c1af690 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> 668499b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 5a187d8 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> 0c7d8bb 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  f613c30 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1720e37 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

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




metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 281 (original), 284 (patched)
<https://reviews.apache.org/r/59205/#comment248225>

This needs to be TimeUnit.MILLISECONDS, as you want it converted to 
MILLISECONDS



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 512 (original), 526 (patched)
<https://reviews.apache.org/r/59205/#comment248226>

isDatabaseCacheDirty.set(true); - this needs to be set after obtaining the 
lock

Otherwise, you can have case where it gets reset by background thread 
before actual update.

(Same with this call in other places)


- Thejas Nair


On May 15, 2017, 7:59 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 15, 2017, 7:59 a.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d6a80ae 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  91a3a38 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b897ffa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> ed19f42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c1af690 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> 668499b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 5a187d8 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> 0c7d8bb 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  f613c30 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1720e37 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

> On May 14, 2017, 7:38 a.m., Daniel Dai wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
> > Line 381 (original), 526 (patched)
> > <https://reviews.apache.org/r/59205/diff/4/?file=1717995#file1717995line552>
> >
> > I think we only need a dirty flag, it will be reset by the next 
> > refresh, not end of individual call.

I agree, this activeDatabaseUpdateCalls params are not going to prevent the 
race-condition. Its better to use dirty flag instead.


- Thejas


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


On May 14, 2017, 5:19 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 14, 2017, 5:19 a.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d6a80ae 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  91a3a38 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b897ffa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> ed19f42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c1af690 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> 668499b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 5a187d8 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> 0c7d8bb 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  f613c30 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1720e37 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

> On May 14, 2017, 7:38 a.m., Daniel Dai wrote:
> > metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
> > Lines 528 (patched)
> > <https://reviews.apache.org/r/59205/diff/4/?file=1717995#file1717995line554>
> >
> > If using dirty flag, databaseCacheLock can be removed, right?

We still need some locking/synchronization to prevent things like this -


1. If (isDirtyFlag == false) {
2.  // if db gets updated in another thread while this thread is here, we lose 
the update.
3.updateDBCache()
2. }

A read/write lock similar to what is used here appropriate for that. The 
background thread is the one that holds the write lock.
However, not sure if we need to busy-wait with trylock though. (I figure, the 
intent is to give this one higher priority). The update DbCache part should not 
take that long. (The busy-wait can be dealt with later also).


- Thejas


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


On May 14, 2017, 5:19 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 14, 2017, 5:19 a.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d6a80ae 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  91a3a38 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b897ffa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> ed19f42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c1af690 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> 668499b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 5a187d8 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> 0c7d8bb 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  f613c30 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1720e37 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

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




metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Lines 341 (patched)
<https://reviews.apache.org/r/59205/#comment248175>

this call has already been made in updateTables. Should we just get it from 
the cache to reduce the rawStore call ? This can be done as part of follow up 
jira if necessary.



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 300 (original), 374 (patched)
<https://reviews.apache.org/r/59205/#comment248179>

This still doesn't prevent the race condition I mentioned (in offline 
discussion).
If another thread calls a method that updates a database object, when this 
thread is here (let say that calls starts and finishes when this thread is at 
this line), it would still go ahead and update the cache. That would result in 
local changes getting lost.

This is not a regression in this patch, we can address that in a follow up 
patch.
To address that, we need a dirty-cache check after acquring the lock.



metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
Line 1131 (original), 1433 (patched)
<https://reviews.apache.org/r/59205/#comment248176>

this would be just like getPartition() . We get it from cache, if its there 
in cache. Otherwise, assume its not there (yet). This can be done as part of 
follow up jira if necessary.


- Thejas Nair


On May 14, 2017, 5:19 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 14, 2017, 5:19 a.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d6a80ae 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  91a3a38 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b897ffa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> ed19f42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c1af690 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> 668499b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 5a187d8 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> 0c7d8bb 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  f613c30 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1720e37 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

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




metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
Lines 1387 (patched)
<https://reviews.apache.org/r/59205/#comment248174>

There is no order-by in the SQL query above. Is it guaranteed that the list 
is ordered ?
Should we add a "order by" to above SQL query ?


- Thejas Nair


On May 14, 2017, 5:19 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 14, 2017, 5:19 a.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java d6a80ae 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  91a3a38 
>   itests/util/src/main/java/org/apache/hadoop/hive/ql/QTestUtil.java b897ffa 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> ed19f42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c1af690 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> 668499b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 5a187d8 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> 0c7d8bb 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  f613c30 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  1720e37 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 59205: HIVE-16579: CachedStore: improvements to partition col stats caching and cache column stats for unpartitioned table

[Thejas Nair]

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
Line 906 (original), 906 (patched)
<https://reviews.apache.org/r/59205/#comment248058>

Lets keep default time in seconds, just as before.


- Thejas Nair


On May 11, 2017, 9:07 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59205/
> ---
> 
> (Updated May 11, 2017, 9:07 p.m.)
> 
> 
> Review request for hive, Daniel Dai and Thejas Nair.
> 
> 
> Bugs: HIVE-16579
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-16579
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 73e0290 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java
>  88b9faf 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java 
> b96c27e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java 
> 870896c 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> a83e12e 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java c22a1db 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CacheUtils.java 
> b438479 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java 
> 39b1676 
>   metastore/src/java/org/apache/hadoop/hive/metastore/cache/SharedCache.java 
> 7beee42 
>   metastore/src/java/org/apache/hadoop/hive/metastore/hbase/HBaseStore.java 
> f6420f5 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  3e3fd20 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  91d8c2a 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java
>  0ab20d6 
> 
> 
> Diff: https://reviews.apache.org/r/59205/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: [VOTE] Apache Hive 2.3.0 Release Candidate 0

[Thejas Nair]

I meant to say +1 to fixing HIVE-16577 before release.


On Thu, May 4, 2017 at 6:43 PM, Thejas Nair <thejas.n...@gmail.com> wrote:

> +1. Yes, we should fix HIVE-16577
>
> On Wed, May 3, 2017 at 12:37 PM, Vihang Karajgaonkar <vih...@cloudera.com>
> wrote:
>
>> I think we need to fix HIVE-16577 before releasing 2.3.0. The metastore
>> schema initialization script for mssql is broken since 2.2.0.
>>
>> Also, I noticed that JIRA shows 2.2.0 as unreleased. Does anyone know how
>> to fix it?
>>
>> On Wed, May 3, 2017 at 8:48 AM, Sergio Pena <sergio.p...@cloudera.com>
>> wrote:
>>
>> > Thanks Rui.
>> >
>> > Pengcheng, the patch is reverted, you may continue with the RC1.
>> >
>> > On Tue, May 2, 2017 at 11:02 PM, Rui Li <lirui.fu...@gmail.com> wrote:
>> >
>> > > The patch has been reverted in master and branch-2.3
>> > >
>> > > On Wed, May 3, 2017 at 3:01 AM, Sergio Pena <sergio.p...@cloudera.com
>> >
>> > > wrote:
>> > >
>> > > > Hi Pengcheng,
>> > > >
>> > > > There is a request from the HDFS team to revert the patch committed
>> on
>> > > > HIVE-16047 from
>> > > > our code because it might cause problems when future Hadoop versions
>> > are
>> > > > released due to being a
>> > > > private API on Hadoop. This API method signature has been changed
>> > between
>> > > > releases, and
>> > > > we don't want to have additional shims to support future Hadoop
>> > versions
>> > > > just for this method.
>> > > >
>> > > > I'd like to revert it from 2.3.0 release before doing the release.
>> It
>> > is
>> > > > marked as being fixed on 2.2 but it is not cherry-picked on
>> branch-2.2
>> > > but
>> > > > branch-2.3.
>> > > >
>> > > > Do you agree?
>> > > >
>> > > > - Sergio
>> > > >
>> > > > On Fri, Apr 28, 2017 at 1:40 PM, Pengcheng Xiong <pxi...@apache.org
>> >
>> > > > wrote:
>> > > >
>> > > > > Withdraw the VOTE on candidate 0. Will propose candidate 1 soon.
>> > > Thanks.
>> > > > >
>> > > > > On Thu, Apr 27, 2017 at 8:10 PM, Owen O'Malley <
>> > owen.omal...@gmail.com
>> > > >
>> > > > > wrote:
>> > > > >
>> > > > > > -1 you need a release of storage-API first.
>> > > > > >
>> > > > > > .. Owen
>> > > > > >
>> > > > > > > On Apr 27, 2017, at 17:43, Pengcheng Xiong <pxi...@apache.org
>> >
>> > > > wrote:
>> > > > > > >
>> > > > > > > Apache Hive 2.3.0 Release Candidate 0 is available here:
>> > > > > > > http://home.apache.org/~pxiong/apache-hive-2.3.0-rc0/
>> > > > > > >
>> > > > > > >
>> > > > > > > Maven artifacts are available here:
>> > > > > > > https://repository.apache.org/content/repositories/
>> > > > orgapachehive-1073/
>> > > > > > >
>> > > > > > >
>> > > > > > > Source tag for RC0 is at:
>> > > > > > >
>> > > > > > > https://github.com/apache/hive/releases/tag/release-2.3.0-rc0
>> > > > > > >
>> > > > > > > Voting will conclude in 72 hours.
>> > > > > > >
>> > > > > > > Hive PMC Members: Please test and vote.
>> > > > > > >
>> > > > > > > Thanks.
>> > > > > >
>> > > > >
>> > > >
>> > >
>> > >
>> > >
>> > > --
>> > > Best regards!
>> > > Rui Li
>> > > Cell: (+86) 13564950210
>> > >
>> >
>>
>
>

Re: [VOTE] Apache Hive 2.3.0 Release Candidate 0

[Thejas Nair]

+1. Yes, we should fix HIVE-16577

On Wed, May 3, 2017 at 12:37 PM, Vihang Karajgaonkar 
wrote:

> I think we need to fix HIVE-16577 before releasing 2.3.0. The metastore
> schema initialization script for mssql is broken since 2.2.0.
>
> Also, I noticed that JIRA shows 2.2.0 as unreleased. Does anyone know how
> to fix it?
>
> On Wed, May 3, 2017 at 8:48 AM, Sergio Pena 
> wrote:
>
> > Thanks Rui.
> >
> > Pengcheng, the patch is reverted, you may continue with the RC1.
> >
> > On Tue, May 2, 2017 at 11:02 PM, Rui Li  wrote:
> >
> > > The patch has been reverted in master and branch-2.3
> > >
> > > On Wed, May 3, 2017 at 3:01 AM, Sergio Pena 
> > > wrote:
> > >
> > > > Hi Pengcheng,
> > > >
> > > > There is a request from the HDFS team to revert the patch committed
> on
> > > > HIVE-16047 from
> > > > our code because it might cause problems when future Hadoop versions
> > are
> > > > released due to being a
> > > > private API on Hadoop. This API method signature has been changed
> > between
> > > > releases, and
> > > > we don't want to have additional shims to support future Hadoop
> > versions
> > > > just for this method.
> > > >
> > > > I'd like to revert it from 2.3.0 release before doing the release. It
> > is
> > > > marked as being fixed on 2.2 but it is not cherry-picked on
> branch-2.2
> > > but
> > > > branch-2.3.
> > > >
> > > > Do you agree?
> > > >
> > > > - Sergio
> > > >
> > > > On Fri, Apr 28, 2017 at 1:40 PM, Pengcheng Xiong 
> > > > wrote:
> > > >
> > > > > Withdraw the VOTE on candidate 0. Will propose candidate 1 soon.
> > > Thanks.
> > > > >
> > > > > On Thu, Apr 27, 2017 at 8:10 PM, Owen O'Malley <
> > owen.omal...@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > -1 you need a release of storage-API first.
> > > > > >
> > > > > > .. Owen
> > > > > >
> > > > > > > On Apr 27, 2017, at 17:43, Pengcheng Xiong 
> > > > wrote:
> > > > > > >
> > > > > > > Apache Hive 2.3.0 Release Candidate 0 is available here:
> > > > > > > http://home.apache.org/~pxiong/apache-hive-2.3.0-rc0/
> > > > > > >
> > > > > > >
> > > > > > > Maven artifacts are available here:
> > > > > > > https://repository.apache.org/content/repositories/
> > > > orgapachehive-1073/
> > > > > > >
> > > > > > >
> > > > > > > Source tag for RC0 is at:
> > > > > > >
> > > > > > > https://github.com/apache/hive/releases/tag/release-2.3.0-rc0
> > > > > > >
> > > > > > > Voting will conclude in 72 hours.
> > > > > > >
> > > > > > > Hive PMC Members: Please test and vote.
> > > > > > >
> > > > > > > Thanks.
> > > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Best regards!
> > > Rui Li
> > > Cell: (+86) 13564950210
> > >
> >
>

Re: ptest not running?

[Thejas Nair]

Thanks for the update Vihang!


On Tue, Apr 25, 2017 at 12:54 PM, Vihang Karajgaonkar <vih...@cloudera.com>
wrote:

> This seems to be related to a larger infrastructure issue. Other projects
> (hadoop, sentry, hbase etc) are facing similar issues as well.
>
> https://issues.apache.org/jira/browse/INFRA-13985
>
> On Tue, Apr 25, 2017 at 11:54 AM, Thejas Nair <thejas.n...@gmail.com>
> wrote:
>
> > Looks like nothing is getting scheduled since yesterday evening, even
> with
> > the manual launch
> >
> > https://builds.apache.org/job/PreCommit-HIVE-Build/
> >
> > Can someone with access please take a look ?
> >
> >
> > On Mon, Apr 24, 2017 at 5:26 PM, Eugene Koifman <
> ekoif...@hortonworks.com>
> > wrote:
> >
> > > Something is not working with the part of the flow that triggers the
> > build
> > > automatically.
> > > In the meantime, if you log in with apache credentials, and go to
> “Build
> > > with Parameters”
> > > you can manually launch the test by entering the bug number: just the
> > > digits, skip the “HIVE-“ part
> > >
> > >
> > > On 4/24/17, 5:01 PM, "Vihang Karajgaonkar" <vih...@cloudera.com>
> wrote:
> > >
> > > I see that builds are getting picked up at
> > https://builds.apache.org/
> > > job/PreCommit-HIVE-Build/ The PTest server was restarted a couple
> of
> > > days
> > > back. If you had a patch in the queue around that time, may be it
> was
> > > lost
> > > due to restart. You may have to re-attach the patch on the upstream
> > > JIRA if
> > > that is the case.
> > >
> > >
> > > On Mon, Apr 24, 2017 at 2:44 PM, Eugene Koifman <
> > > ekoif...@hortonworks.com>
> > > wrote:
> > >
> > > > Hi,
> > > > It is not picking up new patches and there are none queued up.
> > Could
> > > > someone check please?
> > > >
> > > > Thanks,
> > > > Eugene
> > > >
> > > >
> > >
> > >
> > >
> >
>

Re: ptest not running?

[Thejas Nair]

Looks like nothing is getting scheduled since yesterday evening, even with
the manual launch

https://builds.apache.org/job/PreCommit-HIVE-Build/

Can someone with access please take a look ?


On Mon, Apr 24, 2017 at 5:26 PM, Eugene Koifman 
wrote:

> Something is not working with the part of the flow that triggers the build
> automatically.
> In the meantime, if you log in with apache credentials, and go to “Build
> with Parameters”
> you can manually launch the test by entering the bug number: just the
> digits, skip the “HIVE-“ part
>
>
> On 4/24/17, 5:01 PM, "Vihang Karajgaonkar"  wrote:
>
> I see that builds are getting picked up at https://builds.apache.org/
> job/PreCommit-HIVE-Build/ The PTest server was restarted a couple of
> days
> back. If you had a patch in the queue around that time, may be it was
> lost
> due to restart. You may have to re-attach the patch on the upstream
> JIRA if
> that is the case.
>
>
> On Mon, Apr 24, 2017 at 2:44 PM, Eugene Koifman <
> ekoif...@hortonworks.com>
> wrote:
>
> > Hi,
> > It is not picking up new patches and there are none queued up.  Could
> > someone check please?
> >
> > Thanks,
> > Eugene
> >
> >
>
>
>

Re: [VOTE] Apache Hive 1.2.2 Release Candidate 0

[Thejas Nair]

+1 (binding)
- Verified signature and checksum
- Build from source
- Ran simple queries in local mode with binary tar.gz
- Checked RELEASE_NOTES file. Traditionally this file has had the set of
patches fixed in previous releases as well ( ie, each new release was
adding entries to the top of the file). This time it has only the new patch
release patches. The old approach helps to quickly verify if patch is in
the release. I think it would be good to fix that in branch. I think it is
OK for this release.
- README.txt has old 1.2.1 version number in it. IMO, we should just remove
the mention of version in that file. Not a release blocker.




On Wed, Apr 5, 2017 at 3:52 PM, Sergio Pena 
wrote:

> +1 (no-binding)
>
> I unpacked the bin and src packages.
> Verified gpg and md5 signatures.
> Check license and release notes files.
> Run a few queries from hive-cli.
>
> - Sergio
>
> On Tue, Apr 4, 2017 at 11:12 AM, Ashutosh Chauhan 
> wrote:
>
> > Verified md5 of src and binary tar balls.
> > Built from src.
> > Ran some simple queries like join, group by.
> > All looks good.
> >
> > +1
> >
> > Thanks,
> > Ashutosh
> >
> > On Mon, Apr 3, 2017 at 4:47 PM, Vaibhav Gumashta <
> > vgumas...@hortonworks.com>
> > wrote:
> >
> > > Thanks for pointing out Ashutosh. Link to my PGP key:
> > > http://pgp.mit.edu/pks/lookup?search=gumashta=index.
> > >
> > > I think it will take a day or so for the KEYS file to be updated (it is
> > > auto generated), but if you want to test the release in the meantime,
> > > please use the above link to access the signing key.
> > >
> > > Thanks,
> > > ‹Vaibhav
> > >
> > > On 4/3/17, 2:53 PM, "Ashutosh Chauhan"  wrote:
> > >
> > > >Hi Vaibhav,
> > > >
> > > >Can't locate your key at any of standard location. Can you point out
> > which
> > > >key you used to sign the release?
> > > >
> > > >Thanks,
> > > >Ashutosh
> > > >
> > > >On Mon, Apr 3, 2017 at 12:51 AM, Vaibhav Gumashta
> > > > > > >> wrote:
> > > >> Hi everyone,
> > > >>
> > > >> Apache Hive 1.2.2 Release Candidate 0 is available here:
> > > >>
> > > >> https://dist.apache.org/repos/dist/dev/hive/apache-hive-1.2.2-rc0/
> > > >>
> > > >> Maven artifacts are available here:
> > > >>
> > > >> https://repository.apache.org/content/repositories/
> > orgapachehive-1072/
> > > >>
> > > >> Source tag for RC0 is at:
> > > >> https://github.com/apache/hive/releases/tag/release-1.2.2-rc0
> > > >>
> > > >> Voting will conclude in 72 hours.
> > > >>
> > > >> Hive PMC Members: Please test and vote.
> > > >>
> > > >> Thanks,
> > > >> -Vaibhav
> > > >>
> > > >>
> > >
> > >
> >
>

Re: [DISCUSS] split metastore and service

[Thejas Nair]

Also, thanks for the email thread to bring peoples attention to this change.

On Tue, Mar 28, 2017 at 6:35 PM, Thejas Nair <thejas.n...@gmail.com> wrote:

> +1
> Thanks for looking into this!
>
>
> On Tue, Mar 28, 2017 at 11:26 AM, Eugene Koifman <ekoif...@hortonworks.com
> > wrote:
>
>> +1 reduce the number of uber jars
>>
>>
>> On 3/27/17, 1:05 PM, "Sergey Shelukhin" <ser...@hortonworks.com> wrote:
>>
>> Splitting the metastore would also allow us to get rid of compile time
>> dependencies that are resolved via reflection right now.
>> +1 on the feature
>>
>> On 17/3/27, 07:33, "Zoltan Haindrich" <zhaindr...@hortonworks.com>
>> wrote:
>>
>> >Hello,
>> >
>> >Currently the jdbc driver contains lots of hive code; which are not
>> >needed for the driver to function properly - jdbc-standalone is
>> currently
>> >a 60M binary! :)
>> >
>> >I've opened a ticket, to explore the possibilites what can be done in
>> >this aspect to reduce jdbc's dependencies.
>> >
>> >I was able to remove most of the service and the metastore
>> dependencies -
>> >by introducing 2 new modules: I called them metastore-api and
>> >service-client.
>> >As a change like this would mean that the released jars name and
>> purpose
>> >would change - I didn't wanted to just file a jira about it :)
>> >
>> >So...I would like to ask for opinions or any concerns against doing
>> the
>> >following:
>> >
>> >1) Splitting the metastore module; the new module would be named as
>> >metastore-X (my proposals for X are: client,rpc,if or api).
>> >  * the dependency would contain the thrift interface
>> >  * and possibly a few other source files which are needed to use it.
>> >
>> >2) Splitting the service module; the new module would be named
>> service-X
>> >(my propsal for X would be client)
>> >  * the module would contain auth related classes
>> >  * some other basic stuffs like RowSet
>> >  * connected change: jdbc driver would change the support of
>> embedded
>> >mode to only make it usable if 'service' is loaded onto the classpath
>> >
>> >With these two modules available, the size of the jdbc driver have
>> >dropped to about 21M.
>> >
>> >more info:
>> >https://issues.apache.org/jira/browse/HIVE-16214
>> >
>> >regards,
>> >Zoltan
>>
>>
>>
>>
>

Re: [DISCUSS] split metastore and service

[Thejas Nair]

+1
Thanks for looking into this!


On Tue, Mar 28, 2017 at 11:26 AM, Eugene Koifman 
wrote:

> +1 reduce the number of uber jars
>
>
> On 3/27/17, 1:05 PM, "Sergey Shelukhin"  wrote:
>
> Splitting the metastore would also allow us to get rid of compile time
> dependencies that are resolved via reflection right now.
> +1 on the feature
>
> On 17/3/27, 07:33, "Zoltan Haindrich" 
> wrote:
>
> >Hello,
> >
> >Currently the jdbc driver contains lots of hive code; which are not
> >needed for the driver to function properly - jdbc-standalone is
> currently
> >a 60M binary! :)
> >
> >I've opened a ticket, to explore the possibilites what can be done in
> >this aspect to reduce jdbc's dependencies.
> >
> >I was able to remove most of the service and the metastore
> dependencies -
> >by introducing 2 new modules: I called them metastore-api and
> >service-client.
> >As a change like this would mean that the released jars name and
> purpose
> >would change - I didn't wanted to just file a jira about it :)
> >
> >So...I would like to ask for opinions or any concerns against doing
> the
> >following:
> >
> >1) Splitting the metastore module; the new module would be named as
> >metastore-X (my proposals for X are: client,rpc,if or api).
> >  * the dependency would contain the thrift interface
> >  * and possibly a few other source files which are needed to use it.
> >
> >2) Splitting the service module; the new module would be named
> service-X
> >(my propsal for X would be client)
> >  * the module would contain auth related classes
> >  * some other basic stuffs like RowSet
> >  * connected change: jdbc driver would change the support of embedded
> >mode to only make it usable if 'service' is loaded onto the classpath
> >
> >With these two modules available, the size of the jdbc driver have
> >dropped to about 21M.
> >
> >more info:
> >https://issues.apache.org/jira/browse/HIVE-16214
> >
> >regards,
> >Zoltan
>
>
>
>

Re: Backward incompatible changes

[Thejas Nair]

+1
There are some features that are incomplete and what I would not recommend
for any real production use.The 'legacy authorization mode' is a great
example of that -
https://cwiki.apache.org/confluence/display/Hive/Hive+Default+Authorization+-+Legacy+Mode
. It is inherently insecure mode that nobody should be using.

There is also potential to cleanup of the thrift api. However, there are
many users of this api, we would need to go the deprecation then remove
after couple of releases route or so for that.

I am sure there are many other candidates. We will have to evaluate each of
those features on the risk/benefit of keeping them and arriving at a
decision.

Also, +1 on getting a 2.2 release out before we branch.



On Fri, Mar 3, 2017 at 1:50 PM, Ashutosh Chauhan 
wrote:

> Hi all,
>
> Hive project has come a long way. With wide-spread adoption also comes
> expectations. Expectation of being backward compatible and not breaking
> things. However that doesn't come free of cost and results in lot of legacy
> code which can't be refactored without fear of breaking things. As a result
> project has accumulated lot of debt over time. At the same time there are
> also lot of features which have seen little uptake. We may want to drop
> some of those.
>
> In order to move forward and shed that debt we may need a major version
> release which allows us to make backward incompatible changes and drop
> rarely used features. At the same time there are lots of users which are
> consuming currently released 2.1 , 2.2 branches and expect them to stay on
> it for some time. So, I propose that we create branch-2 from current tip
> and do future 2.x releases from that branch and keep it backward
> compatible. This will allow devs to land breaking changes on master and
> pave way to release hive 3.0 in future.
>
> Ofcourse, each specific incompatible change and feature drop  even on
> master need to be evaluated on its own merit on corresponding jira. This
> email is just a solicitation of feedback for creating branch-2 and allowing
> breaking changes in master. Thoughts?
>
> Thanks,
> Ashutosh
>

Re: [VOTE] Drop support for Java7 in master branch

[Thejas Nair]

Owen,
The end of support from Oracle is just one aspect of it. As I mentioned
earlier, we already seeing many libraries (hikaricp and jetty) move to
requiring java8. Note that hadoop 3.0 line already requires java8 as well.
As long as java8 is widely supported and available on systems users are
likely to use for upgrading to a new 2.x release, I think we should be good.



On Tue, Feb 28, 2017 at 11:02 AM, Owen O'Malley <omal...@apache.org> wrote:

> I'm a little worried that we are dropping it too soon given that OpenJDK
> will support 7 for another 1.5 years.
>
> https://access.redhat.com/articles/1299013
>
> .. Owen
>
> On Tue, Feb 28, 2017 at 10:49 AM, Siddharth Seth <ss...@apache.org> wrote:
>
> > +1
> >
> > On Mon, Feb 27, 2017 at 8:54 PM, Thejas Nair <thejas.n...@gmail.com>
> > wrote:
> >
> > > There was a [DISCUSS] thread on the topic of moving to jdk8 for unit
> > tests
> > > [1], and many people also expressed the opinion that we should drop
> JDK 7
> > > support in Hive. Public updates by Oracle was stopped on Apr 2015 [2].
> > >
> > > This vote thread proposes to dropping JDK 7 support in the next Apache
> > Hive
> > > 2.x release (ie master branch), so that we can start leveraging new
> > > features in Java 8 and also libraries that require java8.
> > >
> > > [1] https://s.apache.org/hive-jdk8-test
> > > [2] http://www.oracle.com/technetwork/java/eol-135779.html
> > >
> > > Here is my +1.
> > >
> > > Vote ends in 72 hours.
> > > Thanks,
> > > Thejas
> > >
> > > PS: I think this would fall under "Code change" under Hive-bylaws, so
> it
> > > doesn't seem to really require a formal vote thread. But I think this
> > does
> > > merit wider attention than a jira ticket.
> > >
> >
>

Re: [VOTE] Drop support for Java7 in master branch

[Thejas Nair]

Note that upgrading the minimum required version to JDK8 also gives Hive
the option of using more recent versions of several libraries including
hikaricp (as default connection pool option) [1] and jetty [2]

[1] https://issues.apache.org/jira/browse/HIVE-13931
[2] https://issues.apache.org/jira/browse/HIVE-16049


On Tue, Feb 28, 2017 at 7:45 AM, Sergio Pena <sergio.p...@cloudera.com>
wrote:

> Thanks Alan for the proposal. JDK8 seems to have very useful API. It should
> be good to start using it.
>
> +1.
>
> On Tue, Feb 28, 2017 at 12:55 AM, Rajat Khandelwal <
> rajat.khandel...@inmobi.com> wrote:
>
> > +1
> >
> > On Tue, Feb 28, 2017 at 10:37 AM, Prasanth Jayachandran <
> > pjayachand...@hortonworks.com> wrote:
> >
> > > +1
> > >
> > > Thanks
> > > Prasanth
> > >
> > >
> > >
> > >
> > > On Mon, Feb 27, 2017 at 8:55 PM -0800, "Thejas Nair" <
> > > thejas.n...@gmail.com<mailto:thejas.n...@gmail.com>> wrote:
> > >
> > >
> > > There was a [DISCUSS] thread on the topic of moving to jdk8 for unit
> > tests
> > > [1], and many people also expressed the opinion that we should drop
> JDK 7
> > > support in Hive. Public updates by Oracle was stopped on Apr 2015 [2].
> > >
> > > This vote thread proposes to dropping JDK 7 support in the next Apache
> > Hive
> > > 2.x release (ie master branch), so that we can start leveraging new
> > > features in Java 8 and also libraries that require java8.
> > >
> > > [1] https://s.apache.org/hive-jdk8-test
> > > [2] http://www.oracle.com/technetwork/java/eol-135779.html
> > >
> > > Here is my +1.
> > >
> > > Vote ends in 72 hours.
> > > Thanks,
> > > Thejas
> > >
> > > PS: I think this would fall under "Code change" under Hive-bylaws, so
> it
> > > doesn't seem to really require a formal vote thread. But I think this
> > does
> > > merit wider attention than a jira ticket.
> > >
> > >
> >
> >
> > --
> > Rajat Khandelwal
> > Tech Lead
> >
> > --
> > _
> > The information contained in this communication is intended solely for
> the
> > use of the individual or entity to whom it is addressed and others
> > authorized to receive it. It may contain confidential or legally
> privileged
> > information. If you are not the intended recipient you are hereby
> notified
> > that any disclosure, copying, distribution or taking any action in
> reliance
> > on the contents of this information is strictly prohibited and may be
> > unlawful. If you have received this communication in error, please notify
> > us immediately by responding to this email and then delete it from your
> > system. The firm is neither liable for the proper and complete
> transmission
> > of the information contained in this communication nor for any delay in
> its
> > receipt.
> >
>

[VOTE] Drop support for Java7 in master branch

[Thejas Nair]

There was a [DISCUSS] thread on the topic of moving to jdk8 for unit tests
[1], and many people also expressed the opinion that we should drop JDK 7
support in Hive. Public updates by Oracle was stopped on Apr 2015 [2].

This vote thread proposes to dropping JDK 7 support in the next Apache Hive
2.x release (ie master branch), so that we can start leveraging new
features in Java 8 and also libraries that require java8.

[1] https://s.apache.org/hive-jdk8-test
[2] http://www.oracle.com/technetwork/java/eol-135779.html

Here is my +1.

Vote ends in 72 hours.
Thanks,
Thejas

PS: I think this would fall under "Code change" under Hive-bylaws, so it
doesn't seem to really require a formal vote thread. But I think this does
merit wider attention than a jira ticket.

Re: Precommit jenkins is failing

[Thejas Nair]

+ Sergio,
Any idea what might be causing this ? Will you be able to take a look ?

On Mon, Jan 16, 2017 at 12:42 PM, Deepak Jaiswal 
wrote:

> Is there anyone who is looking into this?
>
> On 1/13/17, 10:46 AM, "Wei Zheng"  wrote:
>
> [INFO] 
> 
> [INFO] BUILD SUCCESS
> [INFO] 
> 
> [INFO] Total time: 12.954 s
> [INFO] Finished at: 2017-01-13T18:39:48+00:00
> [INFO] Final Memory: 47M/705M
> [INFO] 
> 
> + local 'PTEST_CLASSPATH=/home/jenkins/jenkins-slave/
> workspace/PreCommit-HIVE-Build/hive/build/hive/
> testutils/ptest2/target/hive-ptest-1.0-classes.jar:/home/
> jenkins/jenkins-slave/workspace/PreCommit-HIVE-Build/hive/build/hive/
> testutils/ptest2/target/lib/*'
> + java -cp '/home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-
> Build/hive/build/hive/testutils/ptest2/target/hive-
> ptest-1.0-classes.jar:/home/jenkins/jenkins-slave/
> workspace/PreCommit-HIVE-Build/hive/build/hive/testutils/ptest2/target/lib/*'
> org.apache.hive.ptest.api.client.PTestClient --command testStart
> --outputDir /home/jenkins/jenkins-slave/workspace/PreCommit-HIVE-
> Build/hive/build/hive/testutils/ptest2/target --password ''
> --testHandle PreCommit-HIVE-Build-2939 --endpoint
> http://104.198.109.242:8080/hive-ptest-1.0 --logsEndpoint
> http://104.198.109.242/logs/ --profile master-mr2 --patch
> https://issues.apache.org/jira/secure/attachment/
> 12847390/HIVE-15621.1.patch --jira HIVE-15621
> Exception in thread "main" javax.net.ssl.SSLHandshakeException:
> sun.security.validator.ValidatorException: PKIX path building failed:
> sun.security.provider.certpath.SunCertPathBuilderException: unable to
> find valid certification path to requested target
>   at sun.security.ssl.Alerts.getSSLException(Alerts.java:192)
>   at sun.security.ssl.SSLSocketImpl.fatal(SSLSocketImpl.java:1904)
>   at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:279)
>   at sun.security.ssl.Handshaker.fatalSE(Handshaker.java:273)
>   at sun.security.ssl.ClientHandshaker.serverCertificate(
> ClientHandshaker.java:1446)
>   at sun.security.ssl.ClientHandshaker.processMessage(
> ClientHandshaker.java:209)
>   at sun.security.ssl.Handshaker.processLoop(Handshaker.java:913)
>   at sun.security.ssl.Handshaker.process_record(Handshaker.
> java:849)
>   at sun.security.ssl.SSLSocketImpl.readRecord(
> SSLSocketImpl.java:1023)
>   at sun.security.ssl.SSLSocketImpl.performInitialHandshake(
> SSLSocketImpl.java:1332)
>   at sun.security.ssl.SSLSocketImpl.startHandshake(
> SSLSocketImpl.java:1359)
>   at sun.security.ssl.SSLSocketImpl.startHandshake(
> SSLSocketImpl.java:1343)
>   at sun.net.www.protocol.https.HttpsClient.afterConnect(
> HttpsClient.java:559)
>   at sun.net.www.protocol.https.AbstractDelegateHttpsURLConnec
> tion.connect(AbstractDelegateHttpsURLConnection.java:185)
>   at sun.net.www.protocol.http.HttpURLConnection.getInputStream(
> HttpURLConnection.java:1301)
>   at sun.net.www.protocol.https.HttpsURLConnectionImpl.
> getInputStream(HttpsURLConnectionImpl.java:254)
>   at java.net.URL.openStream(URL.java:1041)
>   at com.google.common.io.Resources$UrlByteSource.
> openStream(Resources.java:72)
>   at com.google.common.io.ByteSource.read(ByteSource.java:257)
>   at com.google.common.io.Resources.toByteArray(Resources.java:99)
>   at org.apache.hive.ptest.api.client.PTestClient.testStart(
> PTestClient.java:126)
>   at org.apache.hive.ptest.api.client.PTestClient.main(
> PTestClient.java:320)
> Caused by: sun.security.validator.ValidatorException: PKIX path
> building failed: sun.security.provider.certpath.SunCertPathBuilderException:
> unable to find valid certification path to requested target
>   at sun.security.validator.PKIXValidator.doBuild(
> PKIXValidator.java:385)
>   at sun.security.validator.PKIXValidator.engineValidate(
> PKIXValidator.java:292)
>   at sun.security.validator.Validator.validate(Validator.java:260)
>   at sun.security.ssl.X509TrustManagerImpl.validate(
> X509TrustManagerImpl.java:326)
>   at sun.security.ssl.X509TrustManagerImpl.checkTrusted(
> X509TrustManagerImpl.java:231)
>   at sun.security.ssl.X509TrustManagerImpl.checkServerTrusted(
> X509TrustManagerImpl.java:126)
>   at sun.security.ssl.ClientHandshaker.serverCertificate(
> ClientHandshaker.java:1428)
>   ... 17 more
> Caused by: sun.security.provider.certpath.SunCertPathBuilderException:
> unable to find valid certification path to requested target
>   at 

Re: [DISCUSS] Separate release of storage-api

[Thejas Nair]

+1
I agree that using an independent version number for this release is the
better option.
Regarding the mapping, I see it like a dependency on a third party library.
Its based on what is in the pom.xml .


On Wed, Jan 11, 2017 at 9:36 AM, Alan Gates  wrote:

> 90% of the discussion on the previous thread was on version numbering,
> which never came to a conclusion.  Based on your RC candidate I assume you
> chose the separate version numbering, but starting from the same 2.2.0 base
> number.
>
> I agree this is the only viable option[1], but I wanted to point it out
> here to be clear we're choosing this rather than there being questions next
> time we release something on what the number should be.
>
> Alan.
>
> 1. Support for my statement that this is the only viable option:
> a) Tying together the version numbers of Hive proper and the storage API
> undoes exactly the independence we're trying to enable here.
> b) Lefty's concern that Sergey's solution will make a hash of the JIRAs is
> a deal breaker for that one.  It also will be harder to truly separate
> these in the future if Sergio is correct and this eventually evolves into a
> proper sub-project or separate project.
> c) Other projects with this same issue let their version numbers move
> independently (e.g. datanucleus)
> d) Our pom files will give us the mapping of how the versions fit together.
>
>
> > On Jan 1, 2017, at 10:19 AM, Owen O'Malley  wrote:
> >
> > Hi all,
> >   As we discussed back in
> > https://mid.mail-archive.com/dev@hive.apache.org/msg121112.html . I'd
> like
> > to make a release of storage-api. I've written up a proposal at
> > https://cwiki.apache.org/confluence/display/Hive/
> Storage+API+Release+Proposal
> > and the patch for HIVE-15419 is pretty close. Once HIVE-15419 is
> committed,
> > I'd like to cut a branch (eg storage-branch-2.2) and make a release
> > candidate (eg. storage-release-2.2.0rc0).
> >
> > Any concerns?
> >
> > Thanks,
> >   Owen
>
>

Re: Review Request 54826: HIVE-15448: ChangeManager for replication

[Thejas Nair]

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


Ship it!




Ship It!

- Thejas Nair


On Dec. 27, 2016, 4:44 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54826/
> ---
> 
> (Updated Dec. 27, 2016, 4:44 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-15448
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b4e89b0 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
>  PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 2892da3 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 6aca1b7 
> 
> Diff: https://reviews.apache.org/r/54826/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Re: Review Request 54826: HIVE-15448: ChangeManager for replication

[Thejas Nair]

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




metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 55)
<https://reviews.apache.org/r/54826/#comment231174>

seems better as local to the reycle method, as its not used outside.



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 57)
<https://reviews.apache.org/r/54826/#comment231177>

trailing whitespaces



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 150)
<https://reviews.apache.org/r/54826/#comment231173>

how about doing this as part of ReplChangeManager constructor ? That will 
reduce the number of times this needs to be called.



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 165)
<https://reviews.apache.org/r/54826/#comment231175>

trailing whitespaces in this change. Shows up in red in the diff.



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 172)
<https://reviews.apache.org/r/54826/#comment231176>

trailing whitespaces



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 232)
<https://reviews.apache.org/r/54826/#comment231167>

check for DEBUG enabled, but logging is at INFO level


- Thejas Nair


On Dec. 23, 2016, 8:29 a.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54826/
> ---
> 
> (Updated Dec. 23, 2016, 8:29 a.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-15448
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b4e89b0 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
>  PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 2892da3 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 6aca1b7 
> 
> Diff: https://reviews.apache.org/r/54826/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Re: Review Request 54826: HIVE-15448: ChangeManager for replication

[Thejas Nair]

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




common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 440)
<https://reviews.apache.org/r/54826/#comment230534>

lets keep this disabled for now as this work is in early stages



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 442)
<https://reviews.apache.org/r/54826/#comment230535>

any HCFS should work for this (Hadoop compatible file system) (at least 
eventually). I think we can remove the HDFS reference.



common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (line 443)
<https://reviews.apache.org/r/54826/#comment230536>

should we use hours instead as default unit ? it seems most people would 
think of this parameter in hours/days.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
 (line 56)
<https://reviews.apache.org/r/54826/#comment230765>

with recent changes by Vaibhav to ProxyFileSystem, local files would also 
have checksums. Can we switch to local files, so that test speed is better 
(local fs might be more reliable than minidfs, as it has less parts to it!)



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
 (line 59)
<https://reviews.apache.org/r/54826/#comment230766>

why is this needed ?



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
 (line 124)
<https://reviews.apache.org/r/54826/#comment230768>

how about creating a method for this and calling -Partition part1 = 
createAndPartition("20160101")



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
 (line 241)
<https://reviews.apache.org/r/54826/#comment230772>

can you add a comment like -
// verify cm.recycle(Path) api moves file to cmroot dir



itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
 (line 247)
<https://reviews.apache.org/r/54826/#comment230773>

// verify cm.recycle(db, table) api moves file to cmroot dir



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 151)
<https://reviews.apache.org/r/54826/#comment230782>

should we call it getCksumString() to make it more easy to understand what 
is expected ?



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 175)
<https://reviews.apache.org/r/54826/#comment230776>

is there any generic way to find this from FileSystem api ? these configs 
are specific to HDFS.
But i guess we don't need this if we go with storing only the signature 
(see following comment)



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 182)
<https://reviews.apache.org/r/54826/#comment230780>

as discussed offline, looks like we don't want to rely on the directory 
structure of table.
This can cause problems in finding file in case of insert -> rename -> drop 
.
The signature should get used as the 'key' for finding the file.



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 205)
<https://reviews.apache.org/r/54826/#comment230774>

can you add a log message to indicate that this has started. Since this 
would typically be run only once in hour or so , INFO level should be good IMO



metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
(line 225)
<https://reviews.apache.org/r/54826/#comment230775>

can you add a debug level log message for when files get deleted ?


- Thejas Nair


On Dec. 16, 2016, 11:14 p.m., Daniel Dai wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54826/
> ---
> 
> (Updated Dec. 16, 2016, 11:14 p.m.)
> 
> 
> Review request for hive and Thejas Nair.
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> See HIVE-15448
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 9064e49 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestReplChangeManager.java
>  PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> f7b2ed7 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ReplChangeManager.java 
> PRE-CREATION 
>   metastore/src/java/org/apache/hadoop/hive/metastore/Warehouse.java 6aca1b7 
> 
> Diff: https://reviews.apache.org/r/54826/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Dai
> 
>

Re: Review Request 54771: Capture additional metadata to replicate a simple insert at destination

[Thejas Nair]

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




itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
 (line 1206)
<https://reviews.apache.org/r/54771/#comment230671>

can you add a verifyInsertJSON method or so ? looks like we can avoid the 
many repeating lines that way.


- Thejas Nair


On Dec. 19, 2016, 5:30 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54771/
> ---
> 
> (Updated Dec. 19, 2016, 5:30 p.m.)
> 
> 
> Review request for hive, Sushanth Sowmyan and Thejas Nair.
> 
> 
> Bugs: HIVE-15294
> https://issues.apache.org/jira/browse/HIVE-15294
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-15294
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  119801f 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  0b691b1 
>   metastore/if/hive_metastore.thrift 6f77156 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> d0a66b0 
>   metastore/src/java/org/apache/hadoop/hive/metastore/events/InsertEvent.java 
> 102754e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
>  adf2fd8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONInsertMessage.java
>  ef89b17 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  0407210 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java f62d5f3 
>   shims/common/src/main/java/org/apache/hadoop/fs/ProxyLocalFileSystem.java 
> bd97521 
> 
> Diff: https://reviews.apache.org/r/54771/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 54771: Capture additional metadata to replicate a simple insert at destination

[Thejas Nair]

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




metastore/src/java/org/apache/hadoop/hive/metastore/events/InsertEvent.java 
(line 52)
<https://reviews.apache.org/r/54771/#comment230420>

do we still need the old constructor ?


- Thejas Nair


On Dec. 15, 2016, 9:55 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54771/
> ---
> 
> (Updated Dec. 15, 2016, 9:55 a.m.)
> 
> 
> Review request for hive, Sushanth Sowmyan and Thejas Nair.
> 
> 
> Bugs: HIVE-15294
> https://issues.apache.org/jira/browse/HIVE-15294
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-15294
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  119801f 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  690616d 
>   metastore/if/hive_metastore.thrift baab31b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> f7b2ed7 
>   metastore/src/java/org/apache/hadoop/hive/metastore/events/InsertEvent.java 
> 102754e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
>  adf2fd8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONInsertMessage.java
>  ef89b17 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  0407210 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 750fdef 
> 
> Diff: https://reviews.apache.org/r/54771/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: Review Request 54771: Capture additional metadata to replicate a simple insert at destination

[Thejas Nair]

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




metastore/if/hive_metastore.thrift (line 815)
<https://reviews.apache.org/r/54771/#comment230369>

how about sending byte[] instead of converting to string here. byte[] is 
what we get straight from serializing the checksum



metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 (line 274)
<https://reviews.apache.org/r/54771/#comment230418>

anything specific about ordering here ? List is implicitly ordered. Should 
the more generic List be used instead of ArrayList ?
Also, this and getAsOrderedMap are used only in test. Should it be part of 
a test util method instead of JSONMessageFactory ?



metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
 (line 283)
<https://reviews.apache.org/r/54771/#comment230417>

what is "ordered" map ? should it be just getAsMap() ?



ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java (line 2281)
<https://reviews.apache.org/r/54771/#comment230375>

should we add empty checksum instead of skipping the addition of checksum ?
That seems cleaner. In theory, you could have some files belonging to a 
file system that does support cksum and others that don't. If we skip, then the 
size will be non zero and there will be a mismatch.


- Thejas Nair


On Dec. 15, 2016, 9:55 a.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54771/
> ---
> 
> (Updated Dec. 15, 2016, 9:55 a.m.)
> 
> 
> Review request for hive, Sushanth Sowmyan and Thejas Nair.
> 
> 
> Bugs: HIVE-15294
> https://issues.apache.org/jira/browse/HIVE-15294
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-15294
> 
> 
> Diffs
> -
> 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  119801f 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  690616d 
>   metastore/if/hive_metastore.thrift baab31b 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> f7b2ed7 
>   metastore/src/java/org/apache/hadoop/hive/metastore/events/InsertEvent.java 
> 102754e 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
>  adf2fd8 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONInsertMessage.java
>  ef89b17 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
>  0407210 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 750fdef 
> 
> Diff: https://reviews.apache.org/r/54771/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>

Re: [VOTE] Apache Hive 2.1.1 Release Candidate 1

[Thejas Nair]

+1 (Looks like I have the final word! :) )

Verified checksum and signature for both packages. Build from source and
ran some simple queries.
Also examined LICENSE, NOTICE, and RELEASE_NOTES files.
I see that the RELEASE_NOTES file this time contains only the changes in
latest release, in the past we had the entire history of changes in each
release in that file. I think going forward, we should switch back to that.
That way the entire list of changes in that release is easy to examine.
Also, the update to RELEASE_NOTES should go into master as well, so that
his practice can be followed in future.
Jesus, will you ble able to look into updating the RELEASE_NOTES in the git
branches after the release, so that its fixed for future releases ?



On Wed, Dec 7, 2016 at 3:14 AM, Jesus Camacho Rodriguez  wrote:

> PMC Members,
>
> We still need one +1 vote to be able to release 2.1.1. Please test and
> vote!
>
> Thanks,
> Jesús
>
>
>
>
> On 12/6/16, 4:02 PM, "Ashutosh Chauhan"  wrote:
>
> >+1 Verified md5 on src and binary distribution. Built src distribution.
> All
> >looks good.
> >
> >Thanks,
> >Ashutosh
> >
> >On Fri, Dec 2, 2016 at 7:32 AM, Jesus Camacho Rodriguez <
> jcama...@apache.org
> >> wrote:
> >
> >> PMC Members,
> >>
> >> A reminder that we still need two +1 votes to release 2.1.1. Please test
> >> and vote!
> >>
> >> Thanks,
> >> Jesús
> >>
> >>
> >>
> >>
> >> On 12/1/16, 6:45 PM, "Jesus Camacho Rodriguez"  >> hortonworks.com on behalf of jcama...@apache.org> wrote:
> >>
> >> >Sergio,
> >> >
> >> >I used OSX 10.11.
> >> >
> >> >Maybe it has to do with the version used to verify the md5? Can you
> just
> >> try to verify manually?
> >> >
> >> >$ md5sum apache-hive-2.1.1-bin.tar.gz > apache-hive-2.1.1-bin.tar.gz.
> >> md5.self
> >> >$ diff -q apache-hive-2.1.1-bin.tar.gz.md5
> apache-hive-2.1.1-bin.tar.gz.
> >> md5.self
> >> >
> >> >
> >> >About the KEYS, my key is not in the file you referred, I should have
> >> added it before.
> >> >You can find it here:
> >> >https://people.apache.org/keys/committer/jcamacho.asc
> >> >
> >> >Let me know if that solves your problem.
> >> >
> >> >--
> >> >Jesús
> >> >
> >> >
> >> >
> >> >On 11/30/16, 9:08 PM, "Sergio Pena"  wrote:
> >> >
> >> >>Jesus,
> >> >>
> >> >>I tried verifying the md5 and gpg signatures, but I get these errors:
> >> >>
> >> >>hive/packaging/target⟫ md5sum -c apache-hive-2.1.1-bin.tar.gz.md5
> >> >>apache-hive-2.1.1-bin.tar.gz: FAILED
> >> >>md5sum: WARNING: 1 computed checksum did NOT match
> >> >>
> >> >>hive/packaging/target⟫ gpg --verify apache-hive-2.1.1-bin.tar.gz.asc
> >> >>apache-hive-2.1.1-bin.tar.gz
> >> >>gpg: Signature made Tue 29 Nov 2016 01:57:04 PM CST
> >> >>gpg:using RSA key 931E4AB3C516B444
> >> >>gpg: Can't check signature: No public key
> >> >>
> >> >>I'm using ubuntu, so I think the md5 differs from OSX and Linux
> >> machines. I
> >> >>remember seeing this problem before. What OS did you use?
> >> >>
> >> >>for the GPG keys, I imported the KEYS file mentioned in the Wiki, but
> I
> >> >>still get that error. Any idea what I'm missing?
> >> >>
> >> >>On Tue, Nov 29, 2016 at 6:23 PM, Gary Gregory  >
> >> >>wrote:
> >> >>
> >> >>> FWIW, running 'mvn clean install' has been failing on Git master
> for a
> >> long
> >> >>> time on Windows. Will that ever be fixed?
> >> >>>
> >> >>> Gary
> >> >>>
> >> >>> On Tue, Nov 29, 2016 at 12:17 PM, Jesus Camacho Rodriguez <
> >> >>> jcama...@apache.org> wrote:
> >> >>>
> >> >>> > Apache Hive 2.1.1 Release Candidate 1 is available here:
> >> >>> > http://people.apache.org/~jcamacho/hive-2.1.1-rc1/
> >> >>> >
> >> >>> > Maven artifacts are available here:
> >> >>> > https://repository.apache.org/content/repositories/
> >> orgapachehive-1066/
> >> >>> >
> >> >>> > Source tag for RC1 is at:
> >> >>> > https://github.com/apache/hive/releases/tag/release-2.1.1-rc1/
> >> >>> >
> >> >>> > Voting will conclude in 72 hours.
> >> >>> >
> >> >>> > Hive PMC Members: Please test and vote.
> >> >>> >
> >> >>> > Thanks.
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>> >
> >> >>>
> >> >>>
> >> >>> --
> >> >>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> >> >>> Java Persistence with Hibernate, Second Edition
> >> >>>  >> >>> tl?ie=UTF8=1789=9325=1617290459&
> >> >>> linkCode=as2=garygregory-20=
> cadb800f39946ec62ea2b1af9fe6a2
> >> b8>
> >> >>>
> >> >>>  garygregory-20=am2=1=
> >> >>> 1617290459>
> >> >>> JUnit in Action, Second Edition
> >> >>>  >> >>> tl?ie=UTF8=1789=9325=1935182021&
> >> >>> linkCode=as2=garygregory-20=
> 31ecd1f6b6d1eaf8886ac902a24de4
> >> 18%22
> >> >>> >
> >> >>>
> >> >>>  garygregory-20=am2=1=
> >> >>> 1935182021>
> >> >>> Spring Batch in Action

Re: [VOTE] Apache Hive 2.1.1 Release Candidate 0

[Thejas Nair]

Yes, that sounds good.


On Tue, Nov 29, 2016 at 4:39 AM, Jesus Camacho Rodriguez <
jcamachorodrig...@hortonworks.com> wrote:

> Thanks for the link Thejas.
>
> As I understand the email thread in the link, adding the following line to
> the NOTICE file would suffice, since we fall in the temporary exclusion
> period?
>
> *This project includes software licensed under the JSON license.*
>
> --
> Jesús
>
>
>
>
>
>
> On 11/29/16, 9:39 AM, "Thejas Nair" <thejas.n...@gmail.com> wrote:
>
> >This is the right link -
> >
> >http://mail-archives.apache.org/mod_mbox/www-legal-discuss/201611.mbox/%
> 3C0CE2E8C9-D9B7-404D-93EF-A1F8B07189BF%40apache.org%3E
> >
> >On Tue, Nov 29, 2016 at 12:29 AM, Jesus Camacho Rodriguez <
> >jcamachorodrig...@hortonworks.com> wrote:
> >
> >> I am cancelling the vote then.
> >>
> >> @Alan, I think you shared the wrong link? Could you share the email you
> >> were referring to so I can modify the NOTICE file and roll out the new
> RC?
> >>
> >> Thanks,
> >> Jesús
> >>
> >>
> >>
> >> On 11/28/16, 10:49 PM, "Alan Gates" <alanfga...@gmail.com> wrote:
> >>
> >> >Per this email, https://hortonworks.webex.com/hortonworks/j.php?MTID=
> >> m85864659ba07e06f1fa31f6c511a424e we will need to modify Hive’s NOTICE
> >> file before we can release.
> >> >
> >> >Alan.
> >> >
> >> >> On Nov 28, 2016, at 14:28, Jesus Camacho Rodriguez <
> jcama...@apache.org>
> >> wrote:
> >> >>
> >> >> Apache Hive 2.1.1 Release Candidate 0 is available here:
> >> >> http://people.apache.org/~jcamacho/hive-2.1.1-rc0/
> >> >>
> >> >> Maven artifacts are available here:
> >> >> https://repository.apache.org/content/repositories/
> orgapachehive-1065/
> >> >>
> >> >> Source tag for RC0 is at:
> >> >> https://github.com/apache/hive/releases/tag/release-2.1.1-rc0/
> >> >>
> >> >> Voting will conclude in 72 hours.
> >> >>
> >> >> Hive PMC Members: Please test and vote.
> >> >>
> >> >> Thanks.
> >> >>
> >> >>
> >> >
> >> >
> >>
>

Re: [VOTE] Apache Hive 2.1.1 Release Candidate 0

[Thejas Nair]

This is the right link -

http://mail-archives.apache.org/mod_mbox/www-legal-discuss/201611.mbox/%3C0CE2E8C9-D9B7-404D-93EF-A1F8B07189BF%40apache.org%3E

On Tue, Nov 29, 2016 at 12:29 AM, Jesus Camacho Rodriguez <
jcamachorodrig...@hortonworks.com> wrote:

> I am cancelling the vote then.
>
> @Alan, I think you shared the wrong link? Could you share the email you
> were referring to so I can modify the NOTICE file and roll out the new RC?
>
> Thanks,
> Jesús
>
>
>
> On 11/28/16, 10:49 PM, "Alan Gates"  wrote:
>
> >Per this email, https://hortonworks.webex.com/hortonworks/j.php?MTID=
> m85864659ba07e06f1fa31f6c511a424e we will need to modify Hive’s NOTICE
> file before we can release.
> >
> >Alan.
> >
> >> On Nov 28, 2016, at 14:28, Jesus Camacho Rodriguez 
> wrote:
> >>
> >> Apache Hive 2.1.1 Release Candidate 0 is available here:
> >> http://people.apache.org/~jcamacho/hive-2.1.1-rc0/
> >>
> >> Maven artifacts are available here:
> >> https://repository.apache.org/content/repositories/orgapachehive-1065/
> >>
> >> Source tag for RC0 is at:
> >> https://github.com/apache/hive/releases/tag/release-2.1.1-rc0/
> >>
> >> Voting will conclude in 72 hours.
> >>
> >> Hive PMC Members: Please test and vote.
> >>
> >> Thanks.
> >>
> >>
> >
> >
>

Re: Review Request 53915: Extend JSONMessageFactory to store additional information about metadata objects on different table events

[Thejas Nair]

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


Ship it!




Ship It!

- Thejas Nair


On Nov. 22, 2016, 11:02 p.m., Vaibhav Gumashta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53915/
> ---
> 
> (Updated Nov. 22, 2016, 11:02 p.m.)
> 
> 
> Review request for hive, Sushanth Sowmyan and Thejas Nair.
> 
> 
> Bugs: HIVE-15180
> https://issues.apache.org/jira/browse/HIVE-15180
> 
> 
> Repository: hive-git
> 
> 
> Description
> ---
> 
> https://issues.apache.org/jira/browse/HIVE-15180
> 
> 
> Diffs
> -
> 
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java a9474c4 
>   
> hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
>  ea7520d 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/api/TestHCatClientNotification.java
>  a661962 
>   
> itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
>  4f97cf4 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/AddPartitionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/AlterIndexMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/AlterPartitionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/AlterTableMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/CreateDatabaseMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/CreateFunctionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/CreateIndexMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/CreateTableMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/DropDatabaseMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/DropFunctionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/DropIndexMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/DropPartitionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/DropTableMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/EventMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/EventUtils.java 
> PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/InsertMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/MessageDeserializer.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/MessageFactory.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONAddPartitionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONAlterIndexMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONAlterPartitionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONAlterTableMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONCreateDatabaseMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONCreateFunctionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONCreateIndexMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONCreateTableMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONDropDatabaseMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONDropFunctionMessage.java
>  PRE-CREATION 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONDropIndexMessage.java
>  PRE-CREATION 
>   
> metastore/sr