Re: Review Request 51953: Changed registrar operations to no longer depend on "strict" flag.

[Neil Conway]

> On Sept. 16, 2016, 7:01 p.m., Vinod Kone wrote:
> > src/master/master.hpp, line 1935
> > 
> >
> > looks like our style is to add a comment about the unused parameter.
> > 
> > https://github.com/apache/mesos/blob/master/src/slave/slave.hpp#L487

We remove this parameter in the next review in the chain (and in any case we 
don't follow this style guideline consistently), so I think this is okay as-is.


- Neil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51953/#review149241
---


On Sept. 16, 2016, 7:15 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51953/
> ---
> 
> (Updated Sept. 16, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5951
> https://issues.apache.org/jira/browse/MESOS-5951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Registrar operations to admit and remove slaves now behave the same way,
> regardless of the value of the "strict" flag.
> 
> This commit changes the behavior of these operations by enabling the
> previous "strict" behavior. For example, removing a slave that is not
> registered results in an `Error`, rather than silently ignoring the
> problem. However, the master should never attempt to remove a slave that
> is not registered, so this is a useful invariant to check. Similarly,
> attempting to admit a slave that is already admitted will now result in
> an `Error`; this should only occur when there is a slave ID collision,
> so again it seems appropriate to produce an error rather than ignoring
> the problem.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4992ab0a0bb5babbf6a4fa3e6eff3577590fc879 
>   src/master/master.cpp b88472f6350d3f71e057bab34822423da5427151 
>   src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 
> 
> Diff: https://reviews.apache.org/r/51953/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 51953: Changed registrar operations to no longer depend on "strict" flag.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51953/
---

(Updated Sept. 16, 2016, 7:15 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Tweaked comments.


Bugs: MESOS-5951
https://issues.apache.org/jira/browse/MESOS-5951


Repository: mesos


Description
---

Registrar operations to admit and remove slaves now behave the same way,
regardless of the value of the "strict" flag.

This commit changes the behavior of these operations by enabling the
previous "strict" behavior. For example, removing a slave that is not
registered results in an `Error`, rather than silently ignoring the
problem. However, the master should never attempt to remove a slave that
is not registered, so this is a useful invariant to check. Similarly,
attempting to admit a slave that is already admitted will now result in
an `Error`; this should only occur when there is a slave ID collision,
so again it seems appropriate to produce an error rather than ignoring
the problem.


Diffs (updated)
-

  src/master/master.hpp 4992ab0a0bb5babbf6a4fa3e6eff3577590fc879 
  src/master/master.cpp b88472f6350d3f71e057bab34822423da5427151 
  src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 

Diff: https://reviews.apache.org/r/51953/diff/


Testing
---

`make check` on OSX and Linux.


Thanks,

Neil Conway

Re: Review Request 51953: Changed registrar operations to no longer depend on "strict" flag.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51953/#review149241
---




src/master/master.hpp (line 1935)


looks like our style is to add a comment about the unused parameter.

https://github.com/apache/mesos/blob/master/src/slave/slave.hpp#L487


- Vinod Kone


On Sept. 16, 2016, 12:58 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51953/
> ---
> 
> (Updated Sept. 16, 2016, 12:58 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5951
> https://issues.apache.org/jira/browse/MESOS-5951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Registrar operations to admit and remove slaves now behave the same way,
> regardless of the value of the "strict" flag.
> 
> This commit changes the behavior of these operations by enabling the
> previous "strict" behavior. For example, removing a slave that is not
> registered results in an `Error`, rather than silently ignoring the
> problem. However, the master should never attempt to remove a slave that
> is not registered, so this is a useful invariant to check. Similarly,
> attempting to admit a slave that is already admitted will now result in
> an `Error`; this should only occur when there is a slave ID collision,
> so again it seems appropriate to produce an error rather than ignoring
> the problem.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4992ab0a0bb5babbf6a4fa3e6eff3577590fc879 
>   src/master/master.cpp b88472f6350d3f71e057bab34822423da5427151 
>   src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 
> 
> Diff: https://reviews.apache.org/r/51953/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 51953: Changed registrar operations to no longer depend on "strict" flag.

[Vinod Kone]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51953/#review149232
---


Fix it, then Ship it!





src/master/master.hpp (line 2136)


s/registered/admitted/



src/master/master.cpp (line 7376)


s/registered/admitted/


- Vinod Kone


On Sept. 16, 2016, 12:58 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51953/
> ---
> 
> (Updated Sept. 16, 2016, 12:58 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5951
> https://issues.apache.org/jira/browse/MESOS-5951
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Registrar operations to admit and remove slaves now behave the same way,
> regardless of the value of the "strict" flag.
> 
> This commit changes the behavior of these operations by enabling the
> previous "strict" behavior. For example, removing a slave that is not
> registered results in an `Error`, rather than silently ignoring the
> problem. However, the master should never attempt to remove a slave that
> is not registered, so this is a useful invariant to check. Similarly,
> attempting to admit a slave that is already admitted will now result in
> an `Error`; this should only occur when there is a slave ID collision,
> so again it seems appropriate to produce an error rather than ignoring
> the problem.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 4992ab0a0bb5babbf6a4fa3e6eff3577590fc879 
>   src/master/master.cpp b88472f6350d3f71e057bab34822423da5427151 
>   src/tests/registrar_tests.cpp b04fc92c66449f1094212a534899e16d5f6f01bd 
> 
> Diff: https://reviews.apache.org/r/51953/diff/
> 
> 
> Testing
> ---
> 
> `make check` on OSX and Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>