You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Thomas Marshall <tw...@gmail.com> on 2012/07/10 02:23:54 UTC

Review Request: Static Allocator

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

Review request for mesos.


Description
-------

Simple implementation of a static allocator.


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs
-----

  src/Makefile.am 8760f59 
  src/master/static_allocator_process.hpp PRE-CREATION 
  src/master/static_allocator_process.cpp PRE-CREATION 
  src/tests/allocator_tests.cpp PRE-CREATION 

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


Testing
-------

make check on Lion
new test in allocator_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

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



src/master/simple_allocator_process.hpp
<https://reviews.apache.org/r/5870/#comment22052>

    Please add a comment describing what this struct is and how it gets used below.



src/master/simple_allocator_process.hpp
<https://reviews.apache.org/r/5870/#comment22051>

    s/SlaveDedication/Dedication/



src/master/simple_allocator_process.hpp
<https://reviews.apache.org/r/5870/#comment22047>

    Comment this guy.



src/master/simple_allocator_process.hpp
<https://reviews.apache.org/r/5870/#comment22049>

    s/slaveDedications/dedications/



src/master/simple_allocator_process.hpp
<https://reviews.apache.org/r/5870/#comment22045>

    const&



src/slave/flags.hpp
<https://reviews.apache.org/r/5870/#comment22048>

    Let's make these their own flags.


- Benjamin Hindman


On Aug. 8, 2012, 10:19 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5870/
> -----------------------------------------------------------
> 
> (Updated Aug. 8, 2012, 10:19 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> Simple implementation of a static allocator.
> 
> This patch depends on 2 other pending code review:
> https://reviews.apache.org/r/5599/
> https://reviews.apache.org/r/5913/
> 
> 
> This addresses bug MESOS-230.
>     https://issues.apache.org/jira/browse/MESOS-230
> 
> 
> Diffs
> -----
> 
>   src/master/simple_allocator_process.hpp PRE-CREATION 
>   src/slave/flags.hpp 0c7917f 
>   src/tests/allocator_tests.cpp b3db13d 
>   src/tests/utils.hpp a768360 
> 
> Diff: https://reviews.apache.org/r/5870/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> new test in allocator_process_tests.cpp
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated Sept. 24, 2012, 8:52 p.m.)


Review request for mesos.


Changes
-------

Minor bug fix.


Description
-------

Simple implementation of a static allocator.

This patch depends on 2 other pending code review:
https://reviews.apache.org/r/5599/
https://reviews.apache.org/r/5913/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  include/mesos/mesos.proto 72fccde 
  src/master/hierarchical_allocator_process.hpp 266d339 
  src/slave/flags.hpp 75b8b0f 
  src/slave/slave.cpp 49d8e76 
  src/tests/allocator_tests.cpp ed52ff6 
  src/tests/allocator_zookeeper_tests.cpp bd408c2 
  src/tests/utils.hpp fc2023b 

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


Testing
-------

make check on Lion
new test in allocator_process_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated Sept. 23, 2012, 10:40 p.m.)


Review request for mesos.


Changes
-------

Added support for a dedicated_resource flag on slaves. This allows only part of the resources on the slave to be reserved for the slave's dedicated_user/framework.


Description
-------

Simple implementation of a static allocator.

This patch depends on 2 other pending code review:
https://reviews.apache.org/r/5599/
https://reviews.apache.org/r/5913/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  include/mesos/mesos.proto 72fccde 
  src/master/hierarchical_allocator_process.hpp 266d339 
  src/slave/flags.hpp 75b8b0f 
  src/slave/slave.cpp 49d8e76 
  src/tests/allocator_tests.cpp ed52ff6 
  src/tests/allocator_zookeeper_tests.cpp bd408c2 
  src/tests/utils.hpp fc2023b 

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


Testing
-------

make check on Lion
new test in allocator_process_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated Aug. 21, 2012, 7:20 p.m.)


Review request for mesos.


Changes
-------

Test refactor for consistency.


Description
-------

Simple implementation of a static allocator.

This patch depends on 2 other pending code review:
https://reviews.apache.org/r/5599/
https://reviews.apache.org/r/5913/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  include/mesos/mesos.proto 72fccde 
  src/master/hierarchical_allocator_process.hpp 86c9b1e 
  src/slave/flags.hpp 0c7917f 
  src/slave/slave.cpp 4efd41e 
  src/tests/allocator_tests.cpp 9725c2e 
  src/tests/allocator_zookeeper_tests.cpp c4956dc 
  src/tests/utils.hpp 83d5daa 

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


Testing
-------

make check on Lion
new test in allocator_process_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated Aug. 21, 2012, 6:23 p.m.)


Review request for mesos.


Changes
-------

Updated to latest review.


Description
-------

Simple implementation of a static allocator.

This patch depends on 2 other pending code review:
https://reviews.apache.org/r/5599/
https://reviews.apache.org/r/5913/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  include/mesos/mesos.proto 72fccde 
  src/master/hierarchical_allocator_process.hpp 86c9b1e 
  src/slave/flags.hpp 0c7917f 
  src/slave/slave.cpp 4efd41e 
  src/tests/allocator_tests.cpp 9725c2e 

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


