You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "ZhangYao (Code Review)" <ge...@cloudera.org> on 2019/11/22 07:20:59 UTC
[kudu-CR] KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
ZhangYao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14783
Change subject: KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
......................................................................
KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
---
M src/kudu/master/placement_policy-test.cc
M src/kudu/master/placement_policy.cc
2 files changed, 37 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/14783/1
--
To view, visit http://gerrit.cloudera.org:8080/14783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Gerrit-Change-Number: 14783
Gerrit-PatchSet: 1
Gerrit-Owner: ZhangYao <tr...@gmail.com>
[kudu-CR] KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14783 )
Change subject: KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14783/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14783/1//COMMIT_MSG@8
PS1, Line 8:
Could you add some description here? The idea is to outline why 2 + 1 is better than 3 + 0, even if the location policy constraints are not held in neither case.
I added an example of such a comment on placement_policy.cc file, maybe just add it in here?
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy-test.cc
File src/kudu/master/placement_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy-test.cc@858
PS1, Line 858: // Before this patch, all replicas will located at "B".
nit: this comment doesn't have enough context if reading it as part of the code, not just part of this patch.
Remove this or replace with the description of what's is going on here. Maybe:
Make sure replicas are placed into both locations, even if ideal density distribution would have them in a single location.
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy.cc
File src/kudu/master/placement_policy.cc:
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy.cc@337
PS1, Line 337: // is (3 + 3), while for 3 locations that's (2 + 2 + 2).
Could you add a small note about distributing 3 replicas into 2 locations? The idea is to point to the benefits of that. Maybe, something like:
In case of distributing 3 replicas among 2 locations, 1 + 2 is better than 3 + 0 because if all servers in the first location become unavailable, in the former case the tablet is still available, while in the latter it's not. Also, 1 + 2 is better than 0 + 3 because in case of catastrophic non-recoverable failure of the second location, no replica survives and all data is lost in the latter case, while in the former case there will be 1 replica and it may be used for manual data recovery.
--
To view, visit http://gerrit.cloudera.org:8080/14783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Gerrit-Change-Number: 14783
Gerrit-PatchSet: 1
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 22 Nov 2019 20:03:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14783 )
Change subject: KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy-test.cc
File src/kudu/master/placement_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy-test.cc@858
PS1, Line 858: // Before this patch, all replicas will located at "B".
> nit: this comment doesn't have enough context if reading it as part of the
Also, I think it's better to make this test-wide comment (i.e. put it at line 847)
--
To view, visit http://gerrit.cloudera.org:8080/14783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Gerrit-Change-Number: 14783
Gerrit-PatchSet: 1
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 22 Nov 2019 20:09:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14783 )
Change subject: KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
......................................................................
KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
In case of distributing 3 replicas among 2 locations, 1 + 2 is better
than 3 + 0 because if all servers in the first location become unavailable,
in the former case the tablet is still available, while in the latter it's
not. Also, 1 + 2 is better than 0 + 3 because in case of catastrophic
non-recoverable failure of the second location, no replica survives and
all data is lost in the latter case, while in the former case there will
be 1 replica and it may be used for manual data recovery.
Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Reviewed-on: http://gerrit.cloudera.org:8080/14783
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/master/placement_policy-test.cc
M src/kudu/master/placement_policy.cc
2 files changed, 46 insertions(+), 5 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Gerrit-Change-Number: 14783
Gerrit-PatchSet: 3
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
[kudu-CR] KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14783 )
Change subject: KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14783/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14783/1//COMMIT_MSG@8
PS1, Line 8:
> Of course:) Done
Thank you!
--
To view, visit http://gerrit.cloudera.org:8080/14783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Gerrit-Change-Number: 14783
Gerrit-PatchSet: 2
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Mon, 25 Nov 2019 17:23:49 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
Posted by "ZhangYao (Code Review)" <ge...@cloudera.org>.
ZhangYao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14783 )
Change subject: KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14783/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14783/1//COMMIT_MSG@8
PS1, Line 8:
> Could you add some description here? The idea is to outline why 2 + 1 is b
Of course:) Done
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy-test.cc
File src/kudu/master/placement_policy-test.cc:
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy-test.cc@858
PS1, Line 858:
> Also, I think it's better to make this test-wide comment (i.e. put it at li
Done
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy-test.cc@858
PS1, Line 858:
> nit: this comment doesn't have enough context if reading it as part of the
Done
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy.cc
File src/kudu/master/placement_policy.cc:
http://gerrit.cloudera.org:8080/#/c/14783/1/src/kudu/master/placement_policy.cc@337
PS1, Line 337: // is (3 + 3), while for 3 locations that's (2 + 2 + 2). In case of
> Could you add a small note about distributing 3 replicas into 2 locations?
Done
--
To view, visit http://gerrit.cloudera.org:8080/14783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Gerrit-Change-Number: 14783
Gerrit-PatchSet: 2
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: ZhangYao <tr...@gmail.com>
Gerrit-Comment-Date: Sat, 23 Nov 2019 12:20:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
Posted by "ZhangYao (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14783
to look at the new patch set (#2).
Change subject: KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
......................................................................
KUDU-3008 Spread replicas evenly with 2 locations and odd replica factors
In case of distributing 3 replicas among 2 locations, 1 + 2 is better
than 3 + 0 because if all servers in the first location become unavailable,
in the former case the tablet is still available, while in the latter it's
not. Also, 1 + 2 is better than 0 + 3 because in case of catastrophic
non-recoverable failure of the second location, no replica survives and
all data is lost in the latter case, while in the former case there will
be 1 replica and it may be used for manual data recovery.
Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
---
M src/kudu/master/placement_policy-test.cc
M src/kudu/master/placement_policy.cc
2 files changed, 46 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/83/14783/2
--
To view, visit http://gerrit.cloudera.org:8080/14783
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If96361f6be686679dd72f08e28d964eae25875d0
Gerrit-Change-Number: 14783
Gerrit-PatchSet: 2
Gerrit-Owner: ZhangYao <tr...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)