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

Change subject: [master] KUDU-3182 Allow single master to specify 
--master_addresses
......................................................................


Patch Set 3:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/16340/3/src/kudu/master/sys_catalog.cc@251
PS3, Line 251: Current
nit: do you expect this to change in the future? If not, can you remove 
"Current"? Same below.

It leaves me with the line of questioning, "If that was 'current' at time of 
writing, was it expected to change in the future? If so, did it change?"


http://gerrit.cloudera.org:8080/#/c/16340/3/src/kudu/master/sys_catalog.cc@260
PS3, Line 260:    if (peer.has_last_known_addr()) {
             :         // Verify the supplied master address matches with the 
on-disk Raft config.
             :         auto raft_master_addr = 
HostPortFromPB(peer.last_known_addr());
             :         if (raft_master_addr != master_addr) {
             :           return Status::InvalidArgument(
             :               Substitute("Single master Raft config error. 
On-disk master: $0 and "
             :                          "supplied master: $1 are different", 
raft_master_addr.ToString(),
             :                          master_addr.ToString()));
             :         }
Isn't it possible that of the on-disk peers, the first one isn't the one that 
matches this master's address?


http://gerrit.cloudera.org:8080/#/c/16340/3/src/kudu/master/sys_catalog.cc@270
PS3, Line 270:  // Set the Raft config for single master configuration.
             :         *config.mutable_peers(0)->mutable_last_known_addr() = 
HostPortToPB(master_addr);
Likewise, should this be setting the size of peers to 1?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4fe1bcd217d68f66db72c321397d596cba4224be
Gerrit-Change-Number: 16340
Gerrit-PatchSet: 3
Gerrit-Owner: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 28 Aug 2020 23:35:50 +0000
Gerrit-HasComments: Yes

Reply via email to