You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2015/08/24 20:33:59 UTC

Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

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

(Updated Aug. 24, 2015, 11:33 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rebase and update diff.  No other changes.


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


Repository: mesos


Description
-------

* MachineInfo - Describes a single box that holds one or more agents.
* MachineInfos - A list of boxes.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.
* maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
* Registry::MaintenanceStatus - Holds the maintenance mode of a machine.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
  src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 26, 2015, 10:44 a.m., Joris Van Remoortere wrote:
> > src/master/registry.proto, line 45
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048534#file1048534line45>
> >
> >     Should we call this `info` or rename the type to `MachineId`?

I think I can rename this to `machine_info` (to match how we name it elsewhere) with minimal impact on your reviews.


- Joseph


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


On Aug. 24, 2015, 11:33 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 11:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

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



src/master/registry.proto (line 45)
<https://reviews.apache.org/r/36571/#comment152076>

    Should we call this `info` or rename the type to `MachineId`?


- Joris Van Remoortere


On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 28, 2015, 8:12 a.m., Joris Van Remoortere wrote:
> > include/mesos/mesos.proto, line 160
> > <https://reviews.apache.org/r/36571/diff/19/?file=1057056#file1057056line160>
> >
> >     `a slave` or `an agent` ?

Since I added Machine* to the v1 API, I just used "slave" in one, and "agent" in the other.


> On Aug. 28, 2015, 8:12 a.m., Joris Van Remoortere wrote:
> > include/mesos/mesos.proto, line 162
> > <https://reviews.apache.org/r/36571/diff/19/?file=1057056#file1057056line162>
> >
> >     Is `Missing fields default to the empty string for the purpose of matching` still true?
> >     Do you want to mention the hostname comparison is not case sensitive?

Not as true anymore.
And yes, I should.


> On Aug. 28, 2015, 8:12 a.m., Joris Van Remoortere wrote:
> > include/mesos/mesos.proto, lines 170-175
> > <https://reviews.apache.org/r/36571/diff/19/?file=1057056#file1057056line170>
> >
> >     Did you mention you were getting rid of the list version? Are we still going to use it?

I'll put a TODO here.  We'll want to remove it as soon as https://reviews.apache.org/r/37826/ is submitted.


> On Aug. 28, 2015, 8:12 a.m., Joris Van Remoortere wrote:
> > src/master/registry.proto, lines 55-57
> > <https://reviews.apache.org/r/36571/diff/19/?file=1057059#file1057059line55>
> >
> >     Why not follow the wrapper message pattern in the `NOTE` above:
> >     ```
> >     message Machines {
> >       repeated Machine machines = 1;
> >     }
> >     ```
> >     
> >     Can you add a comment as to why not if you leave it as is?

I was conflicted if I wanted a list accessor like `.machines().machines()`, which is what the `slaves` accessor looks like.
I'll change it to match the pattern though, since it doesn't have too big of an impact.


- Joseph


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


On Aug. 28, 2015, 10:20 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2015, 10:20 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New protobufs:
> 
> * MachineID - Describes a single box that holds one or more agents.
> * MachineIDs - A list of boxes.
> * MachineInfo - Holds a box and some extra information about its status.
> * MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto c40a09b2bb2b9444e9b90e86eefe6fc8f98b613d 
>   include/mesos/v1/mesos.proto ee15b9ae70aeb8cf803ea7e4b06f4443bd7bc9e2 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

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

Ship it!



include/mesos/maintenance/maintenance.proto (line 56)
<https://reviews.apache.org/r/36571/#comment152552>

    s/Period/Interval/



include/mesos/mesos.proto (line 160)
<https://reviews.apache.org/r/36571/#comment152549>

    `a slave` or `an agent` ?



include/mesos/mesos.proto (line 162)
<https://reviews.apache.org/r/36571/#comment152550>

    Is `Missing fields default to the empty string for the purpose of matching` still true?
    Do you want to mention the hostname comparison is not case sensitive?



include/mesos/mesos.proto (lines 170 - 175)
<https://reviews.apache.org/r/36571/#comment152551>

    Did you mention you were getting rid of the list version? Are we still going to use it?



include/mesos/v1/mesos.proto (line 162)
<https://reviews.apache.org/r/36571/#comment152554>

    don't forget to update this if you change it in the old proto.



include/mesos/v1/mesos.proto (lines 170 - 175)
<https://reviews.apache.org/r/36571/#comment152555>

    in case you decide to remove this in the old proto.



src/master/registry.proto (lines 55 - 57)
<https://reviews.apache.org/r/36571/#comment152556>

    Why not follow the wrapper message pattern in the `NOTE` above:
    ```
    message Machines {
      repeated Machine machines = 1;
    }
    ```
    
    Can you add a comment as to why not if you leave it as is?


- Joris Van Remoortere


On Aug. 28, 2015, 12:57 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2015, 12:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New protobufs:
> 
> * MachineID - Describes a single box that holds one or more agents.
> * MachineIDs - A list of boxes.
> * MachineInfo - Holds a box and some extra information about its status.
> * MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 28, 2015, 10:25 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Fix a small, but important typo.


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


Repository: mesos


Description
-------

New protobufs:

* MachineID - Describes a single box that holds one or more agents.
* MachineIDs - A list of boxes.
* MachineInfo - Holds a box and some extra information about its status.
* MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto c40a09b2bb2b9444e9b90e86eefe6fc8f98b613d 
  include/mesos/v1/mesos.proto ee15b9ae70aeb8cf803ea7e4b06f4443bd7bc9e2 
  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 28, 2015, 10:20 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Comments, some tweaks for naming, and a rebase.


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


Repository: mesos


Description
-------

New protobufs:

* MachineID - Describes a single box that holds one or more agents.
* MachineIDs - A list of boxes.
* MachineInfo - Holds a box and some extra information about its status.
* MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto c40a09b2bb2b9444e9b90e86eefe6fc8f98b613d 
  include/mesos/v1/mesos.proto ee15b9ae70aeb8cf803ea7e4b06f4443bd7bc9e2 
  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 27, 2015, 5:57 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


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


Repository: mesos


Description (updated)
-------

New protobufs:

* MachineID - Describes a single box that holds one or more agents.
* MachineIDs - A list of boxes.
* MachineInfo - Holds a box and some extra information about its status.
* MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.


Diffs
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
  include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 27, 2015, 5:57 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rename `MachineAddress` to `MachineID`.  Tweak some comments.


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


Repository: mesos


Description
-------

New protobufs:

* MachineAddress - Describes a single box that holds one or more agents.
* MachineAddresses - A list of boxes.
* MachineInfo - Holds a box and some extra information about its status.
* MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
  include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 27, 2015, 4:58 p.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, lines 37-39
> > <https://reviews.apache.org/r/36571/diff/18/?file=1056922#file1056922line37>
> >
> >     Let's extend one of the windows to multiple lines to indicate that a single window can have multiple machines.

I was thinking the `+` would be appear as part of a bar.  So each window in the picture should have 3 machines.
I can change it from a bar to a rectangle.


> On Aug. 27, 2015, 4:58 p.m., Alexander Rukletsov wrote:
> > src/Makefile.am, lines 156-157
> > <https://reviews.apache.org/r/36571/diff/18/?file=1056925#file1056925line156>
> >
> >     Don't we need to add `v1/**` protos somewhere as well?

The `v1/mesos.proto` should have already been added.  We don't add a `v1/maintenance.proto`


> On Aug. 27, 2015, 4:58 p.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, line 164
> > <https://reviews.apache.org/r/36571/diff/18/?file=1056923#file1056923line164>
> >
> >     Let's add `Info` suffix for clarity.

I'll rename it to `MachineID`.


- Joseph


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


On Aug. 27, 2015, 5:57 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 5:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New protobufs:
> 
> * MachineAddress - Describes a single box that holds one or more agents.
> * MachineAddresses - A list of boxes.
> * MachineInfo - Holds a box and some extra information about its status.
> * MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/#review96796
-----------------------------------------------------------



include/mesos/maintenance/maintenance.proto (lines 37 - 39)
<https://reviews.apache.org/r/36571/#comment152447>

    Let's extend one of the windows to multiple lines to indicate that a single window can have multiple machines.



include/mesos/mesos.proto (line 164)
<https://reviews.apache.org/r/36571/#comment152446>

    Let's add `Info` suffix for clarity.



src/Makefile.am (lines 156 - 157)
<https://reviews.apache.org/r/36571/#comment152450>

    Don't we need to add `v1/**` protos somewhere as well?


- Alexander Rukletsov


On Aug. 27, 2015, 11:41 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New protobufs:
> 
> * MachineAddress - Describes a single box that holds one or more agents.
> * MachineAddresses - A list of boxes.
> * MachineInfo - Holds a box and some extra information about its status.
> * MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 27, 2015, 4:41 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Reworked and rearranged a lot of the protobufs, according to comments.  Added some ascii art for the maintenance schedule.


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


Repository: mesos


Description (updated)
-------

New protobufs:

* MachineAddress - Describes a single box that holds one or more agents.
* MachineAddresses - A list of boxes.
* MachineInfo - Holds a box and some extra information about its status.
* MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
  include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

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

Ship it!



include/mesos/maintenance/maintenance.proto (line 54)
<https://reviews.apache.org/r/36571/#comment152384>

    How about we move Mode into MachineInfo so that we can use this as a property of machines regardless of whether or not we're doing maintenance? I.e., MachineInfo::DRAINING.



include/mesos/mesos.proto (line 127)
<https://reviews.apache.org/r/36571/#comment152382>

    This stuff also needs to go into v1 please!



src/master/registry.proto (line 43)
<https://reviews.apache.org/r/36571/#comment152385>

    If we ever want to capture non-maintenance information about machines here I'd recommend that we s/MaintenanceStatus/MachineStatus/, and for now with only one field ('machine_info') that we might add to later. Meanwhile, we can move 'mode' into MachineInfo and make the current MachineInfo be a MachineID ...


- Benjamin Hindman


On Aug. 27, 2015, 7:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 27, 2015, 12:33 p.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Rebase on master (patch needed an update).


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


Repository: mesos


Description
-------

* MachineInfo - Describes a single box that holds one or more agents.
* MachineInfos - A list of boxes.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.
* maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
* Registry::MaintenanceStatus - Holds the maintenance mode of a machine.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
  src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 27, 2015, 11:58 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Address some of Alex's comments.


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


Repository: mesos


Description
-------

* MachineInfo - Describes a single box that holds one or more agents.
* MachineInfos - A list of boxes.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.
* maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
* Registry::MaintenanceStatus - Holds the maintenance mode of a machine.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
  src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 26, 2015, 11:04 p.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, line 56
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line56>
> >
> >     I'm not sure `0` is a valid protobuf tag.

After double checking the docs... It is valid for enums: https://developers.google.com/protocol-buffers/docs/proto#enum (But not valid for non-enums.)

For example: https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L434


> On Aug. 26, 2015, 11:04 p.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, line 136
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053336#file1053336line136>
> >
> >     Why do we need this message? I've checked several following patches and haven't seen any use cases. Shall I look further up review chain?

We use this later on when we add the `/maintenance/start` and `/maintenance/stop` endpoints, which both take a list of machines.
However, considering this https://reviews.apache.org/r/37826/ new review, it may not be necessary for much longer.


> On Aug. 26, 2015, 11:04 p.m., Alexander Rukletsov wrote:
> > src/Makefile.am, line 913
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053337#file1053337line913>
> >
> >     Looks like we maintain alphabetical order here.

Good catch.  I think the `EXECUTOR_PROTO` and `ISOLATOR_PROTO` threw me off :)


> On Aug. 26, 2015, 11:04 p.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, lines 31-37
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line31>
> >
> >     IIUC, here you group a bunch of machines based on time intervals. Why this grouping is necessary? We can simplify the design significantly and reduce the number of protobufs if we represent schedule by a set of maintenance events, each per machine, which may occasionally overlap in time.
> >     
> >     See my comment in https://reviews.apache.org/r/37314 with a proposal for a joined "maintenance event" message.

We should definitely discuss this further.

The motivation behind this "schedule" and "window" representation is that operators generally think in these terms too.  Since these protos are what the operators are expected to use to interact with maintenance primitives, we'd like it to match their understanding.


- Joseph


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


On Aug. 27, 2015, 11:58 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 11:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Vinod Kone <vi...@gmail.com>.

> On Aug. 27, 2015, 6:04 a.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, line 56
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line56>
> >
> >     I'm not sure `0` is a valid protobuf tag.
> 
> Joseph Wu wrote:
>     After double checking the docs... It is valid for enums: https://developers.google.com/protocol-buffers/docs/proto#enum (But not valid for non-enums.)
>     
>     For example: https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L434

I would strongly recommend to use a start value of 1 instead of 0 because it's hard to tell the difference between an enum being unset to enum being set to the first value. This is especially true if the enum is going to be used as optional. Looks like it is used as a required field for now, but who knows.


- Vinod


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


On Aug. 27, 2015, 7:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 26, 2015, 11:04 p.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, lines 31-37
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line31>
> >
> >     IIUC, here you group a bunch of machines based on time intervals. Why this grouping is necessary? We can simplify the design significantly and reduce the number of protobufs if we represent schedule by a set of maintenance events, each per machine, which may occasionally overlap in time.
> >     
> >     See my comment in https://reviews.apache.org/r/37314 with a proposal for a joined "maintenance event" message.
> 
> Joseph Wu wrote:
>     We should definitely discuss this further.
>     
>     The motivation behind this "schedule" and "window" representation is that operators generally think in these terms too.  Since these protos are what the operators are expected to use to interact with maintenance primitives, we'd like it to match their understanding.
> 
> Alexander Rukletsov wrote:
>     Absolutely, we should not surprise operators. But they do not speak protobufs, they will be sending JSON requests, which we may convert to something (protobufs) that is most convenient for us in terms of storage and retrieval.

I think a 1-to-1 protobuf-JSON symmetry is useful for documenting both JSON and protobuf at once.  We'll probably keep this how it is.  However, we'll be using your suggested "joined message".


> On Aug. 26, 2015, 11:04 p.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, line 56
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line56>
> >
> >     I'm not sure `0` is a valid protobuf tag.
> 
> Joseph Wu wrote:
>     After double checking the docs... It is valid for enums: https://developers.google.com/protocol-buffers/docs/proto#enum (But not valid for non-enums.)
>     
>     For example: https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L434
> 
> Vinod Kone wrote:
>     I would strongly recommend to use a start value of 1 instead of 0 because it's hard to tell the difference between an enum being unset to enum being set to the first value. This is especially true if the enum is going to be used as optional. Looks like it is used as a required field for now, but who knows.

Good point.  I'll increment by one.


- Joseph


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


On Aug. 27, 2015, 4:41 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 4:41 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> New protobufs:
> 
> * MachineAddress - Describes a single box that holds one or more agents.
> * MachineAddresses - A list of boxes.
> * MachineInfo - Holds a box and some extra information about its status.
> * MachineInfo::Mode - An enum for the three states of a machine: UP, DRAINING, DOWN.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   include/mesos/v1/mesos.proto 382b978dca769757171c5558b7f259870592c321 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Alexander Rukletsov <ru...@gmail.com>.

> On Aug. 27, 2015, 6:04 a.m., Alexander Rukletsov wrote:
> > include/mesos/maintenance/maintenance.proto, lines 31-37
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053335#file1053335line31>
> >
> >     IIUC, here you group a bunch of machines based on time intervals. Why this grouping is necessary? We can simplify the design significantly and reduce the number of protobufs if we represent schedule by a set of maintenance events, each per machine, which may occasionally overlap in time.
> >     
> >     See my comment in https://reviews.apache.org/r/37314 with a proposal for a joined "maintenance event" message.
> 
> Joseph Wu wrote:
>     We should definitely discuss this further.
>     
>     The motivation behind this "schedule" and "window" representation is that operators generally think in these terms too.  Since these protos are what the operators are expected to use to interact with maintenance primitives, we'd like it to match their understanding.

Absolutely, we should not surprise operators. But they do not speak protobufs, they will be sending JSON requests, which we may convert to something (protobufs) that is most convenient for us in terms of storage and retrieval.


> On Aug. 27, 2015, 6:04 a.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, line 136
> > <https://reviews.apache.org/r/36571/diff/15/?file=1053336#file1053336line136>
> >
> >     Why do we need this message? I've checked several following patches and haven't seen any use cases. Shall I look further up review chain?
> 
> Joseph Wu wrote:
>     We use this later on when we add the `/maintenance/start` and `/maintenance/stop` endpoints, which both take a list of machines.
>     However, considering this https://reviews.apache.org/r/37826/ new review, it may not be necessary for much longer.

Yeah, I would say the API should not much influence storage decisions. As long as we do not use it for storage, let's avoid it in protobufs.


- Alexander


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


On Aug. 27, 2015, 7:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2015, 7:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 715b8cf38e1e56c18a3f2ddbb82c920bd9414f05 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/#review96642
-----------------------------------------------------------



include/mesos/maintenance/maintenance.proto (lines 31 - 37)
<https://reviews.apache.org/r/36571/#comment152258>

    IIUC, here you group a bunch of machines based on time intervals. Why this grouping is necessary? We can simplify the design significantly and reduce the number of protobufs if we represent schedule by a set of maintenance events, each per machine, which may occasionally overlap in time.
    
    See my comment in https://reviews.apache.org/r/37314 with a proposal for a joined "maintenance event" message.



include/mesos/maintenance/maintenance.proto (line 56)
<https://reviews.apache.org/r/36571/#comment152241>

    I'm not sure `0` is a valid protobuf tag.



include/mesos/mesos.proto (line 123)
<https://reviews.apache.org/r/36571/#comment152265>

    Minor nit: capitalize NOTE.



include/mesos/mesos.proto (line 136)
<https://reviews.apache.org/r/36571/#comment152238>

    Why do we need this message? I've checked several following patches and haven't seen any use cases. Shall I look further up review chain?



src/Makefile.am (line 913)
<https://reviews.apache.org/r/36571/#comment152237>

    Looks like we maintain alphabetical order here.



src/master/registry.proto (line 45)
<https://reviews.apache.org/r/36571/#comment152239>

    Suffix `Info` is used mostly to avoid collisions with non-proto classes, therefore feel free to omit `_info` in the name here.


- Alexander Rukletsov


On Aug. 26, 2015, 6:35 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 26, 2015, 6:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/
-----------------------------------------------------------

