You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yao Xu (Code Review)" <ge...@cloudera.org> on 2019/05/22 08:26:34 UTC
[kudu-CR] KUDU-2802 [java] tableExists API optimizations
Yao Xu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13401
Change subject: KUDU-2802 [java] tableExists API optimizations
......................................................................
KUDU-2802 [java] tableExists API optimizations
Currently the Java client `tableExists()` method uses a ListTables
rpc. Instead it should use a GetTableSchema rpc. Because ListTables
is going to perform really poorly with a cold Sentry cache: it'll
send an RPC to Sentry for every table in the catalog.
Change-Id: I7076aea3850a716822563938d879289beb3fa67c
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 17 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/01/13401/1
--
To view, visit http://gerrit.cloudera.org:8080/13401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7076aea3850a716822563938d879289beb3fa67c
Gerrit-Change-Number: 13401
Gerrit-PatchSet: 1
Gerrit-Owner: Yao Xu <oc...@gmail.com>
[kudu-CR] KUDU-2802 [java] tableExists API optimizations
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13401 )
Change subject: KUDU-2802 [java] tableExists API optimizations
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13401/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13401/1//COMMIT_MSG@12
PS1, Line 12: send an RPC to Sentry for every table in the catalog.
> that's a bit concerning on its own. Should we be priming table-level permis
Yes, this is an open issue that needs to be addressed server-side. Hao and Grant are aware of it.
--
To view, visit http://gerrit.cloudera.org:8080/13401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7076aea3850a716822563938d879289beb3fa67c
Gerrit-Change-Number: 13401
Gerrit-PatchSet: 2
Gerrit-Owner: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 22 May 2019 16:56:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2802 [java] tableExists API optimizations
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13401 )
Change subject: KUDU-2802 [java] tableExists API optimizations
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13401/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/13401/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@845
PS1, Line 845: if (e instanceof NonRecoverableException) {
FWIW, I think this may be guaranteed. That is, by the time we've called this errback, any recoverable exceptions have likely been retried. If we make it here, it's because we got an error we couldn't recover from, or we timed out while retrying a recoverable error.
But this is more defensive, so let's keep it the way it is.
--
To view, visit http://gerrit.cloudera.org:8080/13401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7076aea3850a716822563938d879289beb3fa67c
Gerrit-Change-Number: 13401
Gerrit-PatchSet: 1
Gerrit-Owner: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 22 May 2019 16:55:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2802 [java] tableExists API optimizations
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13401 )
Change subject: KUDU-2802 [java] tableExists API optimizations
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13401/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13401/1//COMMIT_MSG@12
PS1, Line 12: send an RPC to Sentry for every table in the catalog.
that's a bit concerning on its own. Should we be priming table-level permissions in the background or something?
--
To view, visit http://gerrit.cloudera.org:8080/13401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7076aea3850a716822563938d879289beb3fa67c
Gerrit-Change-Number: 13401
Gerrit-PatchSet: 1
Gerrit-Owner: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 22 May 2019 16:54:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2802 [java] tableExists API optimizations
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13401 )
Change subject: KUDU-2802 [java] tableExists API optimizations
......................................................................
KUDU-2802 [java] tableExists API optimizations
Currently the Java client `tableExists()` method uses a ListTables
rpc. Instead it should use a GetTableSchema rpc. Because ListTables
is going to perform really poorly with a cold Sentry cache: it'll
send an RPC to Sentry for every table in the catalog.
Change-Id: I7076aea3850a716822563938d879289beb3fa67c
Reviewed-on: http://gerrit.cloudera.org:8080/13401
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
1 file changed, 17 insertions(+), 8 deletions(-)
Approvals:
Kudu Jenkins: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13401
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7076aea3850a716822563938d879289beb3fa67c
Gerrit-Change-Number: 13401
Gerrit-PatchSet: 2
Gerrit-Owner: Yao Xu <oc...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>