You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2016/08/31 03:19:01 UTC
[kudu-CR] c++ client: expose private GetTablet API
Hello Mike Percy, Todd Lipcon,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4179
to review the following change.
Change subject: c++ client: expose private GetTablet API
......................................................................
c++ client: expose private GetTablet API
The new API can be used to look up tablet information (i.e. replicas and
roles) using a tablet ID.
I'm going to use it in kudu-admin; the "change config" operation needs to
know which tserver hosts the tablet's leader replica. Without this new API,
we'd have to go through the scan token API, which means either forcing the
user to also provide the table name, or abusing the scan token API by
creating tokens for _all_ tables, then hunting for the matching tablet.
As such, I've opted to make this API "private". I've done so via
KUDU_NO_EXPORT, though I could have just as easily declared it a private
method and used friendship. This way it can more easily be tested (as tests
do not use the "exported" client library).
Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
4 files changed, 137 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/4179/1
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged.
Change subject: c++ client: expose private GetTablet API
......................................................................
c++ client: expose private GetTablet API
The new API can be used to look up tablet information (i.e. replicas and
roles) using a tablet ID.
I'm going to use it in kudu-admin; the "change config" operation needs to
know which tserver hosts the tablet's leader replica. Without this new API,
we'd have to go through the scan token API, which means either forcing the
user to also provide the table name, or abusing the scan token API by
creating tokens for _all_ tables, then hunting for the matching tablet.
As such, I've opted to make this API "private". I've done so via
KUDU_NO_EXPORT, though I could have just as easily declared it a private
method and used friendship. This way it can more easily be tested (as tests
do not use the "exported" client library).
Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Reviewed-on: http://gerrit.cloudera.org:8080/4179
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
4 files changed, 141 insertions(+), 9 deletions(-)
Approvals:
Todd Lipcon: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: c++ client: expose private GetTablet API
......................................................................
Patch Set 2:
Build Started http://104.196.14.100/job/kudu-gerrit/3174/
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: c++ client: expose private GetTablet API
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4179/1/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 351: /// Private API.
> is it possible to get doxygen to not document this?
I'll add a tag to force doxygen to ignore this. I'm inclined to punt on the snipping, at least until we have more private APIs.
Line 362: KuduTablet** tablet);
> if we had the 'NO_DISTRIBUTE' type thing then we could use unique_ptr here
Not easily, because we'd still need to #include <memory> somewhere, and that would need to get excised too (since TR1 has it in tr1/memory).
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: c++ client: expose private GetTablet API
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: c++ client: expose private GetTablet API
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4179/1/src/kudu/client/client.h
File src/kudu/client/client.h:
Line 351: /// Private API.
is it possible to get doxygen to not document this?
I also wonder whether we could do something by which the installed client.h that comes as part of the dev package snips this out?
(eg via a // NO_DISTRIBUTE .. // END_NO_DISTRIBUTE marker or something)
Line 362: KuduTablet** tablet);
if we had the 'NO_DISTRIBUTE' type thing then we could use unique_ptr here for the out-param
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: c++ client: expose private GetTablet API
......................................................................
Patch Set 1: -Verified
Build Started http://104.196.14.100/job/kudu-gerrit/3162/
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: c++ client: expose private GetTablet API
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/3159/
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] c++ client: expose private GetTablet API
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/4179
to look at the new patch set (#2).
Change subject: c++ client: expose private GetTablet API
......................................................................
c++ client: expose private GetTablet API
The new API can be used to look up tablet information (i.e. replicas and
roles) using a tablet ID.
I'm going to use it in kudu-admin; the "change config" operation needs to
know which tserver hosts the tablet's leader replica. Without this new API,
we'd have to go through the scan token API, which means either forcing the
user to also provide the table name, or abusing the scan token API by
creating tokens for _all_ tables, then hunting for the matching tablet.
As such, I've opted to make this API "private". I've done so via
KUDU_NO_EXPORT, though I could have just as easily declared it a private
method and used friendship. This way it can more easily be tested (as tests
do not use the "exported" client library).
Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/client.h
4 files changed, 141 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/4179/2
--
To view, visit http://gerrit.cloudera.org:8080/4179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If798820e5d790d07f554aaa6f89b31aaf360a3a5
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>