You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2020/10/07 22:12:42 UTC
[kudu-CR] [master] introduce filter type for ListTables
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16561
Change subject: [master] introduce filter_type for ListTables
......................................................................
[master] introduce filter_type for ListTables
This patch introduces a new 'filter_type' field for the
ListTablesRequestPB message. With this patch, it's possible
to explicitly filter tables to be returned from the system catalog.
I added corresponding unit test as well.
This patch doesn't update kudu CLI -- this is to be done
in a separate patch.
Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.proto
3 files changed, 138 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/16561/1
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [master] introduce filter type for ListTables
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16561 )
Change subject: [master] introduce filter_type for ListTables
......................................................................
[master] introduce filter_type for ListTables
This patch introduces a new 'filter_type' field for the
ListTablesRequestPB message. With this patch, it's possible
to explicitly filter tables to be returned from the system catalog.
I added corresponding unit test as well.
This patch doesn't update kudu CLI -- this is to be done
in a separate patch.
Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Reviewed-on: http://gerrit.cloudera.org:8080/16561
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.proto
3 files changed, 138 insertions(+), 10 deletions(-)
Approvals:
Andrew Wong: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [master] introduce filter type for ListTables
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 )
Change subject: [master] introduce filter_type for ListTables
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG@7
PS1, Line 7: [master] introduce filter_type for ListTables
Just making sure I understand the context of this work: I was under the assumption this blocked some tests, but I don't think that's the case, given we already don't list system tables from the ListTables() endpoint. This is more about adding functionality to the CLI tool than unblocking tests -- is that right?
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Oct 2020 22:27:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 )
Change subject: [master] introduce filter_type for ListTables
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG@7
PS1, Line 7: [master] introduce filter_type for ListTables
> That's right -- that's a piece to be consistent with table type filtering.
Thanks for the context. It actually clarified some things for me as well. I'd thought you were blocked on some ListTables-related behavior and I assumed you were working on this to unblock tests, so I took up what I assumed was the less important ListTablets behavior. If it's actually the ListTablets behavior you need to unbreak tests, feel free to take over the patch I have:
https://gerrit.cloudera.org/c/16560/
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Oct 2020 23:05:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 )
Change subject: [master] introduce filter_type for ListTables
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16561/1//COMMIT_MSG@7
PS1, Line 7: [master] introduce filter_type for ListTables
> Just making sure I understand the context of this work: I was under the ass
That's right -- that's a piece to be consistent with table type filtering. I haven't see breakages of current tests due to the absence of this piece in the scope of my TxnManager patch.
Since we discussed this offline, I'd like to add some context here (it might be useful for other reviewers and for posterity at least).
The story behind this change is that I was pondering about adding 'filter_type' for ListTablets since the current behavior of ListTablets leaves many tests (50+) broken once TxnManager creates txn status table upon initialization. However, adding that field only for ListTablets but leaving ListTables without corresponding functionality seemed not very consistent to me, given that I anticipated we were to add this flag anyways at least in sake of kudu CLI's functionality. We discussed this offline, and you were ready to help there with the corresponding piece for ListTablets, and I took care of ListTables in this patch.
http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc@696
PS1, Line 696: fitler
> typo
Done
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Oct 2020 22:53:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 )
Change subject: [master] introduce filter_type for ListTables
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/16561/1/src/kudu/master/master-test.cc@696
PS1, Line 696: fitler
typo
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Oct 2020 22:20:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] [master] introduce filter type for ListTables
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16561 )
Change subject: [master] introduce filter_type for ListTables
......................................................................
Patch Set 2:
> (1 comment)
Thank you for putting together the patch! I'll take care of https://gerrit.cloudera.org/c/16560/ revving it further.
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 07 Oct 2020 23:36:10 +0000
Gerrit-HasComments: No
[kudu-CR] [master] introduce filter type for ListTables
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16561
to look at the new patch set (#2).
Change subject: [master] introduce filter_type for ListTables
......................................................................
[master] introduce filter_type for ListTables
This patch introduces a new 'filter_type' field for the
ListTablesRequestPB message. With this patch, it's possible
to explicitly filter tables to be returned from the system catalog.
I added corresponding unit test as well.
This patch doesn't update kudu CLI -- this is to be done
in a separate patch.
Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/master-test.cc
M src/kudu/master/master.proto
3 files changed, 138 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/16561/2
--
To view, visit http://gerrit.cloudera.org:8080/16561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9d81ad740d2fc1bad0d1ce8a26cb9b9c6429c786
Gerrit-Change-Number: 16561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)