You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2021/06/09 19:27:01 UTC

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore 

kishen@cloudera.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17569


Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................

IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
---
M fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java
1 file changed, 81 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/17569/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Posted by "Yu-Wen Lai (Code Review)" <ge...@cloudera.org>.
Yu-Wen Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17569 )

Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17569/2/fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java
File fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java:

http://gerrit.cloudera.org:8080/#/c/17569/2/fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java@344
PS2, Line 344: ,
nit: put a space after comma



-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 21:38:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Vihang Karajgaonkar, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17569

to look at the new patch set (#2).

Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................

IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
---
M fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java
1 file changed, 84 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/17569/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17569 )

Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................


Patch Set 2:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/8900/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Sun, 13 Jun 2021 17:59:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17569 )

Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17569/2/fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java
File fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java:

http://gerrit.cloudera.org:8080/#/c/17569/2/fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java@28
PS2, Line 28: import org.apache.hadoop.hive.metastore.DefaultThriftHiveMetastore.RemoteCacheMetaStoreClient;
line too long (94 > 90)



-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Fri, 11 Jun 2021 21:26:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17569 )

Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................


Patch Set 1:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/8871/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Jun 2021 19:38:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17569 )

Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17569/1/fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java
File fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java:

http://gerrit.cloudera.org:8080/#/c/17569/1/fe/src/main/java/org/apache/impala/catalog/metastore/MetastoreServiceHandler.java@340
PS1, Line 340:   public IMetaStoreClient getMetaStoreClient() {
             :     try (MetaStoreClient client = catalog_.getMetaStoreClient()){
             :       return client.getHiveClient();
             :     }
This logic won't work. The try with resources block will put back the MetastoreClient into the pool again. This would cause a leak of IMetastoreClient from the MetaStoreClientPool and same IMetastoreClient could be used to issue multiple HMS API calls concurrently and may cause race conditions like (one thread closing the client while other thread is issuing a HMS call).



-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Wed, 09 Jun 2021 21:52:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore

Posted by "Vihang Karajgaonkar (Code Review)" <ge...@cloudera.org>.
Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17569 )

Change subject: IMPALA-10740: MetastoreServiceHandler should extend DefaultThriftHiveMetastore
......................................................................


Patch Set 2:

Can you rebase the patch and see why the gerrit-code-review-checks job is coming back as failed? The previous run log is not available anymore.


-- 
To view, visit http://gerrit.cloudera.org:8080/17569
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e3f74dd96a7fec2ed13b0e5929f2b0a6b66e39f
Gerrit-Change-Number: 17569
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <ki...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Reviewer: Yu-Wen Lai <yu...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Jul 2021 22:25:26 +0000
Gerrit-HasComments: No