(Updated Aug. 26, 2015, 11:35 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.


Changes
-------

Comment changes + a rename of a field in the Registry::MaintenanceStatus object.


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


Repository: mesos


Description
-------

* MachineInfo - Describes a single box that holds one or more agents.
* MachineInfos - A list of boxes.
* maintenance::Window - A set of machines and a planned downtime period.
* maintenance::Schedule - A set of maintenance windows.
* maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
* Registry::MaintenanceStatus - Holds the maintenance mode of a machine.


Diffs (updated)
-----

  include/mesos/maintenance/maintenance.hpp PRE-CREATION 
  include/mesos/maintenance/maintenance.proto PRE-CREATION 
  include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
  src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
  src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 

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


Testing
-------

`make check`


Thanks,

Joseph Wu


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 26, 2015, 9:57 a.m., Joris Van Remoortere wrote:
> > include/mesos/maintenance/maintenance.proto, line 33
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048531#file1048531line33>
> >
> >     Why the choice of repeated `MachineInfo` over `MachineInfos`?

Mostly because of how this is created/accessed.  If I used `MachineInfos`, we'd end up with `.machine_infos().machine_infos()`, instead of just `.machine_infos()`.  And I don't think there will be any extension of MachineInfos with extra fields.


> On Aug. 26, 2015, 9:57 a.m., Joris Van Remoortere wrote:
> > include/mesos/maintenance/maintenance.proto, line 29
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048531#file1048531line29>
> >
> >     is `interval` referring to the old proto here?
> >     Should we say `A set of machines scheduled to go into maintenance during the same unavailability.`

Yes and no.  In my head, unavailability, interval, period, range of time, etc are all sort of synonyms.

I'll reword anyway to be more clear.


> On Aug. 26, 2015, 9:57 a.m., Joris Van Remoortere wrote:
> > src/Makefile.am, line 428
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048533#file1048533line428>
> >
> >     Is this just a whitespace change?
> >     Want to make sure I'm not missing anything here.

Yes.  Apparently, that line used spaces instead of tabs, contrary to everything else in the file.


> On Aug. 26, 2015, 9:57 a.m., Joris Van Remoortere wrote:
> > src/master/registry.proto, line 25
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048534#file1048534line25>
> >
> >     s/master/top level/ to avoid any confusion? Or did you really mean `master` as in the entity?

Top level sounds better.  I think `master` was used initially because the registry persists on nodes related to the master...


- Joseph


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


On Aug. 24, 2015, 11:33 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 11:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 26, 2015, 9:57 a.m., Joris Van Remoortere wrote:
> > A higher level wording question: Do we want to be consistent with `Mode` vs `Status`, or did you explicitly use different words? For example, we could change `MaintenanceStatus` to `MaintenanceModes`. I'm not necessarily saying we should, just asking if this was done for a specific reason.

I named it `status` because we might be storing more than just the mode in the future.  (Not for the MVP, but there were considerations of including data like the accept/decline reason from frameworks and such.)


- Joseph


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


On Aug. 24, 2015, 11:33 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 11:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

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


A higher level wording question: Do we want to be consistent with `Mode` vs `Status`, or did you explicitly use different words? For example, we could change `MaintenanceStatus` to `MaintenanceModes`. I'm not necessarily saying we should, just asking if this was done for a specific reason.


include/mesos/maintenance/maintenance.proto (line 29)
<https://reviews.apache.org/r/36571/#comment151977>

    is `interval` referring to the old proto here?
    Should we say `A set of machines scheduled to go into maintenance during the same unavailability.`



include/mesos/maintenance/maintenance.proto (line 33)
<https://reviews.apache.org/r/36571/#comment152001>

    Why the choice of repeated `MachineInfo` over `MachineInfos`?



include/mesos/maintenance/maintenance.proto (line 42)
<https://reviews.apache.org/r/36571/#comment152003>

    Looks like you're trying to stick to `agent` in this file. s/slaves/agents
    Can we find a better word for `blackout`? Maybe something like `upgrade`?



include/mesos/maintenance/maintenance.proto (line 58)
<https://reviews.apache.org/r/36571/#comment152005>

    Can we reword this a little? I understand what you are trying to say, but it might not be clear to a newcomer.
    
    The `agent` is not releasing the resources, rather the agent is in co-operation with the frameworks to try and drain / evacuate / release resources in such a way that we still maximize utilization but without knowingly violating SLAs.
    
    The ideal scenario would be that the agent is fully utilized right up until the maintenance window, but no critical tasks end up getting killed, right?



include/mesos/mesos.proto (line 111)
<https://reviews.apache.org/r/36571/#comment151973>

    Can you expand on this comment and explain how the IP vs. hostname matching will work? I know you're explaining this elsewhere, but here in the proto is a common place for users to come look to understand how this message works, and what the expectations are.



src/Makefile.am (line 428)
<https://reviews.apache.org/r/36571/#comment152009>

    Is this just a whitespace change?
    Want to make sure I'm not missing anything here.



src/master/registry.proto (line 25)
<https://reviews.apache.org/r/36571/#comment152013>

    s/master/top level/ to avoid any confusion? Or did you really mean `master` as in the entity?



src/master/registry.proto (line 42)
<https://reviews.apache.org/r/36571/#comment152016>

    Can we use backticks (`)
    s/exists/exist



src/master/registry.proto (line 58)
<https://reviews.apache.org/r/36571/#comment152017>

    backticks (`)?


- Joris Van Remoortere


On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/#review96359
-----------------------------------------------------------

Ship it!


Ship It!

- Guangya Liu


On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joris Van Remoortere <jo...@gmail.com>.

> On Aug. 25, 2015, 6:04 a.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, line 111
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048532#file1048532line111>
> >
> >     Can you please show a case why end user want to hold more agents on a single machine? This may cause resource overcommit for the single machine.
> 
> Joseph Wu wrote:
>     In this case, it doesn't matter *why* the user puts 2+ agents on a single machine.  It matters that the user *can* put 2+ agents on a single machine.

Just to clarify this: It is possible to run multiple agents on the same machine by constraining the --resources for each such that their sum is not greater than the total amount of resources on the agent.
Some users have even purposely overcommitted the resources on their agents to try out a simple form of oversubscription (prior to 0.23).


- Joris


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


On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Joseph Wu <jo...@mesosphere.io>.

> On Aug. 24, 2015, 11:04 p.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, line 111
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048532#file1048532line111>
> >
> >     Can you please show a case why end user want to hold more agents on a single machine? This may cause resource overcommit for the single machine.

In this case, it doesn't matter *why* the user puts 2+ agents on a single machine.  It matters that the user *can* put 2+ agents on a single machine.


> On Aug. 24, 2015, 11:04 p.m., Guangya Liu wrote:
> > include/mesos/maintenance/maintenance.proto, line 19
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048531#file1048531line19>
> >
> >     I think that you will handle v1 api in other patches?

That is correct.  Specifically, for the maintenance protobufs, we will add it to the V1 API when it is necessary to do so.


- Joseph


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


On Aug. 24, 2015, 11:33 a.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 11:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Guangya Liu <gy...@gmail.com>.

> On Aug. 25, 2015, 6:04 a.m., Guangya Liu wrote:
> > include/mesos/mesos.proto, line 111
> > <https://reviews.apache.org/r/36571/diff/14/?file=1048532#file1048532line111>
> >
> >     Can you please show a case why end user want to hold more agents on a single machine? This may cause resource overcommit for the single machine.
> 
> Joseph Wu wrote:
>     In this case, it doesn't matter *why* the user puts 2+ agents on a single machine.  It matters that the user *can* put 2+ agents on a single machine.
> 
> Joris Van Remoortere wrote:
>     Just to clarify this: It is possible to run multiple agents on the same machine by constraining the --resources for each such that their sum is not greater than the total amount of resources on the agent.
>     Some users have even purposely overcommitted the resources on their agents to try out a simple form of oversubscription (prior to 0.23).

<p>Good to know, thanks Joris!</p>


- Guangya


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


On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Re: Review Request 36571: Maintenance Primitives: Add maintenance-related, registry protobufs.

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36571/#review96301
-----------------------------------------------------------



include/mesos/maintenance/maintenance.proto (line 19)
<https://reviews.apache.org/r/36571/#comment151624>

    I think that you will handle v1 api in other patches?



include/mesos/mesos.proto (line 111)
<https://reviews.apache.org/r/36571/#comment151623>

    Can you please show a case why end user want to hold more agents on a single machine? This may cause resource overcommit for the single machine.


- Guangya Liu


On Aug. 24, 2015, 6:33 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36571/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2015, 6:33 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, and Vinod Kone.
> 
> 
> Bugs: MESOS-3066
>     https://issues.apache.org/jira/browse/MESOS-3066
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> * MachineInfo - Describes a single box that holds one or more agents.
> * MachineInfos - A list of boxes.
> * maintenance::Window - A set of machines and a planned downtime period.
> * maintenance::Schedule - A set of maintenance windows.
> * maintenance::Mode - An enum for the three states of maintenance: Normal, Draining, Deactivated.
> * Registry::MaintenanceStatus - Holds the maintenance mode of a machine.
> 
> 
> Diffs
> -----
> 
>   include/mesos/maintenance/maintenance.hpp PRE-CREATION 
>   include/mesos/maintenance/maintenance.proto PRE-CREATION 
>   include/mesos/mesos.proto 33e1b28f1ccbe227657a14395f81df20e0a9e193 
>   src/Makefile.am 9fd71d1ddf442712977596e7a13969ff5c1d68db 
>   src/master/registry.proto a1995e56886f5296bc71d1bdfebec0d8316396c6 
> 
> Diff: https://reviews.apache.org/r/36571/diff/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>