Testing
-------

make check on Lion
new test in allocator_process_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

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



src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/5870/#comment22582>

    I believe it should be the case that a SlaveID should be valid. Please assert thusly via: CHECK(slaves.contains(slaveId));



src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/5870/#comment22583>

    How about:
    
    if (slaves[slaveId].has_dedicated_user() &&
        slaves[slaveId].dedicated_user() != frameworks[frameworkId].user()) {
      return true;
    }
    
    if (slaves[slaveId].has_dedicated_framework() &&
        slaves[slaveId].dedicated_framework() != frameworks[frameworkId].name()) {
      return true;
    }
    
    return false;



src/slave/flags.hpp
<https://reviews.apache.org/r/5870/#comment22580>

    s/Spcifies/Specifies/



src/slave/slave.cpp
<https://reviews.apache.org/r/5870/#comment22581>

    Let's make these optional instead (i.e., 'if (flags.dedicated_user.isSome())').


- Benjamin Hindman


On Aug. 16, 2012, 9:13 p.m., Thomas Marshall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5870/
> -----------------------------------------------------------
> 
> (Updated Aug. 16, 2012, 9:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Description
> -------
> 
> Simple implementation of a static allocator.
> 
> This patch depends on 2 other pending code review:
> https://reviews.apache.org/r/5599/
> https://reviews.apache.org/r/5913/
> 
> 
> This addresses bug MESOS-230.
>     https://issues.apache.org/jira/browse/MESOS-230
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 72fccde 
>   src/master/hierarchical_allocator_process.hpp PRE-CREATION 
>   src/slave/flags.hpp 0c7917f 
>   src/slave/slave.cpp 4efd41e 
>   src/tests/allocator_tests.cpp 98efb0d 
> 
> Diff: https://reviews.apache.org/r/5870/diff/
> 
> 
> Testing
> -------
> 
> make check on Lion
> new test in allocator_process_tests.cpp
> 
> 
> Thanks,
> 
> Thomas Marshall
> 
>


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated Aug. 16, 2012, 9:13 p.m.)


Review request for mesos.


Changes
-------

Updated to latest review. In particular, the isStatic bool has been removed from HierarchicalAllocatorProcess, which now assumes that it should do static allocations any time a slave has its dedicated_user and/or dedicated_framework flag set (which have also been moved from being part of attributes to being top level flags and optional members of SlaveInfo).


Description
-------

Simple implementation of a static allocator.

This patch depends on 2 other pending code review:
https://reviews.apache.org/r/5599/
https://reviews.apache.org/r/5913/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  include/mesos/mesos.proto 72fccde 
  src/master/hierarchical_allocator_process.hpp PRE-CREATION 
  src/slave/flags.hpp 0c7917f 
  src/slave/slave.cpp 4efd41e 
  src/tests/allocator_tests.cpp 98efb0d 

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


Testing
-------

make check on Lion
new test in allocator_process_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated Aug. 8, 2012, 10:19 p.m.)


Review request for mesos.


Changes
-------

Updated to reflect changes made further up the review pipeline (eg templating SimpleAllocatorProcess).


Description
-------

Simple implementation of a static allocator.

This patch depends on 2 other pending code review:
https://reviews.apache.org/r/5599/
https://reviews.apache.org/r/5913/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  src/master/simple_allocator_process.hpp PRE-CREATION 
  src/slave/flags.hpp 0c7917f 
  src/tests/allocator_tests.cpp b3db13d 
  src/tests/utils.hpp a768360 

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


Testing
-------

make check on Lion
new test in allocator_process_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated July 11, 2012, 11:28 p.m.)


Review request for mesos.


Changes
-------

Updated to trunk.

This patch now assumes that you've applied https://reviews.apache.org/r/5913/


Description (updated)
-------

Simple implementation of a static allocator.

This patch depends on 2 other pending code review:
https://reviews.apache.org/r/5599/
https://reviews.apache.org/r/5913/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  src/Makefile.am 99bee0a 
  src/master/static_allocator_process.hpp PRE-CREATION 
  src/master/static_allocator_process.cpp PRE-CREATION 
  src/tests/allocator_process_tests.cpp PRE-CREATION 

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


Testing (updated)
-------

make check on Lion
new test in allocator_process_tests.cpp


Thanks,

Thomas Marshall


Re: Review Request: Static Allocator

Posted by Thomas Marshall <tw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5870/
-----------------------------------------------------------

(Updated July 11, 2012, 12:09 a.m.)


Review request for mesos.


Changes
-------

Updated to trunk.


Description (updated)
-------

Simple implementation of a static allocator.

This patch depends on another pending code review:
https://reviews.apache.org/r/5599/


This addresses bug MESOS-230.
    https://issues.apache.org/jira/browse/MESOS-230


Diffs (updated)
-----

  src/Makefile.am eb1944f 
  src/master/static_allocator_process.hpp PRE-CREATION 
  src/master/static_allocator_process.cpp PRE-CREATION 
  src/tests/allocator_tests.cpp 610826b 

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


Testing
-------

make check on Lion
new test in allocator_tests.cpp


Thanks,

Thomas Marshall