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 2017/06/09 22:08:37 UTC
[kudu-CR] Use the same replica selection when adding a server as table creation
Will Berkeley has uploaded a new change for review.
http://gerrit.cloudera.org:8080/7143
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Use the same replica selection when adding a server as table creation
When selecting TS to host replicas at table creation time, the
master uses a selection algorithm that (roughly) picks the less
loaded of two randomly chosen TS that haven't already been
picked to hold a replica. When adding a replica, for instance
after an eviction, it just picks a tserver at random from those
not already holding a replica. Let's use the same algorithm for
both, since the table creation algorithm has the benefit that it
tends to fill less-loaded tablet servers, like ones newly added
to the cluster.
Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
2 files changed, 103 insertions(+), 121 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/7143/1
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7143/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3352: rng_(GetRandomSeed32()) {
Can we reuse the catalog manager's ThreadSafeRandom instead of making a new one per task?
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/7143/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3248: vector<shared_ptr<TSDescriptor> > two_choices;
nit: in c++11 using >> is no longer a compiler parse error so no need for the space in between the brackets
Line 3255: if (two_choices.size() == 1) {
Why remove the CHECK from L3901 in the previous version of the file?
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-HasComments: Yes
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/7143
to look at the new patch set (#2).
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Use the same replica selection when adding a server as table creation
When selecting TS to host replicas at table creation time, the
master uses a selection algorithm that (roughly) picks the less
loaded of two randomly chosen TS that haven't already been
picked to hold a replica. When adding a replica, for instance
after an eviction, it just picks a tserver at random from those
not already holding a replica. Let's use the same algorithm for
both, since the table creation algorithm has the benefit that it
tends to fill less-loaded tablet servers, like ones newly added
to the cluster.
Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
2 files changed, 108 insertions(+), 123 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/43/7143/2
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Patch Set 1:
(6 comments)
Want some feedback from Adar before submitting the next PS
http://gerrit.cloudera.org:8080/#/c/7143/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3182: ThreadSafeRandom& rng) {
> warning: non-const reference parameter 'rng', make it const or use a pointe
Done
Line 3208: } else if (load_b < load_a) {
> warning: do not use 'else' after 'return' [readability-else-after-return]
Done
Line 3221: ThreadSafeRandom& rng) {
> warning: non-const reference parameter 'rng', make it const or use a pointe
Done
Line 3248: vector<shared_ptr<TSDescriptor> > two_choices;
> nit: in c++11 using >> is no longer a compiler parse error so no need for t
Done
Line 3255: if (two_choices.size() == 1) {
> Why remove the CHECK from L3901 in the previous version of the file?
The DCHECK asserted that, if there's not 2 choices, there's 1 choice. However, this function is now used in the add replica case as well as the create table case, and it's possible in the former that there's 2, 1, or 0 choices. The equivalent DCHECK for the create table case is now just after the call to SelectReplica() in SelectReplicas() ~L3906.
Line 3352: rng_(GetRandomSeed32()) {
> Can we reuse the catalog manager's ThreadSafeRandom instead of making a new
What's your advice on how to do that? If you copy it then that would copy the state and potentially all the tasks spawned at about the same time would pick the same server. Should AsyncAddServer task hold a bare pointer to catalog manager's rng_? It seems safe since the catalog manager waits for its tasks to be aborted/destroyed before it shuts down itself.
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7143/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
Line 3352: rng_(GetRandomSeed32()) {
> What's your advice on how to do that? If you copy it then that would copy t
Yeah I think a bare pointer to the RNG should be fine (for the reason you provided).
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has submitted this change and it was merged.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Use the same replica selection when adding a server as table creation
When selecting TS to host replicas at table creation time, the
master uses a selection algorithm that (roughly) picks the less
loaded of two randomly chosen TS that haven't already been
picked to hold a replica. When adding a replica, for instance
after an eviction, it just picks a tserver at random from those
not already holding a replica. Let's use the same algorithm for
both, since the table creation algorithm has the benefit that it
tends to fill less-loaded tablet servers, like ones newly added
to the cluster.
Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Reviewed-on: http://gerrit.cloudera.org:8080/7143
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
2 files changed, 108 insertions(+), 123 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7143/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
PS1, Line 3412: target_ts_desc_
replacement_replica
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/7143/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
PS1, Line 3412: target_ts_desc_
> replacement_replica
Done
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] Use the same replica selection when adding a server as table creation
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: Use the same replica selection when adding a server as table creation
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/7143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I199e7a59c2c7832e7a87842b357ba3aa29e34685
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No