[kudu-CR] KUDU-2170 Masters can start with duplicates specified

[Andrew Wong (Code Review)]

Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8341 )

Change subject: KUDU-2170 Masters can start with duplicates specified
..


Patch Set 2: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
Gerrit-Change-Number: 8341
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 19 Oct 2017 23:48:28 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2170 Masters can start with duplicates specified

[Adar Dembo (Code Review)]

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

Change subject: KUDU-2170 Masters can start with duplicates specified
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
Gerrit-Change-Number: 8341
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 19 Oct 2017 23:11:34 +
Gerrit-HasComments: No

[kudu-CR] KUDU-2170 Masters can start with duplicates specified

[Will Berkeley (Code Review)]

Will Berkeley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8341 )

Change subject: KUDU-2170 Masters can start with duplicates specified
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.h@a23
PS1, Line 23:
> There is a std::string reference here.
Done


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.cc
File src/kudu/master/master.cc:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.cc@311
PS1, Line 311:   auto uuid_cmp = [](ServerEntryPB left, ServerEntryPB right) {
> Shouldn't this comparator expect its args by cref?
Done


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.cc@314
PS1, Line 314:   std::set 
masters_by_uuid(uuid_cmp);
> Add a comment explaining why we're deduplicating.
Done


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.h@a23
PS1, Line 23:
> Odd, this header definitely has std::string references.
Sorry, I'm trying to guess the correct IWYU fixes for "real life" based off 
running it on macOS. I must've forgot to rebuild after I changed this.


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.cc@202
PS1, Line 202:"the unique set of addresses 
is $0",
> Nit: indentation.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
Gerrit-Change-Number: 8341
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley 
Gerrit-Comment-Date: Thu, 19 Oct 2017 22:57:23 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2170 Masters can start with duplicates specified

[Will Berkeley (Code Review)]

Hello Kudu Jenkins, Adar Dembo,

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

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

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

Change subject: KUDU-2170 Masters can start with duplicates specified
..

KUDU-2170 Masters can start with duplicates specified

Users can specify duplicate masters in --master_addresses, where a
duplicate includes duplicated names and the same server known by
different names. This doesn't cause problems as far as I know, but it
looks scary because, for example, the web ui shows extra masters listed in
/masters. This is because /masters shows one master per result from
ListMasters. This patch deduplicates the output of ListMasters by UUID.

It also warns on start if there are exact string duplicates in
--master_addresses, at the time the master compares on-disk and
configured master addresses, but before the masters are ready to
resolve UUIDs.

Also, the error message when on-disk and configured masters don't match
is confusing. This patch makes it easy to understand.

Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
---
M src/kudu/master/master.cc
M src/kudu/master/sys_catalog.cc
2 files changed, 24 insertions(+), 3 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
Gerrit-Change-Number: 8341
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2170 Masters can start with duplicates specified

[Adar Dembo (Code Review)]

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

Change subject: KUDU-2170 Masters can start with duplicates specified
..


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.h@a23
PS1, Line 23:
There is a std::string reference here.


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.cc
File src/kudu/master/master.cc:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.cc@311
PS1, Line 311:   auto uuid_cmp = [](ServerEntryPB left, ServerEntryPB right) {
Shouldn't this comparator expect its args by cref?


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/master.cc@314
PS1, Line 314:   std::set 
masters_by_uuid(uuid_cmp);
Add a comment explaining why we're deduplicating.


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.h
File src/kudu/master/sys_catalog.h:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.h@a23
PS1, Line 23:
Odd, this header definitely has std::string references.


http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/8341/1/src/kudu/master/sys_catalog.cc@202
PS1, Line 202:"the unique set of addresses 
is $0",
Nit: indentation.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
Gerrit-Change-Number: 8341
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Thu, 19 Oct 2017 22:39:31 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2170 Masters can start with duplicates specified

[Will Berkeley (Code Review)]

Will Berkeley has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8341


Change subject: KUDU-2170 Masters can start with duplicates specified
..

KUDU-2170 Masters can start with duplicates specified

Users can specify duplicate masters in --master_addresses, where a
duplicate includes duplicated names and the same server known by
different names. This doesn't cause problems as far as I know, but it
looks scary because, for example, the web ui shows extra masters listed in
/masters. This is because /masters shows one master per result from
ListMasters. This patch deduplicates the output of ListMasters by UUID.

It also warns on start if there are exact string duplicates in
--master_addresses, at the time the master compares on-disk and
configured master addresses, but before the masters are ready to
resolve UUIDs.

Also, the error message when on-disk and configured masters don't match
is confusing. This patch makes it easy to understand.

Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
---
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
4 files changed, 22 insertions(+), 5 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5047ccf92dde5377da18a1a84e105a951fa5b48c
Gerrit-Change-Number: 8341
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley