----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38473/#review99812 -----------------------------------------------------------
src/master/master.cpp (line 345) <https://reviews.apache.org/r/38473/#comment156781> Why are you reading LIBPROCESS_IP here? Why not `flags.ip`? There appears to be a very non-obvious global invariant and I'd like to capture the dependency here as a comment (for posterity!) as well as where we're setting LIBPROCESS_IP in the main.cpp files. src/master/master.cpp (line 349) <https://reviews.apache.org/r/38473/#comment156780> In circumstances where a user can easily avoid the master from crashing we prefer NOT to use LOG(FATAL) because it prints a stack trace which can hide the actual error message. Instead, an EXIT(EXIT_FAILURE) is a good thing to use here. Same for the LOG(FATAL) in the slave below. src/tests/cluster.hpp (line 126) <https://reviews.apache.org/r/38473/#comment156782> Thank you for the doxygen comments!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! src/tests/master_tests.cpp (line 1098) <https://reviews.apache.org/r/38473/#comment156777> This should be virtual because you're extending `MasterTest`. src/tests/master_tests.cpp (line 1121) <https://reviews.apache.org/r/38473/#comment156783> Why not just do `cluster.find`? Not sure I understand the need for this alias. src/tests/master_tests.cpp (lines 1122 - 1123) <https://reviews.apache.org/r/38473/#comment156784> What does someone running the test get from this extra output information? src/tests/master_tests.cpp (line 1126) <https://reviews.apache.org/r/38473/#comment156785> Indention. - Benjamin Hindman On Sept. 19, 2015, 1:25 a.m., Marco Massenzio wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38473/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2015, 1:25 a.m.) > > > Review request for mesos, Benjamin Hindman, Cody Maloney, and Neil Conway. > > > Bugs: MESOS-3457 > https://issues.apache.org/jira/browse/MESOS-3457 > > > Repository: mesos > > > Description > ------- > > Under certain circumstances, dynamic lookup of hostname, while > successful, provides undesirable results; we would also like, in > those circumstances, be able to just set the hostname to the chosen > IP address (possibly set via the --ip_discovery_command method). > > The flag we add here, --[no]-hostname_lookup is `true` by default > (which is the existing behavior) and will work under most > circumstances: however, by disabling lookup (using, for example, > --no_hostname_lookup) the hostname used will be the same as the > one configured (possibly, via --ip or MESOS_IP) in `LIBPROCESS_IP`. > > This change affects both Master/Agent nodes. > > WARNING - the `Address::hostname()` method always does a dynamic > hostname lookup, which may in fact return inconsistent results > wrt the Master's configuration (this is *not* affected by > this change) and should be avoided; use instead > `Master::info()::hostname()` which is always consistent with > the Master's configuration. > > > Diffs > ----- > > docs/configuration.md dd7f4aa806a3c1a8653a0fda9a326a3707308e7c > src/master/flags.hpp e4b1df3f5a33049defff4688463274067f1f1ebf > src/master/flags.cpp 80879611fbcfd764c9fc8f60a31613a9c8fc2364 > src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b > src/slave/flags.hpp e31a4183170c3442ac4a15365c229391e7e91480 > src/slave/flags.cpp add4196dfd06c0f602ff5ebd39960dc05c4cd11f > src/slave/slave.cpp ad710d7b930a2f115d503ceb8f8fd7421ad30287 > src/tests/cluster.hpp 114583de8c867495a2b7a953e6826708838e5d23 > src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a > src/tests/mesos.hpp 3db97aca921c9216d90384e1eb17030849516454 > > Diff: https://reviews.apache.org/r/38473/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Marco Massenzio > >
