You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2019/06/05 20:45:10 UTC
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13521
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
[c++] Support table rename between scan token creation and rehydration
Previously, if a scan token was created against table 'foo', and table
'foo' was renamed to table 'bar', rehydrating the scan token into a
scanner would either
- fail if there is no longer a table 'foo', or
- attempt to scan another table if that table had been renamed to 'foo'.
This patch alters how the C++ client manages scan tokens. It prefers to
use the table id to identify a table, rather than a table name. This
eliminates the above two problems.
This is a follow up to the Java client patch for the same (and that
patch includes the necessary protobuf changes).
Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
5 files changed, 157 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13521/1
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@144
PS2, Line 144: enum class TableIdentifierType {
> Maybe just pass the TableIdentifierPB into these functions directly? They'r
Done
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@151
PS2, Line 151: Status OpenTable(KuduClient* client,
> warning: function 'kudu::client::KuduClient::Data::OpenTable' has a definit
Done
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@152
PS2, Line 152: identifer
> ah, it's complaining about a typo here: should be indentifier_type
Done
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@169
PS2, Line 169: Status GetTableSchema(KuduClient* client,
: const std::string& table_name,
: const MonoTime& deadline,
: KuduSchema* schema,
: PartitionSchema* partition_schema,
: std::string* table_id,
: int* num_replicas);
> Can we eliminate this variant? Seems like it's only used in one or two plac
Done
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.cc@563
PS2, Line 563: nullptr
> nit: /*table_name=*/nullptr
<code deleted>
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 17:56:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 2: Code-Review+1
(2 comments)
Looks good, just some nits and a failing existing casing-related test
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@152
PS2, Line 152: identifer
ah, it's complaining about a typo here: should be indentifier_type
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.cc@563
PS2, Line 563: nullptr
nit: /*table_name=*/nullptr
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jun 2019 01:47:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 3: Code-Review+2
lgtm, might want to check w/ Adar as well
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 18:59:09 +0000
Gerrit-HasComments: No
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13521
to look at the new patch set (#4).
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
[c++] Support table rename between scan token creation and rehydration
Previously, if a scan token was created against table 'foo', and table
'foo' was renamed to table 'bar', rehydrating the scan token into a
scanner would either
- fail if there is no longer a table 'foo', or
- attempt to scan another table if that table had been renamed to 'foo'.
This patch alters how the C++ client manages scan tokens. It prefers to
use the table id to identify a table, rather than a table name. This
eliminates the above two problems.
This is a follow up to the Java client patch for the same (and that
patch includes the necessary protobuf changes).
Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
5 files changed, 132 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13521/4
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 21:35:21 +0000
Gerrit-HasComments: No
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 4:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.h@148
PS3, Line 148: // 'table_id_or_name' should contain a table id or name as 'identifier_type'
: // is ID or NAME, respectively.
: Status GetTableSchema(KuduClien
> Update this.
Done
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.h@156
PS3, Line 156: std::string* table_name,
: int* num_replicas);
> You could also make the contract such that 'table' is both IN and OUT, and
Negatory.
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.cc@457
PS3, Line 457: const TableIdentifierPB& table_identifier,
> warning: the parameter 'table_identifier' is copied for each invocation but
Done
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.cc@457
PS3, Line 457: const TableIdentifierPB& table_identifier,
> Agreed with clang-tidy here (and below).
Done
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.cc@497
PS3, Line 497: const TableIdentifierPB& table,
> warning: the parameter 'table' is copied for each invocation but only used
Done
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/scan_token-internal.cc@48
PS3, Line 48: #include "kudu/gutil/ref_counted.h"
> And this too.
Done
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/scan_token-internal.cc@48
PS3, Line 48: #include "kudu/gutil/ref_counted.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 21:11:23 +0000
Gerrit-HasComments: Yes
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
[c++] Support table rename between scan token creation and rehydration
Previously, if a scan token was created against table 'foo', and table
'foo' was renamed to table 'bar', rehydrating the scan token into a
scanner would either
- fail if there is no longer a table 'foo', or
- attempt to scan another table if that table had been renamed to 'foo'.
This patch alters how the C++ client manages scan tokens. It prefers to
use the table id to identify a table, rather than a table name. This
eliminates the above two problems.
This is a follow up to the Java client patch for the same (and that
patch includes the necessary protobuf changes).
Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Reviewed-on: http://gerrit.cloudera.org:8080/13521
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
5 files changed, 132 insertions(+), 31 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
Mike Percy: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13521/1/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
http://gerrit.cloudera.org:8080/#/c/13521/1/src/kudu/client/client-internal.cc@456
PS1, Line 456: Status KuduClient::Data::OpenTable(KuduClient* client,
> error: out-of-line definition of 'OpenTable' does not match any declaration
Done
http://gerrit.cloudera.org:8080/#/c/13521/1/src/kudu/client/client-internal.cc@476
PS1, Line 476: // TODO: in the future, probably will look up the table in some map to reuse
> warning: missing username/bug in TODO [google-readability-todo]
Done
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 05 Jun 2019 20:51:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13521
to look at the new patch set (#3).
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
[c++] Support table rename between scan token creation and rehydration
Previously, if a scan token was created against table 'foo', and table
'foo' was renamed to table 'bar', rehydrating the scan token into a
scanner would either
- fail if there is no longer a table 'foo', or
- attempt to scan another table if that table had been renamed to 'foo'.
This patch alters how the C++ client manages scan tokens. It prefers to
use the table id to identify a table, rather than a table name. This
eliminates the above two problems.
This is a follow up to the Java client patch for the same (and that
patch includes the necessary protobuf changes).
Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
5 files changed, 133 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13521/3
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@144
PS2, Line 144: enum class TableIdentifierType {
Maybe just pass the TableIdentifierPB into these functions directly? They're all internal so it should be a clean way to encapsulate the "this string or that string" logic without new code.
Too bad we can't replace the table name string with a TableIdentifierPB in the scan token protobuf though.
http://gerrit.cloudera.org:8080/#/c/13521/2/src/kudu/client/client-internal.h@169
PS2, Line 169: Status GetTableSchema(KuduClient* client,
: const std::string& table_name,
: const MonoTime& deadline,
: KuduSchema* schema,
: PartitionSchema* partition_schema,
: std::string* table_id,
: int* num_replicas);
Can we eliminate this variant? Seems like it's only used in one or two places; shouldn't be much more work for existing callers to use the new variant you added.
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Jun 2019 05:29:10 +0000
Gerrit-HasComments: Yes
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 22:59:54 +0000
Gerrit-HasComments: No
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13521 )
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.h
File src/kudu/client/client-internal.h:
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.h@142
PS3, Line 142: // Open the table identifier by 'table_id_or_name'. 'table_id_or_name' should
: // contain a table id or name as 'identifier_type' is ID or NAME, respectively.
Update this.
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.h@148
PS3, Line 148: // Retrieve table information about the table identified by 'table_id_or_name'.
: // 'table_id_or_name' should contain a table id or name as 'identifier_type'
: // is ID or NAME, respectively.
Update this.
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.h@156
PS3, Line 156: std::string* table_id,
: std::string* table_name,
You could also make the contract such that 'table' is both IN and OUT, and GetTableSchema will update table's values to reflect both the ID and name.
Not sure if it's much clearer though.
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.cc
File src/kudu/client/client-internal.cc:
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/client-internal.cc@457
PS3, Line 457: TableIdentifierPB table_identifier,
> warning: the parameter 'table_identifier' is copied for each invocation but
Agreed with clang-tidy here (and below).
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/scan_token-internal.cc
File src/kudu/client/scan_token-internal.cc:
http://gerrit.cloudera.org:8080/#/c/13521/3/src/kudu/client/scan_token-internal.cc@48
PS3, Line 48: #include "kudu/master/master.pb.h"
> warning: #includes are not sorted properly [llvm-include-order]
And this too.
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 3
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 07 Jun 2019 21:03:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] [c++] Support table rename between scan token creation and rehydration
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13521
to look at the new patch set (#2).
Change subject: [c++] Support table rename between scan token creation and rehydration
......................................................................
[c++] Support table rename between scan token creation and rehydration
Previously, if a scan token was created against table 'foo', and table
'foo' was renamed to table 'bar', rehydrating the scan token into a
scanner would either
- fail if there is no longer a table 'foo', or
- attempt to scan another table if that table had been renamed to 'foo'.
This patch alters how the C++ client manages scan tokens. It prefers to
use the table id to identify a table, rather than a table name. This
eliminates the above two problems.
This is a follow up to the Java client patch for the same (and that
patch includes the necessary protobuf changes).
Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
---
M src/kudu/client/client-internal.cc
M src/kudu/client/client-internal.h
M src/kudu/client/client.cc
M src/kudu/client/scan_token-internal.cc
M src/kudu/client/scan_token-test.cc
5 files changed, 157 insertions(+), 29 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/21/13521/2
--
To view, visit http://gerrit.cloudera.org:8080/13521
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib4d48513dff67012f26a99877b168d777d3049fd
Gerrit-Change-Number: 13521
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)