You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/09/02 21:33:27 UTC

Re: Review Request 37623: Maintenance Primitives: Prevent Slaves from registering if the machine is under maintenance.

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

(Updated Sept. 2, 2015, 7:33 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
  src/tests/master_maintenance_tests.cpp fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 37623: Maintenance Primitives: Prevent Slaves from registering if the machine is under maintenance.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37623/#review97528
-----------------------------------------------------------


Bad patch!

Reviews applied: [37655, 36321, 36571, 37314, 37900, 37325, 37358, 37362, 37364, 37170, 37172, 37173, 37175, 37176, 37177]

Failed command: ./support/apply-review.sh -n -r 37177

Error:
 2015-09-02 20:39:49 URL:https://reviews.apache.org/r/37177/diff/raw/ [36178/36178] -> "37177.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.hpp:175
error: src/master/allocator/mesos/hierarchical.hpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 2, 2015, 7:33 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37623/
> -----------------------------------------------------------
> 
> (Updated Sept. 2, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
>   src/tests/master_maintenance_tests.cpp fb8dca3757a9565d5eb5a69eed10aa34602bb15c 
> 
> Diff: https://reviews.apache.org/r/37623/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 37623: Maintenance Primitives: Prevent Slaves from registering if the machine is under maintenance.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37623/#review98808
-----------------------------------------------------------

Ship it!



include/mesos/type_utils.hpp (line 326)
<https://reviews.apache.org/r/37623/#comment155406>

    Maybe put the IP in parens rather than after a comma? And what if one is missing? Do you want to print just the IP with no parens if no hostname and just the hostname if no IP?



src/master/master.cpp (line 3755)
<https://reviews.apache.org/r/37623/#comment155401>

    s/Down/DOWN/



src/master/master.cpp (line 3760)
<https://reviews.apache.org/r/37623/#comment155404>

    Rather than say the machine is "under maintenance", can we just specify that it's down? I can see an operator bringing the machine down having nothing to do with maintenance and this log message throwing somebody off track (this is in fact one of the reasons we decided to just make the 'machine' endpoints be independent of 'maintenance' IIRC). Same comment applies below too.



src/master/master.cpp (line 3763)
<https://reviews.apache.org/r/37623/#comment155402>

    s/Down/DOWN/



src/master/master.cpp (line 3944)
<https://reviews.apache.org/r/37623/#comment155403>

    s/Down/DOWN/



src/tests/master_maintenance_tests.cpp (line 689)
<https://reviews.apache.org/r/37623/#comment155405>

    Are you waiting for the same reason as above? I'd add more to the comment here as you did above as well.


- Benjamin Hindman


On Sept. 13, 2015, 8:34 p.m., Joris Van Remoortere wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37623/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.
> 
> 
> Bugs: MESOS-1474
>     https://issues.apache.org/jira/browse/MESOS-1474
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
>   src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574 
>   src/tests/master_maintenance_tests.cpp fb8dca3757a9565d5eb5a69eed10aa34602bb15c 
> 
> Diff: https://reviews.apache.org/r/37623/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>


Re: Review Request 37623: Maintenance Primitives: Prevent Slaves from registering if the machine is under maintenance.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37623/
-----------------------------------------------------------

(Updated Sept. 14, 2015, 1:26 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


Changes
-------

fixed issues.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
  include/mesos/v1/mesos.hpp 0d695f36558fdc390461a8767a0c9db8e321a07a 
  src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574 
  src/tests/master_maintenance_tests.cpp fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
-------


Thanks,

Joris Van Remoortere


Re: Review Request 37623: Maintenance Primitives: Prevent Slaves from registering if the machine is under maintenance.

Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37623/
-----------------------------------------------------------

(Updated Sept. 13, 2015, 8:34 p.m.)


Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Joseph Wu.


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


Repository: mesos


Description
-------

See summary.


Diffs (updated)
-----

  include/mesos/type_utils.hpp 4fb0037a224cab7ebeb6644b5274227fedb172c8 
  src/master/master.cpp c90311fa2152810e7604a0a2dee630bd14929574 
  src/tests/master_maintenance_tests.cpp fb8dca3757a9565d5eb5a69eed10aa34602bb15c 

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


Testing
-------


Thanks,

Joris Van Remoortere