[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Patch Set 3:

> (1 comment)

And another option is ReplicaMatchPolicy -- I think I'll use that one.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 05 Oct 2017 20:10:46 +
Gerrit-HasComments: No

[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8161/3/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/8161/3/src/kudu/master/master.proto@378
PS3, Line 378: ReplicaSelector
> Bike-shedding, but "ReplicaSelector" (especially when seen in C++) suggests
That's a good observation, thanks.

I just found we already have ReplicaSelection enumeration in Kudu C++ client 
API, so maybe TabletReplicaType would fit better.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 05 Oct 2017 20:08:37 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] introduce replica selector

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8161/3/src/kudu/master/master.proto
File src/kudu/master/master.proto:

http://gerrit.cloudera.org:8080/#/c/8161/3/src/kudu/master/master.proto@378
PS3, Line 378: ReplicaSelector
Bike-shedding, but "ReplicaSelector" (especially when seen in C++) suggests a 
stateful class that uses some sort of heuristic or algorithm to make a 
selection. The reality is that it's just an enum, so perhaps 
ReplicaSelectionMode would be a clearer name?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 02 Oct 2017 21:36:53 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/8161/2/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

http://gerrit.cloudera.org:8080/#/c/8161/2/src/kudu/master/catalog_manager.h@692
PS2, Line 692:   Status BuildLocationsForTablet(const 
scoped_refptr& tablet,
> warning: function 'kudu::master::CatalogManager::BuildLocationsForTablet' h
Done


http://gerrit.cloudera.org:8080/#/c/8161/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8161/2/src/kudu/master/catalog_manager.cc@4110
PS2, Line 4110: } else if (s.IsNotFound()) {
> warning: do not use 'else' after 'continue' [readability-else-after-return]
Done


http://gerrit.cloudera.org:8080/#/c/8161/2/src/kudu/master/master_service.cc
File src/kudu/master/master_service.cc:

http://gerrit.cloudera.org:8080/#/c/8161/2/src/kudu/master/master_service.cc@239
PS2, Line 239: // TODO: once we have catalog data. ACL checks would also go 
here, probably.
> warning: missing username/bug in TODO [google-readability-todo]
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 29 Sep 2017 23:13:34 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Hello Tidy Bot, Kudu Jenkins, Todd Lipcon,

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

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

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

Change subject: [catalog manager] introduce replica selector
..

[catalog manager] introduce replica selector

Introduced replica selector for GetTabletLocations RPC and
various wrapper methods used in master, catalog manager, and tests.
The selector is used to specify replica membership matching policy
while populating the result of the GetTabletLocations RPC.

As of now, only VOTER_REPLICA policy is used for all GetTabletLocations
invocations and only VOTER replicas are returned in the result.
As soon as NON_VOTER replicas appear in a follow-up commit, it will
be possible to use ANY_REPLICA policy with GetTabletLocation request
to receive information on NON_VOTER replicas as well.

The 'regular' client operations should work with voter replicas only.
The use case for getting all replicas is when the kudu CLI tool fetches
that information to report on replica membership status for listings
in 'table list --tablets' and 'ksck' sub-commands.

Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/tools/kudu-admin-test.cc
10 files changed, 164 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8161/3
--
To view, visit http://gerrit.cloudera.org:8080/8161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [catalog manager] introduce replica selector

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8161/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8161/2//COMMIT_MSG@18
PS2, Line 18: to receive information on NON_VOTER replicas as well.
> The 'regular' client operations should work with voter replicas only, as I
k, would be good to include this in the commit message as well



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 29 Sep 2017 17:51:36 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8161/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8161/2//COMMIT_MSG@18
PS2, Line 18: to receive information on NON_VOTER replicas as well.
> can you elaborate on the use case here? What client operations need to spec
The 'regular' client operations should work with voter replicas only, as I 
understand.

The use case for getting all replicas is when kudu CLI tool fetches that 
information to report on replica membership status for listings in 'table list 
--tablets' and 'ksck' sub-commands.

I hope to post corresponding patch for that later today.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 28 Sep 2017 22:39:50 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] introduce replica selector

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8161/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8161/2//COMMIT_MSG@18
PS2, Line 18: to receive information on NON_VOTER replicas as well.
can you elaborate on the use case here? What client operations need to specify 
a selection policy instead of always returning all members, and why? Would be 
nice to motivate the change use-case-first to evaluate whether the protocol 
changes make sense for what is needed by the clients, since it's pretty hard to 
change the proto later if it doesn't fit.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 28 Sep 2017 20:21:37 +
Gerrit-HasComments: Yes

[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..

[catalog manager] introduce replica selector

Introduced replica selector for GetTabletLocations RPC and
various wrapper methods used in master, catalog manager, and tests.
The selector is used to specify replica membership matching policy
while populating the result of the GetTabletLocations RPC.

As of now, only VOTER_REPLICA policy is used for all GetTabletLocations
invocations and only VOTER replicas are returned in the result.
As soon as NON_VOTER replicas appear in a follow-up commit, it will
be possible to use ANY_REPLICA policy with GetTabletLocation request
to receive information on NON_VOTER replicas as well.

Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/integration-tests/table_locations-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/tools/kudu-admin-test.cc
10 files changed, 160 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8161/2
--
To view, visit http://gerrit.cloudera.org:8080/8161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 

[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Alexey Serbin has removed Kudu Jenkins from this change.  ( 
http://gerrit.cloudera.org:8080/8161 )

Change subject: [catalog manager] introduce replica selector
..


Removed reviewer Kudu Jenkins.
--
To view, visit http://gerrit.cloudera.org:8080/8161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 

[kudu-CR] [catalog manager] introduce replica selector

[Alexey Serbin (Code Review)]

Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8161


Change subject: [catalog manager] introduce replica selector
..

[catalog manager] introduce replica selector

Introduced replica selector for GetTabletLocations RPC and
various wrapper methods used in master, catalog manager, and tests.

The selector will be used for listing tablet replicas once NON_VOTER
replicas are introduced in a follow-up commit.

Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/registration-test.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.proto
M src/kudu/master/master_service.cc
M src/kudu/tools/kudu-admin-test.cc
9 files changed, 113 insertions(+), 47 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8161/1
--
To view, visit http://gerrit.cloudera.org:8080/8161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I303a6d158184575a9a105c2d2bf26961ae8b3e93
Gerrit-Change-Number: 8161
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin