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 2018/09/12 04:44:59 UTC
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11429
Change subject: [catalog_manager] optimization in AsyncAddReplicaTask
......................................................................
[catalog_manager] optimization in AsyncAddReplicaTask
Introduced a minor optimization on AsyncAddReplicaTask::SendRequest()
while populating the set of blacklisted candidates for an extra replica.
With this patch, the code iterates over the UUIDs of the peers in the
committed config to populate the set of the blacklisted candidates,
rather than filtering all tablet servers using the IsRaftConfigMember()
utility function.
Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
---
M src/kudu/master/catalog_manager.cc
1 file changed, 9 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/11429/1
--
To view, visit http://gerrit.cloudera.org:8080/11429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
Gerrit-Change-Number: 11429
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11429 )
Change subject: [catalog_manager] optimization in AsyncAddReplicaTask
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3474
PS1, Line 3474: excluded.emplace(std::move(desc));
> Nit: EmplaceOrDie()
Done
http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3480
PS1, Line 3480: auto replacement_replica = SelectReplica(ts_descs, excluded, rng_);
> Does SelectReplica() assume that every member of 'excluded' must be in 'ts_
SelectReplica() does not assume that 'excluded' to be among 'ts_descs'.
The 'presumably dead' servers are not included into 'ts_descs', but they might be in the 'excluded' set. That should work as expected.
Overall it's a good point; I add a comment about that to clarify.
--
To view, visit http://gerrit.cloudera.org:8080/11429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
Gerrit-Change-Number: 11429
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 05:31:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11429 )
Change subject: [catalog_manager] optimization in AsyncAddReplicaTask
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3474
PS1, Line 3474: excluded.emplace(std::move(desc));
Nit: EmplaceOrDie()
http://gerrit.cloudera.org:8080/#/c/11429/1/src/kudu/master/catalog_manager.cc@3480
PS1, Line 3480: auto replacement_replica = SelectReplica(ts_descs, excluded, rng_);
Does SelectReplica() assume that every member of 'excluded' must be in 'ts_descs'?
I'm asking because the new approach to populating excluded includes a slight behaviorial change: tservers "presumed dead" are no longer filtered out because LookupTSByUUID doesn't filter out dead tservers as GetAllLiveDescriptors did.
--
To view, visit http://gerrit.cloudera.org:8080/11429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
Gerrit-Change-Number: 11429
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 05:06:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11429 )
Change subject: [catalog_manager] optimization in AsyncAddReplicaTask
......................................................................
[catalog_manager] optimization in AsyncAddReplicaTask
Introduced a minor optimization on AsyncAddReplicaTask::SendRequest()
while populating the set of blacklisted candidates for an extra replica.
With this patch, the code iterates over the UUIDs of the peers in the
committed config to populate the set of the blacklisted candidates,
rather than filtering all tablet servers using the IsRaftConfigMember()
utility function.
Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
Reviewed-on: http://gerrit.cloudera.org:8080/11429
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/master/catalog_manager.cc
1 file changed, 21 insertions(+), 12 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/11429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
Gerrit-Change-Number: 11429
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11429 )
Change subject: [catalog_manager] optimization in AsyncAddReplicaTask
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
Gerrit-Change-Number: 11429
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 12 Sep 2018 06:12:49 +0000
Gerrit-HasComments: No
[kudu-CR] [catalog manager] optimization in AsyncAddReplicaTask
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11429
to look at the new patch set (#2).
Change subject: [catalog_manager] optimization in AsyncAddReplicaTask
......................................................................
[catalog_manager] optimization in AsyncAddReplicaTask
Introduced a minor optimization on AsyncAddReplicaTask::SendRequest()
while populating the set of blacklisted candidates for an extra replica.
With this patch, the code iterates over the UUIDs of the peers in the
committed config to populate the set of the blacklisted candidates,
rather than filtering all tablet servers using the IsRaftConfigMember()
utility function.
Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
---
M src/kudu/master/catalog_manager.cc
1 file changed, 21 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/29/11429/2
--
To view, visit http://gerrit.cloudera.org:8080/11429
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1f46ef2e3e3f385f0abc7e02e521e03d47569413
Gerrit-Change-Number: 11429
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>