You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2013/11/04 21:24:37 UTC
Re: Review Request 14847: Added --hostname flag to slave and removed
webui_hostname.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14847/#review28134
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/14847/#comment54741>
Sorry didn't review this earlier but there are couple of issues with this.
SlaveInfo is checkpointed, so changing the proto doesn't necessarily guarantee that what's already on disk will change. Infact the slave constructs its SlaveInfo from the checkpointed SlaveInfo. In this particular case it is probably safe because it will just ignore the webui_hostname and webui_port fields.
Removing a required field is almost always a bad idea? In this case an old executor/master cannot work with a new slave that is using the updated SlaveInfo because the required field webui_hostname will be missing.
- Vinod Kone
On Oct. 24, 2013, 3:26 a.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14847/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2013, 3:26 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> A slave can be started with --hostname which explicitly sets hostname instead of usual system hostname.
> This is necessary in situations where system hostname resolves to internal names which cannot be accessed from the web ui.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto fe1d82b
> src/common/type_utils.hpp c48411f
> src/slave/flags.hpp db777e3
> src/slave/slave.cpp debb2f4
> src/tests/state_tests.cpp f39dee5
>
> Diff: https://reviews.apache.org/r/14847/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Niklas Nielsen
>
>
Re: Review Request 14847: Added --hostname flag to slave and removed
webui_hostname.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On Nov. 4, 2013, 8:24 p.m., Vinod Kone wrote:
> > include/mesos/mesos.proto, lines 179-180
> > <https://reviews.apache.org/r/14847/diff/2/?file=370571#file370571line179>
> >
> > Sorry didn't review this earlier but there are couple of issues with this.
> >
> > SlaveInfo is checkpointed, so changing the proto doesn't necessarily guarantee that what's already on disk will change. Infact the slave constructs its SlaveInfo from the checkpointed SlaveInfo. In this particular case it is probably safe because it will just ignore the webui_hostname and webui_port fields.
> >
> > Removing a required field is almost always a bad idea? In this case an old executor/master cannot work with a new slave that is using the updated SlaveInfo because the required field webui_hostname will be missing.
I'm missing the incompatibility issues. The 'webui_hostname' and 'webui_port' fields have been deprecated for a while now and except for in a few tests and in Slave::initialize they were not being used (including in the master). A quick inspection shows that the last version to use webui_hostname was 0.10, which for a volatile project like Mesos is sufficient deprecation time (we've deprecated other things after just one release!).
Am I missing something else?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14847/#review28134
-----------------------------------------------------------
On Oct. 24, 2013, 3:26 a.m., Niklas Nielsen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14847/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2013, 3:26 a.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> A slave can be started with --hostname which explicitly sets hostname instead of usual system hostname.
> This is necessary in situations where system hostname resolves to internal names which cannot be accessed from the web ui.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto fe1d82b
> src/common/type_utils.hpp c48411f
> src/slave/flags.hpp db777e3
> src/slave/slave.cpp debb2f4
> src/tests/state_tests.cpp f39dee5
>
> Diff: https://reviews.apache.org/r/14847/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Niklas Nielsen
>
>