You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2016/09/16 00:51:59 UTC

[kudu-CR] csd: also use master address for --rpc bind address

Hello Todd Lipcon,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: csd: also use master_address for --rpc_bind_address
......................................................................

csd: also use master_address for --rpc_bind_address

If set, master_address is an alias of some sort for the master, used in the
distributed master Raft configuration as well as in the tserver's
--tserver_master_addrs gflag. Thus, it should also be used as the master's
--rpc_bind_address gflag.

Note: I'm not sure whether this change makes sense. For one,
webserver_interface is still its own parameter; maybe it should be removed
and master_address used there too? For two, how will all of this interact
with multi-homed machines, or machines where different interfaces are used
for different purposes?

Change-Id: Ibc9f0be27b134bcfd3625f0cd2ba2bb85928f8d4
---
M java/kudu-csd/src/descriptor/service.sdl
M java/kudu-csd/src/scripts/kudu.sh
2 files changed, 9 insertions(+), 2 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibc9f0be27b134bcfd3625f0cd2ba2bb85928f8d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] csd: also use master address for --rpc bind address

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has abandoned this change.

Change subject: csd: also use master_address for --rpc_bind_address
......................................................................


Abandoned

Todd and I discussed this patch in person.

We agreed that it doesn't really make sense for the reason he pointed out in the patch, and it'd be better to tackle issues like these (i.e. multi-homing and interface naming) comprehensively, with a clear set of goals in mind instead of ad-hoc the way I have.

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Ibc9f0be27b134bcfd3625f0cd2ba2bb85928f8d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] csd: also use master address for --rpc bind address

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: csd: also use master_address for --rpc_bind_address
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4431/1/java/kudu-csd/src/scripts/kudu.sh
File java/kudu-csd/src/scripts/kudu.sh:

Line 110:     RPC_BIND_ADDRESSES="--rpc_bind_addresses=$MASTER_ADDRESS"
not 100% sure this is always a good idea, either. For one, this means it won't listen on localhost anymore, which is potentially annoying when trying to run CLI tools from the master machine, right?


Line 126:     --tserver_master_addrs="$MASTER_IPS" \
a little odd we have MASTER_ADDRESSES, MASTER_ADDRESS, and MASTER_IP variables all accessed by this script. Worth renaming some for clarity?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibc9f0be27b134bcfd3625f0cd2ba2bb85928f8d4
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes