You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Alexander Rukletsov <al...@mesosphere.io> on 2015/01/14 19:51:34 UTC

Review Request 29890: Refactored allocator interface to support general implementations.

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

Review request for mesos, Bernd Mathiske and Niklas Nielsen.


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.

[WIP]: more refactoring will be done either in an update to this RR or in a separate RR.


Diffs
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 224569a 
  src/master/hierarchical_allocator_process.hpp ccd37b4 
  src/master/main.cpp 193d53f 
  src/tests/cluster.hpp 74cedb3 
  src/tests/hierarchical_allocator_tests.cpp 7c05123 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/mesos.hpp 591134b 
  src/tests/mesos.cpp 3b98c69 
  src/tests/resource_offers_tests.cpp d098e70 
  src/tests/slave_recovery_tests.cpp 809822e 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

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


Patch looks great!

Reviews applied: [29890]

All tests passed.

- Mesos ReviewBot


On Jan. 14, 2015, 6:51 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 6:51 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> [WIP]: more refactoring will be done either in an update to this RR or in a separate RR.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> On Jan. 23, 2015, 1:05 a.m., Niklas Nielsen wrote:
> > src/master/allocator.hpp, line 231
> > <https://reviews.apache.org/r/29890/diff/4/?file=829895#file829895line231>
> >
> >     Don't we also want to change this to MesosAllocatorProcess and make the template handle 'template <typename MesosAllocatorProcess>' be the generalized 'AllocatorProcess' one?

Totally makes sense, I agree.


- Alexander


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


On Jan. 22, 2015, 4:55 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2015, 4:55 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review69317
-----------------------------------------------------------

Ship it!


One naming nit - but otherwise looks good to go.


src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment113979>

    Don't we also want to change this to MesosAllocatorProcess and make the template handle 'template <typename MesosAllocatorProcess>' be the generalized 'AllocatorProcess' one?


- Niklas Nielsen


On Jan. 22, 2015, 8:55 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 22, 2015, 8:55 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review70405
-----------------------------------------------------------


Looks like there is a problem viewing diffs on review board :(

- Vinod Kone


On Jan. 30, 2015, 3:33 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2015, 3:33 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 318a756 
>   src/master/hierarchical_allocator_process.hpp 6b44892 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 90fda52 
>   src/tests/fault_tolerance_tests.cpp f927d4a 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 018a6cc 
>   src/tests/master_authorization_tests.cpp 20adaa9 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 17c2d8f 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp 24a7eab 
>   src/tests/slave_recovery_tests.cpp 7e2e63d 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> On Jan. 31, 2015, 1:05 a.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 676
> > <https://reviews.apache.org/r/29890/diff/7/?file=841772#file841772line676>
> >
> >     why does T have to be a HierarchicalDRFAllocator instead of an Allocator or MesosAllocator?

We expect a "real" allocator here, `HierarchicalDRFAllocator` is the only "real" one we have now.


- Alexander


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


On Jan. 30, 2015, 3:33 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2015, 3:33 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 318a756 
>   src/master/hierarchical_allocator_process.hpp 6b44892 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 90fda52 
>   src/tests/fault_tolerance_tests.cpp f927d4a 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 018a6cc 
>   src/tests/master_authorization_tests.cpp 20adaa9 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 17c2d8f 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp 24a7eab 
>   src/tests/slave_recovery_tests.cpp 7e2e63d 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

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

> On Jan. 31, 2015, 1:05 a.m., Vinod Kone wrote:
> > src/tests/mesos.hpp, line 676
> > <https://reviews.apache.org/r/29890/diff/7/?file=841772#file841772line676>
> >
> >     why does T have to be a HierarchicalDRFAllocator instead of an Allocator or MesosAllocator?
> 
> Alexander Rukletsov wrote:
>     We expect a "real" allocator here, `HierarchicalDRFAllocator` is the only "real" one we have now.

That's fine. The class will need to be instantiated with a real 'T'. If 'T' is HierarchicalDRFAllocator what's the point of template? Do you imagine in future we would subclass HierarchicalDRFAllocator? Previously 'T' was an abstract class 'AllocatorProcess', so I'm not sure why is this any different.


- Vinod


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


On Feb. 4, 2015, 4:47 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2015, 4:47 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 318a756 
>   src/master/hierarchical_allocator_process.hpp 6b44892 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 90fda52 
>   src/tests/fault_tolerance_tests.cpp f927d4a 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 018a6cc 
>   src/tests/master_authorization_tests.cpp 20adaa9 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 17c2d8f 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp 24a7eab 
>   src/tests/slave_recovery_tests.cpp 7e2e63d 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review70462
-----------------------------------------------------------



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment115665>

    Can you do a forward declaration of MesosAllocatorProcess and pull the implementation below MesosAllocator?



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment115666>

    s/invokations/invocations/



src/tests/cluster.hpp
<https://reviews.apache.org/r/29890/#comment115678>

    This comment should be inside the else block.



src/tests/mesos.hpp
<https://reviews.apache.org/r/29890/#comment115686>

    why does T have to be a HierarchicalDRFAllocator instead of an Allocator or MesosAllocator?


- Vinod Kone


On Jan. 30, 2015, 3:33 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2015, 3:33 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-incubating
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 318a756 
>   src/master/hierarchical_allocator_process.hpp 6b44892 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 90fda52 
>   src/tests/fault_tolerance_tests.cpp f927d4a 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 018a6cc 
>   src/tests/master_authorization_tests.cpp 20adaa9 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 17c2d8f 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp 24a7eab 
>   src/tests/slave_recovery_tests.cpp 7e2e63d 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review72004
-----------------------------------------------------------

Ship it!


Ship It!

- Vinod Kone


On Feb. 10, 2015, 7:59 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2015, 7:59 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators that do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator users and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 3b128c406dfadffd3e7344dccb9abda2b99734ab 
>   src/master/allocator.hpp 2149ea48dd8614dc29350efaddcb43b6a082a43e 
>   src/master/hierarchical_allocator_process.hpp 10fa6ec4116174f0fa597714976507a6c54f082b 
>   src/master/main.cpp d4adae5a6044aef9f7cc214f0f467359b6f7a29a 
>   src/tests/cluster.hpp 2ea404747f64e94d8908bd84a5aa575d2c010f71 
>   src/tests/fault_tolerance_tests.cpp dc8efcd5e3ea4580af95138ee22aa2a93cee9196 
>   src/tests/hierarchical_allocator_tests.cpp df844b57df5f9bb886e2e380d8751c809d3fbd5b 
>   src/tests/master_allocator_tests.cpp cad8024c9ae779e55e036c39ffa9165071a89a15 
>   src/tests/master_authorization_tests.cpp 6fd0efa444dce5203e2e755d4de0483fcaa277f8 
>   src/tests/master_tests.cpp 18eabd4af5effe83fed7479fda19242b73a8536f 
>   src/tests/mesos.hpp 83a369968ab2403fa341829ac5d11f7243095190 
>   src/tests/mesos.cpp 21a405366f56c963611324076efe775f85b9d9f7 
>   src/tests/partition_tests.cpp bd7940d0a2b6ef324c74bf3fe990dc0e52592669 
>   src/tests/rate_limiting_tests.cpp 8b55bffff0b7713806fc0bcc63d8a7097a8408e0 
>   src/tests/resource_offers_tests.cpp b37115398ca26a8dc424e026c9e400ede43ccc29 
>   src/tests/slave_recovery_tests.cpp bd2bdefbed0efbd8ee27c288097cecca86e86969 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Feb. 12, 2015, 7:39 p.m.)


Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.


Changes
-------

Rebased.


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


Repository: mesos


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators that do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator users and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 3b128c406dfadffd3e7344dccb9abda2b99734ab 
  src/master/allocator.hpp 2149ea48dd8614dc29350efaddcb43b6a082a43e 
  src/master/hierarchical_allocator_process.hpp 10fa6ec4116174f0fa597714976507a6c54f082b 
  src/master/main.cpp 1dce7fba66697338451d9f9b544b33bfdf880f46 
  src/tests/cluster.hpp 2ea404747f64e94d8908bd84a5aa575d2c010f71 
  src/tests/fault_tolerance_tests.cpp dc8efcd5e3ea4580af95138ee22aa2a93cee9196 
  src/tests/hierarchical_allocator_tests.cpp df844b57df5f9bb886e2e380d8751c809d3fbd5b 
  src/tests/master_allocator_tests.cpp b16462571bc3760aedb0a86019d6627ded15ea49 
  src/tests/master_authorization_tests.cpp 4b8c134aa5e7300da262d48595bf361e2dc79949 
  src/tests/master_tests.cpp c678527942506ef23fa4f1aebf9664e4cc21b561 
  src/tests/mesos.hpp 2b0c90d166ed33065f2a4a2909c61cc5119da511 
  src/tests/mesos.cpp ac2dc737eff48d122cfd604d4820f423e5bc0472 
  src/tests/partition_tests.cpp bd7940d0a2b6ef324c74bf3fe990dc0e52592669 
  src/tests/rate_limiting_tests.cpp 8114c79786850d551ce7ddcaca65f66a1d03fd64 
  src/tests/resource_offers_tests.cpp b37115398ca26a8dc424e026c9e400ede43ccc29 
  src/tests/slave_recovery_tests.cpp bd2bdefbed0efbd8ee27c288097cecca86e86969 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Feb. 10, 2015, 7:59 p.m.)


Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.


Changes
-------

Addressed Vinod's comments & rebased.


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


Repository: mesos


Description (updated)
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators that do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator users and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 3b128c406dfadffd3e7344dccb9abda2b99734ab 
  src/master/allocator.hpp 2149ea48dd8614dc29350efaddcb43b6a082a43e 
  src/master/hierarchical_allocator_process.hpp 10fa6ec4116174f0fa597714976507a6c54f082b 
  src/master/main.cpp d4adae5a6044aef9f7cc214f0f467359b6f7a29a 
  src/tests/cluster.hpp 2ea404747f64e94d8908bd84a5aa575d2c010f71 
  src/tests/fault_tolerance_tests.cpp dc8efcd5e3ea4580af95138ee22aa2a93cee9196 
  src/tests/hierarchical_allocator_tests.cpp df844b57df5f9bb886e2e380d8751c809d3fbd5b 
  src/tests/master_allocator_tests.cpp cad8024c9ae779e55e036c39ffa9165071a89a15 
  src/tests/master_authorization_tests.cpp 6fd0efa444dce5203e2e755d4de0483fcaa277f8 
  src/tests/master_tests.cpp 18eabd4af5effe83fed7479fda19242b73a8536f 
  src/tests/mesos.hpp 83a369968ab2403fa341829ac5d11f7243095190 
  src/tests/mesos.cpp 21a405366f56c963611324076efe775f85b9d9f7 
  src/tests/partition_tests.cpp bd7940d0a2b6ef324c74bf3fe990dc0e52592669 
  src/tests/rate_limiting_tests.cpp 8b55bffff0b7713806fc0bcc63d8a7097a8408e0 
  src/tests/resource_offers_tests.cpp b37115398ca26a8dc424e026c9e400ede43ccc29 
  src/tests/slave_recovery_tests.cpp bd2bdefbed0efbd8ee27c288097cecca86e86969 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Feb. 4, 2015, 4:47 p.m.)


Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.


Changes
-------

Addressed Vinod's comments


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


Repository: mesos


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 318a756 
  src/master/hierarchical_allocator_process.hpp 6b44892 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 90fda52 
  src/tests/fault_tolerance_tests.cpp f927d4a 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 018a6cc 
  src/tests/master_authorization_tests.cpp 20adaa9 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 17c2d8f 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/partition_tests.cpp fea7801 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp 24a7eab 
  src/tests/slave_recovery_tests.cpp 7e2e63d 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 30, 2015, 3:33 p.m.)


Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.


Changes
-------

Rebased.


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 318a756 
  src/master/hierarchical_allocator_process.hpp 6b44892 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 90fda52 
  src/tests/fault_tolerance_tests.cpp f927d4a 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 018a6cc 
  src/tests/master_authorization_tests.cpp 20adaa9 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 17c2d8f 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/partition_tests.cpp fea7801 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp 24a7eab 
  src/tests/slave_recovery_tests.cpp 7e2e63d 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 28, 2015, 9:50 p.m.)


Review request for mesos, Bernd Mathiske, Niklas Nielsen, and Vinod Kone.


Changes
-------

adding myself as reviewer -- @vinodkone


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 318a756 
  src/master/hierarchical_allocator_process.hpp 6b44892 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 74cedb3 
  src/tests/fault_tolerance_tests.cpp e0065b3 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/master_authorization_tests.cpp 42ffe24 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 3f4704b 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/partition_tests.cpp fea7801 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp ffad1f8 
  src/tests/slave_recovery_tests.cpp d135856 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 28, 2015, 5:26 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
-------

Addressed Niklas' comment.


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 318a756 
  src/master/hierarchical_allocator_process.hpp 6b44892 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 74cedb3 
  src/tests/fault_tolerance_tests.cpp e0065b3 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/master_authorization_tests.cpp 42ffe24 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 3f4704b 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/partition_tests.cpp fea7801 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp ffad1f8 
  src/tests/slave_recovery_tests.cpp d135856 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review69841
-----------------------------------------------------------



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment114652>

    Can we change this to MesosAllocatorProcess?


- Niklas Nielsen


On Jan. 27, 2015, 5:15 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2015, 5:15 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 318a756 
>   src/master/hierarchical_allocator_process.hpp 6b44892 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/fault_tolerance_tests.cpp e0065b3 
>   src/tests/hierarchical_allocator_tests.cpp f44d9e9 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/master_authorization_tests.cpp 42ffe24 
>   src/tests/master_tests.cpp 678d27f 
>   src/tests/mesos.hpp 3f4704b 
>   src/tests/mesos.cpp 5ed4df5 
>   src/tests/partition_tests.cpp fea7801 
>   src/tests/rate_limiting_tests.cpp 7f5ca25 
>   src/tests/resource_offers_tests.cpp ffad1f8 
>   src/tests/slave_recovery_tests.cpp d135856 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 27, 2015, 1:15 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
-------

Address Niklas' comments.


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 318a756 
  src/master/hierarchical_allocator_process.hpp 6b44892 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 74cedb3 
  src/tests/fault_tolerance_tests.cpp e0065b3 
  src/tests/hierarchical_allocator_tests.cpp f44d9e9 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/master_authorization_tests.cpp 42ffe24 
  src/tests/master_tests.cpp 678d27f 
  src/tests/mesos.hpp 3f4704b 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/partition_tests.cpp fea7801 
  src/tests/rate_limiting_tests.cpp 7f5ca25 
  src/tests/resource_offers_tests.cpp ffad1f8 
  src/tests/slave_recovery_tests.cpp d135856 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 22, 2015, 4:55 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
-------

Address Niklas' comments.


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 224569a 
  src/master/hierarchical_allocator_process.hpp ccd37b4 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 74cedb3 
  src/tests/hierarchical_allocator_tests.cpp 7c05123 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/mesos.hpp 591134b 
  src/tests/mesos.cpp 5ed4df5 
  src/tests/resource_offers_tests.cpp d098e70 
  src/tests/slave_recovery_tests.cpp 809822e 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> On Jan. 21, 2015, 7:28 p.m., Niklas Nielsen wrote:
> > src/tests/cluster.hpp, lines 100-101
> > <https://reviews.apache.org/r/29890/diff/3/?file=826906#file826906line100>
> >
> >     This should be implied and applies to many places in the source base. One example is start() in for the slave where the containerizer's lifetime is managed by the caller (if provided). I don't think this comment is necessary.

Ok, let's drop it.


> On Jan. 21, 2015, 7:28 p.m., Niklas Nielsen wrote:
> > src/tests/cluster.hpp, line 129
> > <https://reviews.apache.org/r/29890/diff/3/?file=826906#file826906line129>
> >
> >     I still don't understand why you need the raw pointer. Is it to distinguish between the life-time of the provided one and the "default" one? Try to take a look at how the containerizer is being handled in slave start() - could we reuse that pattern for consistency?

I think my design is a bit clearer about what's our intention and according object lifetimes are, but let's go for consistency. I'll re-work this.


> On Jan. 21, 2015, 7:28 p.m., Niklas Nielsen wrote:
> > src/tests/master_allocator_tests.cpp, lines 79-82
> > <https://reviews.apache.org/r/29890/diff/3/?file=826908#file826908line79>
> >
> >     Is this still relevant with the allocator destructor (which tears down the process as well)?

No, and this comment is gone in a following RR (where the status quo is restored). I'll kill it here.


- Alexander


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


On Jan. 20, 2015, 5:59 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 5:59 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review68980
-----------------------------------------------------------



src/tests/cluster.hpp
<https://reviews.apache.org/r/29890/#comment113546>

    This should be implied and applies to many places in the source base. One example is start() in for the slave where the containerizer's lifetime is managed by the caller (if provided). I don't think this comment is necessary.



src/tests/cluster.hpp
<https://reviews.apache.org/r/29890/#comment113541>

    I still don't understand why you need the raw pointer. Is it to distinguish between the life-time of the provided one and the "default" one? Try to take a look at how the containerizer is being handled in slave start() - could we reuse that pattern for consistency?



src/tests/master_allocator_tests.cpp
<https://reviews.apache.org/r/29890/#comment113547>

    Is this still relevant with the allocator destructor (which tears down the process as well)?


- Niklas Nielsen


On Jan. 20, 2015, 9:59 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2015, 9:59 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp e5e76ce 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 20, 2015, 5:59 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
-------

Address comments and issues.


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs (updated)
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 224569a 
  src/master/hierarchical_allocator_process.hpp ccd37b4 
  src/master/main.cpp e5e76ce 
  src/tests/cluster.hpp 74cedb3 
  src/tests/hierarchical_allocator_tests.cpp 7c05123 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/mesos.hpp 591134b 
  src/tests/mesos.cpp 3b98c69 
  src/tests/resource_offers_tests.cpp d098e70 
  src/tests/slave_recovery_tests.cpp 809822e 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> On Jan. 16, 2015, 8:26 p.m., Niklas Nielsen wrote:
> > src/tests/cluster.hpp, lines 128-129
> > <https://reviews.apache.org/r/29890/diff/2/?file=822660#file822660line128>
> >
> >     Why have both?

If we don't specify an allocator explicitly, we would like to have a default. This is consistent with the previous behaviour.


> On Jan. 16, 2015, 8:26 p.m., Niklas Nielsen wrote:
> > src/tests/master_allocator_tests.cpp, lines 79-82
> > <https://reviews.apache.org/r/29890/diff/2/?file=822662#file822662line79>
> >
> >     The destructor of the MesosAllocator will stop the process, right? How is this different from status quo, or are you trying to solve other problems mid-way?

By removing these lines I change the original logic, which is restored in [this patch](https://reviews.apache.org/r/29927/). Hence the comment. A use case for the original behaviour: we would like to stop allocation but we are not managing the lifetime of the allocator instance.


- Alexander


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


On Jan. 15, 2015, 5:17 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2015, 5:17 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review68273
-----------------------------------------------------------



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112707>

    This is the public API for _______ (fill blank) :)



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112706>

    Nit: "Add explanatory comments ..." or simply "Document API calls".



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112708>

    'There is no need ...'



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112709>

    s/Extract/Move/



src/tests/cluster.hpp
<https://reviews.apache.org/r/29890/#comment112448>

    Why have both?



src/tests/master_allocator_tests.cpp
<https://reviews.apache.org/r/29890/#comment112711>

    The destructor of the MesosAllocator will stop the process, right? How is this different from status quo, or are you trying to solve other problems mid-way?


- Niklas Nielsen


On Jan. 15, 2015, 9:17 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2015, 9:17 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> On Jan. 18, 2015, 1:06 p.m., Bernd Mathiske wrote:
> > src/master/allocator.hpp, line 227
> > <https://reviews.apache.org/r/29890/diff/2/?file=822657#file822657line227>
> >
> >     Not sure if that is needed. It seems to be a common accepted pattern the way it is.

You are right, we decided to be consistent with the current usage pattern.


> On Jan. 18, 2015, 1:06 p.m., Bernd Mathiske wrote:
> > src/master/hierarchical_allocator_process.hpp, line 56
> > <https://reviews.apache.org/r/29890/diff/2/?file=822658#file822658line56>
> >
> >     Is this still needed?
> >     If not, this also affects the comment just above.

This patch doesn't touch the way Hierarchical DRF allocator is declared and implemented. These `typedef`s are used for creating allocator instances throughout the code.


- Alexander


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


On Jan. 15, 2015, 5:17 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2015, 5:17 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Bernd Mathiske <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review68561
-----------------------------------------------------------



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112883>

    Not sure if that is needed. It seems to be a common accepted pattern the way it is.



src/master/hierarchical_allocator_process.hpp
<https://reviews.apache.org/r/29890/#comment112885>

    Is this still needed?
    If not, this also affects the comment just above.


- Bernd Mathiske


On Jan. 15, 2015, 9:17 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2015, 9:17 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 15, 2015, 5:17 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
-------

Update description.


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


Repository: mesos-git


Description (updated)
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.


Diffs
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 224569a 
  src/master/hierarchical_allocator_process.hpp ccd37b4 
  src/master/main.cpp 193d53f 
  src/tests/cluster.hpp 74cedb3 
  src/tests/hierarchical_allocator_tests.cpp 7c05123 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/mesos.hpp 591134b 
  src/tests/mesos.cpp 3b98c69 
  src/tests/resource_offers_tests.cpp d098e70 
  src/tests/slave_recovery_tests.cpp 809822e 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/
-----------------------------------------------------------

(Updated Jan. 15, 2015, 5:13 p.m.)


Review request for mesos, Bernd Mathiske and Niklas Nielsen.


Changes
-------

Address Niklas' comments.


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


Repository: mesos-git


Description
-------

Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.

[WIP]: more refactoring will be done either in an update to this RR or in a separate RR.


Diffs (updated)
-----

  src/local/local.cpp 76e73a4 
  src/master/allocator.hpp 224569a 
  src/master/hierarchical_allocator_process.hpp ccd37b4 
  src/master/main.cpp 193d53f 
  src/tests/cluster.hpp 74cedb3 
  src/tests/hierarchical_allocator_tests.cpp 7c05123 
  src/tests/master_allocator_tests.cpp 2430622 
  src/tests/mesos.hpp 591134b 
  src/tests/mesos.cpp 3b98c69 
  src/tests/resource_offers_tests.cpp d098e70 
  src/tests/slave_recovery_tests.cpp 809822e 

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


Testing
-------

make distcheck (Ubuntu, OS X)


Thanks,

Alexander Rukletsov


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/master/allocator.hpp, line 227
> > <https://reviews.apache.org/r/29890/diff/1/?file=821527#file821527line227>
> >
> >     Did you run into any problems making it owned? If not, can we do it now?

No, just postponed it for a separate review to keep things simple: https://reviews.apache.org/r/29926/


> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/tests/cluster.hpp, lines 100-101
> > <https://reviews.apache.org/r/29890/diff/1/?file=821530#file821530line100>
> >
> >     How is this different from before?
> 
> Alexander Rukletsov wrote:
>     No, just postponed it for a separate review to keep things simple: https://reviews.apache.org/r/29926/

Before we were provided with AllocatorProcess managed by the caller, that we wrapped into an Allocator instance managed by the class instance. Now we accept Allocator instance directly.


- Alexander


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


On Jan. 15, 2015, 5:13 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 15, 2015, 5:13 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> [WIP]: more refactoring will be done either in an update to this RR or in a separate RR.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Alexander Rukletsov <al...@mesosphere.io>.

> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > Very nice refactor, Alex!
> > 
> > RR comment: This patch touches quite a few files and could be made more consumable if you broke it up a bit further.
> > Are you familiar with support/post-reviews.py ?
> > 
> > General: A high-level question is what to call the default allocator and whether to move it to its own allocators directory (where it can exist along with other allocator implementations).

The RR is a bit big, I agree, but that's the atomic change that breaks neither build, nor tests. An alternative was to cut this into smaller RRs with the assumption that all will be committed together. I try to avoid the latter approach, since a single commit does break the build, which may influence several git workflows, like bisecting for a bug.

I'll submit additional patches to finish the refactoring as separate RR to keep this patch as small as possible.


> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/master/allocator.hpp, line 61
> > <https://reviews.apache.org/r/29890/diff/1/?file=821527#file821527line61>
> >
> >     This will be the public API and it would be great to have comments on all calls to be implemented. Can we leave a todo, so we get that fixed?

TODO added, JIRA filed: https://issues.apache.org/jira/browse/MESOS-2224


> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/master/allocator.hpp, lines 144-145
> > <https://reviews.apache.org/r/29890/diff/1/?file=821527#file821527line144>
> >
> >     Let's do this now.

Let's do it in a separate RR: https://reviews.apache.org/r/29931/


> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/master/allocator.hpp, line 147
> > <https://reviews.apache.org/r/29890/diff/1/?file=821527#file821527line147>
> >
> >     How about calling ActorAllocator the DRFAllocator or MesosAllocator (similiar to the default containerizer)? Think we could come up with a more precise name than ActorAllocator. Hopefully, other allocators are also actors :)

I like `MesosAllocator`, `DRFAllocator` is too specific. This class is a wrapper for libprocess process based allocators, the allocation algorithm can be arbitrary. The base `Allocator` doesn't require implementations to be [libprocess based] actors any more.


> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/tests/cluster.hpp, lines 100-101
> > <https://reviews.apache.org/r/29890/diff/1/?file=821530#file821530line100>
> >
> >     How is this different from before?

No, just postponed it for a separate review to keep things simple: https://reviews.apache.org/r/29926/


> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/tests/master_allocator_tests.cpp, lines 79-82
> > <https://reviews.apache.org/r/29890/diff/1/?file=821532#file821532line79>
> >
> >     So the patch will make tests flaky without a stop() method?

Yes, I would assume that, though I was unable to reproduce the test failure with `--gtest_repeat=10000 --gtest_break_on_failure`. Nevertheless, I restored the logic here: https://reviews.apache.org/r/29927/


> On Jan. 14, 2015, 11:36 p.m., Niklas Nielsen wrote:
> > src/tests/resource_offers_tests.cpp, lines 40-41
> > <https://reviews.apache.org/r/29890/diff/1/?file=821535#file821535line40>
> >
> >     Any particular reason you removed this?

This type used only once, so I opted for a fully-qualified name at the point where the type is used.


- Alexander


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


On Jan. 14, 2015, 6:51 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 6:51 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> [WIP]: more refactoring will be done either in an update to this RR or in a separate RR.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>


Re: Review Request 29890: Refactored allocator interface to support general implementations.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29890/#review68160
-----------------------------------------------------------


Very nice refactor, Alex!

RR comment: This patch touches quite a few files and could be made more consumable if you broke it up a bit further.
Are you familiar with support/post-reviews.py ?

General: A high-level question is what to call the default allocator and whether to move it to its own allocators directory (where it can exist along with other allocator implementations).


src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112324>

    This will be the public API and it would be great to have comments on all calls to be implemented. Can we leave a todo, so we get that fixed?



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112322>

    Let's do this now.



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112319>

    How about calling ActorAllocator the DRFAllocator or MesosAllocator (similiar to the default containerizer)? Think we could come up with a more precise name than ActorAllocator. Hopefully, other allocators are also actors :)



src/master/allocator.hpp
<https://reviews.apache.org/r/29890/#comment112325>

    Did you run into any problems making it owned? If not, can we do it now?



src/tests/cluster.hpp
<https://reviews.apache.org/r/29890/#comment112323>

    How is this different from before?



src/tests/master_allocator_tests.cpp
<https://reviews.apache.org/r/29890/#comment112321>

    So the patch will make tests flaky without a stop() method?



src/tests/master_allocator_tests.cpp
<https://reviews.apache.org/r/29890/#comment112320>

    Let us kill these.



src/tests/resource_offers_tests.cpp
<https://reviews.apache.org/r/29890/#comment112315>

    Any particular reason you removed this?


- Niklas Nielsen


On Jan. 14, 2015, 10:51 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29890/
> -----------------------------------------------------------
> 
> (Updated Jan. 14, 2015, 10:51 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Niklas Nielsen.
> 
> 
> Bugs: MESOS-2213
>     https://issues.apache.org/jira/browse/MESOS-2213
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Introduce a basic Allocator interface that every allocator should implement. This interface does not require allocators to be based on libprocess process. For allocators they do implement allocation logic via an internal libprocess process, a special wrapper is provided. Allocator uses and tests are updated to use Allocator type instead of AllocatorProcess.
> 
> [WIP]: more refactoring will be done either in an update to this RR or in a separate RR.
> 
> 
> Diffs
> -----
> 
>   src/local/local.cpp 76e73a4 
>   src/master/allocator.hpp 224569a 
>   src/master/hierarchical_allocator_process.hpp ccd37b4 
>   src/master/main.cpp 193d53f 
>   src/tests/cluster.hpp 74cedb3 
>   src/tests/hierarchical_allocator_tests.cpp 7c05123 
>   src/tests/master_allocator_tests.cpp 2430622 
>   src/tests/mesos.hpp 591134b 
>   src/tests/mesos.cpp 3b98c69 
>   src/tests/resource_offers_tests.cpp d098e70 
>   src/tests/slave_recovery_tests.cpp 809822e 
> 
> Diff: https://reviews.apache.org/r/29890/diff/
> 
> 
> Testing
> -------
> 
> make distcheck (Ubuntu, OS X)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>