Todd Lipcon has posted comments on this change. Change subject: KUDU-1776: Fix "kudu remote_replica copy" connecting to wildcard address ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/5378/2/src/kudu/integration-tests/external_mini_cluster.cc File src/kudu/integration-tests/external_mini_cluster.cc: PS2, Line 773: // Retain the wildcard addr of "0.0.0.0" if rpc host matches : // that of current host. This is needed to restart the daemons : // using wildcard addresses again. I don't follow this logic. Why are you checking the FQDN instead of just checking whether the original specified bind addr was 0.0.0.0? eg: if (bind_host_ != "0.0.0.0") { bound_rpc_ = bound_rpc_hostport(); } else { bound_rpc_.set_port(bound_rpc_hostport().port()); } (not sure those are exactly the right methods but you get the point) PS2, Line 1055: ("--rpc_bind_addresses=$0:$1", : bound_rpc_.host(), bound_rpc_.port()) what effect does this have? isn't it equivalent? Line 1061: bound_http_.host())); was this change necessary? http://gerrit.cloudera.org:8080/#/c/5378/2/src/kudu/integration-tests/external_mini_cluster.h File src/kudu/integration-tests/external_mini_cluster.h: PS2, Line 126: bind to hostname not sure what 'bind to hostname' means. This should also specify whether it affects RPC or webserver or both. Also please specify which takes precedence between this and 'bind_to_unique_loopback_addresses' above. Perhaps it would be better to change the latter boolean to be an enum like BindMode { LOOPBACK, UNIQUE_LOOPBACK, WILDCARD }? -- To view, visit http://gerrit.cloudera.org:8080/5378 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie5d0a37b39a3774caab5b5d8dba3d9750bf5f21f Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
