You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@gmail.com> on 2015/09/21 07:02:42 UTC

Re: Review Request 37993: WIP: Add explanatory comments for Allocator interface

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

(Updated 九月 21, 2015, 5:02 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


Changes
-------

rebase


Summary (updated)
-----------------

WIP: Add explanatory comments for Allocator interface


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs (updated)
-----

  3rdparty/libprocess/3rdparty/CMakeLists.txt c746ba51c9985365428e3caaae1327a4f9bfd7d3 
  3rdparty/libprocess/3rdparty/Makefile.am e64e3d9e958fab3c7c25fad17dcc2b279d255500 
  3rdparty/libprocess/3rdparty/picojson-1.3.0.tar.gz 0ade1be899f63a8749e06344f2b30b960648d05e 
  3rdparty/libprocess/3rdparty/picojson-4f93734.tar.gz PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp e36f4b9aed50a75767d29306cb8d4974c4995156 
  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9eaba668d5215c108849055fe92163da4d 
  3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp bc167f1fa9e64f89138f131e726e7733c66da29c 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 88b6147fa47ebfcb6000de5b2f0891d4438d26bf 
  3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 6f154cd5ab12e1d1dd64e56574d2186ae53ee66e 
  3rdparty/libprocess/3rdparty/versions.am 98195b8eb60b2673d610d8ab7ea31103f137debf 
  3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 8f15ee7e89e5346d306df71eb06a7161da13af48 
  docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
  include/mesos/master/allocator.hpp fb09e2a6502bc8c78ddcc8a595bcd9320da136ea 
  include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
  include/mesos/v1/mesos.proto 58e5a6b7912d6f96d495a34d0913b9d27ad65fb5 
  src/Makefile.am 834f69afe87a093c6d275916f7781d470535a728 
  src/common/protobuf_utils.hpp 3817c6a3374b2e8f333784261a0c8edabf854fd9 
  src/common/protobuf_utils.cpp 4dc58fed315d99fd9cdde49e91eab1f4947ef046 
  src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
  src/master/http.cpp 8bb59355e3852617faccad8cc65e1f9dbd7988b4 
  src/master/maintenance.hpp a37412ff928b0313752ca45364c0ac9e14a0f70c 
  src/master/maintenance.cpp 5fe935886a029c7959e07b82ec20cb31b0e7055c 
  src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
  src/slave/containerizer/provisioner/docker/token_manager.cpp 18b29c3d0ddf25fd3f4713c5de10aeec3b381df0 
  src/tests/fault_tolerance_tests.cpp c97bc4691f9bac4a8677e6d2247be96ee9674b57 
  src/tests/hierarchical_allocator_tests.cpp 505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 
  src/tests/master_maintenance_tests.cpp 9892bc329ff850f638e9f5e79d047efaf2fcb037 
  src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
  src/tests/monitor_tests.cpp 583e71183e7bc80608047bd8c323170ebfd95646 
  src/tests/rate_limiting_tests.cpp e512aa690413ee871b56011a9bb1292fa8ab8c33 
  src/tests/registrar_tests.cpp 5131b570e1b94431dfaea74d5f504a29e520b1f1 
  src/tests/slave_tests.cpp dccdbb0ec5352df5da63d5ef7261bfc7fd599acb 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: Add explanatory comments for Allocator interface

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


Per AlexR, I'll review as a native speaker.

General note:
There are quite a few run-on sentences (i.e. using commas instead of periods).  This is fairly common in chinese, but most english word processors will point this out automatically :P


include/mesos/master/allocator.hpp (line 45)
<https://reviews.apache.org/r/37993/#comment157847>

    Add a newline after `/**` (Inconsistent style.)



include/mesos/master/allocator.hpp (line 51)
<https://reviews.apache.org/r/37993/#comment157848>

    s/a/an/



include/mesos/master/allocator.hpp (lines 75 - 78)
<https://reviews.apache.org/r/37993/#comment157854>

    Suggestion:
    
    Any errors in initialization should fail fast and result in an `ABORT`.  The master expects the allocator to be successfully initialized if this call returns.



include/mesos/master/allocator.hpp (lines 86 - 88)
<https://reviews.apache.org/r/37993/#comment157856>

    The roles are actually checked by the master (see `Master::subscribe`).  Instead, you could replace the second sentence with:
    
    All frameworks that are added to the allocator will fall into one of these roles.



include/mesos/master/allocator.hpp (line 104)
<https://reviews.apache.org/r/37993/#comment157858>

    Re-adding a framework will not call this.
    See: https://github.com/apache/mesos/blob/master/src/master/master.cpp#L5370-L5371



include/mesos/master/allocator.hpp (lines 108 - 110)
<https://reviews.apache.org/r/37993/#comment157861>

    Note that this field is non-zero when the master fails-over.  The frameworks report that they are using resources to the recovered master and that gets passed to the allocator.
    
    Suggestion:
    
    Resources used this framework.  The allocator should account for these resources when updating the allocation of this framework.



include/mesos/master/allocator.hpp (lines 130 - 131)
<https://reviews.apache.org/r/37993/#comment157862>

    Suggestion:
    
    Activates a framework in the Mesos cluster.  Offers are only sent to active frameworks.



include/mesos/master/allocator.hpp (lines 139 - 140)
<https://reviews.apache.org/r/37993/#comment157863>

    Suggestion:
    
    Deactivates a framework in the Mesos cluster.  Resource offers are not sent to deactivated frameworks.



include/mesos/master/allocator.hpp (line 150)
<https://reviews.apache.org/r/37993/#comment157866>

    s/can/should/



include/mesos/master/allocator.hpp (line 152)
<https://reviews.apache.org/r/37993/#comment157865>

    I think this doc is a better reference than the JIRA: 
    https://cwiki.apache.org/confluence/display/MESOS/Design+doc:+Updating+Framework+Info
    
    (If it's too long for one line, add ` // NOLINT` at the end.)



include/mesos/master/allocator.hpp (lines 154 - 156)
<https://reviews.apache.org/r/37993/#comment157867>

    `@param` not really necessary.



include/mesos/master/allocator.hpp (line 168)
<https://reviews.apache.org/r/37993/#comment157869>

    Remove "going".



include/mesos/master/allocator.hpp (line 169)
<https://reviews.apache.org/r/37993/#comment157870>

    Period after "Detailed info of the agent".



include/mesos/master/allocator.hpp (line 171)
<https://reviews.apache.org/r/37993/#comment157872>

    s/'total'/`total`/



include/mesos/master/allocator.hpp (lines 186 - 187)
<https://reviews.apache.org/r/37993/#comment157875>

    Suggestion: 
    
    Removes an agent from the Mesos cluster.  All resources belonging to this agent should be released by the allocator.



include/mesos/master/allocator.hpp (lines 188 - 189)
<https://reviews.apache.org/r/37993/#comment157874>

    Not really necessary.



include/mesos/master/allocator.hpp (line 202)
<https://reviews.apache.org/r/37993/#comment157876>

    Not really necessary.



include/mesos/master/allocator.hpp (lines 203 - 205)
<https://reviews.apache.org/r/37993/#comment157877>

    Split the comment into two sentences:
    
    s/,/./



include/mesos/master/allocator.hpp (lines 214 - 215)
<https://reviews.apache.org/r/37993/#comment157878>

    Suggestion: 
    
    Activates an agent.  This is invoked when an agent reregisters.  Offers are only sent for activated agents.



include/mesos/master/allocator.hpp (lines 216 - 217)
<https://reviews.apache.org/r/37993/#comment157879>

    Not necessary.



include/mesos/master/allocator.hpp (lines 225 - 227)
<https://reviews.apache.org/r/37993/#comment157887>

    The second sentence is incorrect:
    * The allocator should treat all offers from the deactivated agent as rescinded.  (There is no separate call to the allocator to handle this).
    * Resources aren't "recovered" when an agent deactivates (because the resources are lost).



include/mesos/master/allocator.hpp (lines 228 - 229)
<https://reviews.apache.org/r/37993/#comment157881>

    Not necessary.



include/mesos/master/allocator.hpp (line 237)
<https://reviews.apache.org/r/37993/#comment157889>

    s/It/This/



include/mesos/master/allocator.hpp (lines 238 - 242)
<https://reviews.apache.org/r/37993/#comment157891>

    These two comments pretty much say the same thing.  I suggest moving merging them.
    
    i.e.
    @param whitelist A set of agents that are allowed to contribute their resources to the resource pool.



include/mesos/master/allocator.hpp (line 250)
<https://reviews.apache.org/r/37993/#comment157892>

    s/an/the/



include/mesos/master/allocator.hpp (line 252)
<https://reviews.apache.org/r/37993/#comment157893>

    s/an/the/



include/mesos/master/allocator.hpp (lines 253 - 255)
<https://reviews.apache.org/r/37993/#comment157894>

    Not necessary.



include/mesos/master/allocator.hpp (line 265)
<https://reviews.apache.org/r/37993/#comment157896>

    s/An/The/



include/mesos/master/allocator.hpp (line 266)
<https://reviews.apache.org/r/37993/#comment157897>

    s/operations, it may/operations.  The allocator should/



include/mesos/master/allocator.hpp (lines 269 - 272)
<https://reviews.apache.org/r/37993/#comment157899>

    Not necessary.



include/mesos/master/allocator.hpp (line 290)
<https://reviews.apache.org/r/37993/#comment157901>

    Suggestion:
    
    Updates unavailability for an agent.



include/mesos/master/allocator.hpp (lines 296 - 299)
<https://reviews.apache.org/r/37993/#comment157900>

    Not necessary.



include/mesos/master/allocator.hpp (lines 311 - 312)
<https://reviews.apache.org/r/37993/#comment157903>

    Not necessary.



include/mesos/master/allocator.hpp (line 332)
<https://reviews.apache.org/r/37993/#comment157904>

    Be careful with your rebase :)



include/mesos/master/allocator.hpp (line 338)
<https://reviews.apache.org/r/37993/#comment157906>

    s/Recover/Recovers/



include/mesos/master/allocator.hpp (lines 340 - 344)
<https://reviews.apache.org/r/37993/#comment157910>

    Suggestion:
    
    Used to update the set of available resources for a specific agent.  This method is invoked to inform the allocator about allocated resources that have been refused or are no longer in use.



include/mesos/master/allocator.hpp (lines 345 - 351)
<https://reviews.apache.org/r/37993/#comment157907>

    Not necessary.



include/mesos/master/allocator.hpp (lines 362 - 363)
<https://reviews.apache.org/r/37993/#comment157913>

    Suggestion:
    
    Revives offers for a framework.  This is invoked when by a framework when it wishes to receive filtered resources or offers immediately.



include/mesos/master/allocator.hpp (lines 364 - 365)
<https://reviews.apache.org/r/37993/#comment157914>

    Not necessary.



include/mesos/master/allocator.hpp (line 373)
<https://reviews.apache.org/r/37993/#comment157915>

    s/resources/offers/
    s/for/to/


- Joseph Wu


On Sept. 25, 2015, 10:22 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:22 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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


Patch looks great!

Reviews applied: [37993]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 5:22 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 5:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

Ship it!


This is looking very good now!

Thanks for being patient with our comments :)


include/mesos/master/allocator.hpp (line 154)
<https://reviews.apache.org/r/37993/#comment158904>

    Typo: s/desing/design/



include/mesos/master/allocator.hpp (line 340)
<https://reviews.apache.org/r/37993/#comment158906>

    Missing a period.


- Joseph Wu


On Oct. 3, 2015, 9:24 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Oct. 3, 2015, 9:24 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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


Patch looks great!

Reviews applied: [37993]

All tests passed.

- Mesos ReviewBot


On Oct. 4, 2015, 4:24 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Oct. 4, 2015, 4:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

> On 十月 12, 2015, 6:15 p.m., Michael Park wrote:
> > I've shipped this patch with the minor typos and style fixes outlined below.

Thanks Michael Park for the update ;-)


- Guangya


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


On 十月 6, 2015, 12:26 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated 十月 6, 2015, 12:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37993/#review102266
-----------------------------------------------------------

Ship it!


I've shipped this patch with the minor typos and style fixes outlined below.


include/mesos/master/allocator.hpp (lines 111 - 112)
<https://reviews.apache.org/r/37993/#comment159849>

    Indent 4 spaces.



include/mesos/master/allocator.hpp (line 153)
<https://reviews.apache.org/r/37993/#comment159850>

    `s/new/newly/`



include/mesos/master/allocator.hpp (line 229)
<https://reviews.apache.org/r/37993/#comment159858>

    `s/a/the/`



include/mesos/master/allocator.hpp (line 231)
<https://reviews.apache.org/r/37993/#comment159859>

    `s/invokved/invoked/`
    `s/a whitelist flag/the --whitelist flag`



include/mesos/master/allocator.hpp (line 254)
<https://reviews.apache.org/r/37993/#comment159861>

    `s/reservationa/reservation/`



include/mesos/master/allocator.hpp (line 333)
<https://reviews.apache.org/r/37993/#comment159874>

    `s/when//`



include/mesos/master/allocator.hpp (line 342)
<https://reviews.apache.org/r/37993/#comment159877>

    `s/for/to/`


- Michael Park


On Oct. 6, 2015, 12:26 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 12:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37993/#review101566
-----------------------------------------------------------

Ship it!


Ship It!

- haosdent huang


On Oct. 6, 2015, 12:26 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 12:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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


Patch looks great!

Reviews applied: [37993]

All tests passed.

- Mesos ReviewBot


On Oct. 6, 2015, 12:26 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 12:26 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

(Updated 十月 6, 2015, 12:26 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs (updated)
-----

  include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

(Updated 十月 4, 2015, 4:24 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


Changes
-------

Addressed Joseph and Alex's comments. Thanks both for the detailed review!


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs (updated)
-----

  include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

(Updated 九月 25, 2015, 5:22 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs (updated)
-----

  include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: Add explanatory comments for Allocator interface

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


Patch looks great!

Reviews applied: [37993]

All tests passed.

- Mesos ReviewBot


On Sept. 25, 2015, 4:21 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 4:21 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

(Updated 九月 25, 2015, 4:21 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


Changes
-------

Rebase


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs (updated)
-----

  include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: Add explanatory comments for Allocator interface

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


Patch looks great!

Reviews applied: [37993]

All tests passed.

- Mesos ReviewBot


On Sept. 21, 2015, 7:28 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 7:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > Looking almost perfect! Really appreciate that you diligently address all the comments I throw into you and don't give up.
> > 
> > While you're touching this file, I would say it makes sense to clean up everything we can in one patch. Here are some suggestions:
> > - Wrap all types and variables in comments in backticks;
> > - As I've mentioned in one of my previous reviews, let's use the same tense everywhere in the commens for consistency;
> > - Not a native speaker, but I think we should consolidate the use of articles when referencing allocator in comments. I think "an allocator" makes more sense since it is explicitly indicates that it's implementation dependent and not tied to a particular implementation;
> > - Again, not a native speaker, but let's refer to a framework as "which" or "that", not "who".

Thanks Alex for the review, I found that there are new interfaces added to allocator and will add comments for the new interface later. You can take a look the updated patch to see if there are anything need improvement. ;-)


- Guangya


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


On 九月 25, 2015, 5:22 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated 九月 25, 2015, 5:22 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

> On Sept. 24, 2015, 9:35 a.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, lines 60-61
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line60>
> >
> >     Not yours, but let's wrap all types and variable names in backticks "`"
> 
> Guangya Liu wrote:
>     Not quite catch your point, can you please show more detail for what does this mean?

i.e. 
s/Try/`Try`/
s/Allocator*/`Allocator*`/


- Joseph


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


On Sept. 25, 2015, 10:22 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 25, 2015, 10:22 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, line 54
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line54>
> >
> >     I think you killed that line in previous versions of the review, why do you want to bring it back?

removed.


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, lines 60-61
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line60>
> >
> >     Not yours, but let's wrap all types and variable names in backticks "`"

Not quite catch your point, can you please show more detail for what does this mean?


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, line 291
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line291>
> >
> >     s/time/interval

I was using "time interval" here, hope it is OK.


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, lines 270-271
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line270>
> >
> >     Again, I'm afraid the next engineer to update the set of offer operations will forget to update this list. Do you think it's valuable to keep this list here explicitly?

Agree,  I have removed the list here.


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, lines 151-152
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line151>
> >
> >     I'm a bit hesitant to put a reference to the built-in allocator here. The behaviour you describe is correct, but 
> >     1) It's how the built-in allocator works
> >     2) It's how the built-in allocator works now: when it the behaviour changes, my bet is that this comment won't be updated, hence it will become misleading.
> >     
> >     What do you think?

Yes, I have removed this part.


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, line 162
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line162>
> >
> >     s/new agent joins the Mesos cluster or in the case of a agent recovery./new agent joins the cluster or in case of agent recovery.
> >     
> >     not a native speaker though

updated.


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, line 183
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line183>
> >
> >     s/should/are

I was replacing "should be" to "is", hope it is ok.


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, line 193
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line193>
> >
> >     What do you mean by "new" here? New feature or new update?

new update, updated the comments here as: Updates the latest ....


> On 九月 24, 2015, 4:35 p.m., Alexander Rukletsov wrote:
> > include/mesos/master/allocator.hpp, lines 247-250
> > <https://reviews.apache.org/r/37993/diff/10/?file=1077928#file1077928line247>
> >
> >     Suggestion:
> >     
> >     A framework may request resources via this call. It is up to an allocator how to react to this request. For example, a request may be ignored, or may influence internal priorities an allocator may keep for frameworks.

Done.


- Guangya


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


On 九月 25, 2015, 4:21 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated 九月 25, 2015, 4:21 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 8100f14220599247a340c919a03f29755b5349d8 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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


Looking almost perfect! Really appreciate that you diligently address all the comments I throw into you and don't give up.

While you're touching this file, I would say it makes sense to clean up everything we can in one patch. Here are some suggestions:
- Wrap all types and variables in comments in backticks;
- As I've mentioned in one of my previous reviews, let's use the same tense everywhere in the commens for consistency;
- Not a native speaker, but I think we should consolidate the use of articles when referencing allocator in comments. I think "an allocator" makes more sense since it is explicitly indicates that it's implementation dependent and not tied to a particular implementation;
- Again, not a native speaker, but let's refer to a framework as "which" or "that", not "who".


include/mesos/master/allocator.hpp (lines 45 - 51)
<https://reviews.apache.org/r/37993/#comment157571>

    While you're touching this file, let's doxygenify this one as well.



include/mesos/master/allocator.hpp (line 54)
<https://reviews.apache.org/r/37993/#comment157570>

    I think you killed that line in previous versions of the review, why do you want to bring it back?



include/mesos/master/allocator.hpp (lines 60 - 61)
<https://reviews.apache.org/r/37993/#comment157569>

    Not yours, but let's wrap all types and variable names in backticks "`"



include/mesos/master/allocator.hpp (lines 74 - 75)
<https://reviews.apache.org/r/37993/#comment157583>

    Mind switching to Present Simple here for consistency? s/will be/is



include/mesos/master/allocator.hpp (line 75)
<https://reviews.apache.org/r/37993/#comment157572>

    I'm not sure `initialize()` can fail: it returns nothing and we do not throw exceptions. I see two possibilities here:
    - an allocator uses an exception, which is not caught in the master => master fails over;
    - an allocator uses `CHECK*` macros for error situations => master fails over.
    
    Could you please rephrase that in order not to mislead the reader?



include/mesos/master/allocator.hpp (line 78)
<https://reviews.apache.org/r/37993/#comment157584>

    s/perform the allocation/perform the batch allocation/
    
    An allocator may also perform allocation based on events (a framework is added and so on). However, it's up to the implementation.



include/mesos/master/allocator.hpp (lines 151 - 152)
<https://reviews.apache.org/r/37993/#comment157591>

    I'm a bit hesitant to put a reference to the built-in allocator here. The behaviour you describe is correct, but 
    1) It's how the built-in allocator works
    2) It's how the built-in allocator works now: when it the behaviour changes, my bet is that this comment won't be updated, hence it will become misleading.
    
    What do you think?



include/mesos/master/allocator.hpp (line 159)
<https://reviews.apache.org/r/37993/#comment157594>

    s/a/an



include/mesos/master/allocator.hpp (line 161)
<https://reviews.apache.org/r/37993/#comment157595>

    s/will be/is



include/mesos/master/allocator.hpp (line 162)
<https://reviews.apache.org/r/37993/#comment157596>

    s/new agent joins the Mesos cluster or in the case of a agent recovery./new agent joins the cluster or in case of agent recovery.
    
    not a native speaker though



include/mesos/master/allocator.hpp (line 180)
<https://reviews.apache.org/r/37993/#comment157597>

    s/a/an
    
    here and below.



include/mesos/master/allocator.hpp (line 183)
<https://reviews.apache.org/r/37993/#comment157598>

    s/should/are



include/mesos/master/allocator.hpp (line 191)
<https://reviews.apache.org/r/37993/#comment157599>

    s/a/an



include/mesos/master/allocator.hpp (line 193)
<https://reviews.apache.org/r/37993/#comment157600>

    What do you mean by "new" here? New feature or new update?



include/mesos/master/allocator.hpp (line 208)
<https://reviews.apache.org/r/37993/#comment157601>

    s/a/an
    
    here and below



include/mesos/master/allocator.hpp (line 210)
<https://reviews.apache.org/r/37993/#comment157602>

    s/will be/is
    s/when re-register a agent/when an agent reregisters



include/mesos/master/allocator.hpp (line 213)
<https://reviews.apache.org/r/37993/#comment157604>

    s/going to be/being



include/mesos/master/allocator.hpp (line 219)
<https://reviews.apache.org/r/37993/#comment157605>

    s/a/an
    
    here and below



include/mesos/master/allocator.hpp (lines 221 - 223)
<https://reviews.apache.org/r/37993/#comment157609>

    How about this:
    
    This is triggered if an agent disconnects from the master. Outstanding offers on a deactivated agent are removed and resources are recovered in a separate call.



include/mesos/master/allocator.hpp (line 232)
<https://reviews.apache.org/r/37993/#comment157610>

    How about this one, should be more readable:
    
    Updates a list of trusted agents



include/mesos/master/allocator.hpp (lines 234 - 237)
<https://reviews.apache.org/r/37993/#comment157611>

    How about this:
    
    It is invokved when the master starts up with a whitelist flag. In this case an allocator must allocate resources only to the hosts in the whitelist.



include/mesos/master/allocator.hpp (line 239)
<https://reviews.apache.org/r/37993/#comment157614>

    Or:
    
    A whitelist of agents that are allowed to contribute their resources to the allocation pool.
    
    (agents do not offer resources to frameworks directly, let's avoid misleading comments)



include/mesos/master/allocator.hpp (line 245)
<https://reviews.apache.org/r/37993/#comment157615>

    Suggestion: Requests resources for a framework.



include/mesos/master/allocator.hpp (lines 247 - 250)
<https://reviews.apache.org/r/37993/#comment157616>

    Suggestion:
    
    A framework may request resources via this call. It is up to an allocator how to react to this request. For example, a request may be ignored, or may influence internal priorities an allocator may keep for frameworks.



include/mesos/master/allocator.hpp (line 260)
<https://reviews.apache.org/r/37993/#comment157617>

    s/Update/Updates



include/mesos/master/allocator.hpp (lines 260 - 265)
<https://reviews.apache.org/r/37993/#comment157618>

    Suggestion:
    
    Updates allocation by applying offer operations.
    
    This call is mainly intended to support persistence-related features (dynamic reservationa and persistent volumes). An allocator may react differently for certain offer operations, it may use this call to update bookkeeping information related to the framework.



include/mesos/master/allocator.hpp (line 267)
<https://reviews.apache.org/r/37993/#comment157621>

    s/who has just got resources./which reacted to an offer.



include/mesos/master/allocator.hpp (lines 270 - 271)
<https://reviews.apache.org/r/37993/#comment157623>

    Again, I'm afraid the next engineer to update the set of offer operations will forget to update this list. Do you think it's valuable to keep this list here explicitly?



include/mesos/master/allocator.hpp (line 279)
<https://reviews.apache.org/r/37993/#comment157629>

    Let's kill this for now, it doesn't really add extra information.



include/mesos/master/allocator.hpp (line 291)
<https://reviews.apache.org/r/37993/#comment157630>

    s/time/interval



include/mesos/master/allocator.hpp (line 299)
<https://reviews.apache.org/r/37993/#comment157631>

    Unfinished sentence?



include/mesos/master/allocator.hpp (lines 355 - 361)
<https://reviews.apache.org/r/37993/#comment157633>

    Suggestion:
    
    Revives offers for a framework.
    
    It is invoked when a framework that has filtered resources before wants to revive offers for those resources.
    
    @param frameworkId ID of the framework which wants to revive offers.



include/mesos/master/allocator.hpp (lines 366 - 369)
<https://reviews.apache.org/r/37993/#comment157632>

    It's resolved, isn't it?


- Alexander Rukletsov


On Sept. 21, 2015, 7:28 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 7:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

Posted by Jian Qiu <qi...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37993/#review99732
-----------------------------------------------------------



include/mesos/master/allocator.hpp (line 80)
<https://reviews.apache.org/r/37993/#comment156712>

    needs adding explanation for @param inverseOfferCallback


- Jian Qiu


On Sept. 21, 2015, 7:28 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 7:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Re: Review Request 37993: Add explanatory comments for Allocator interface

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

(Updated 九月 21, 2015, 7:28 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


Summary (updated)
-----------------

Add explanatory comments for Allocator interface


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs
-----

  include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: WIP: Add explanatory comments for Allocator interface

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

(Updated 九月 21, 2015, 7:25 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs (updated)
-----

  include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: WIP: Add explanatory comments for Allocator interface

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

(Updated 九月 21, 2015, 6:20 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.


Changes
-------

rebase


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


Repository: mesos


Description
-------

Add explanatory comments for Allocator interface


Diffs (updated)
-----

  include/mesos/master/allocator.hpp 3fea47ffcc69531308068e2701502e481605b912 

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


Testing
-------


Thanks,

Guangya Liu


Re: Review Request 37993: WIP: Add explanatory comments for Allocator interface

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


Bad patch!

Reviews applied: [37993]

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

Error:
 2015-09-21 05:27:48 URL:https://reviews.apache.org/r/37993/diff/raw/ [92103/92103] -> "37993.patch" [1]
error: patch failed: 3rdparty/libprocess/3rdparty/CMakeLists.txt:35
error: 3rdparty/libprocess/3rdparty/CMakeLists.txt: patch does not apply
error: patch failed: include/mesos/master/allocator.hpp:159
error: include/mesos/master/allocator.hpp: patch does not apply
error: patch failed: src/master/master.cpp:785
error: src/master/master.cpp: patch does not apply
error: patch failed: src/tests/master_maintenance_tests.cpp:1026
error: src/tests/master_maintenance_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Sept. 21, 2015, 5:02 a.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37993/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2015, 5:02 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Michael Park.
> 
> 
> Bugs: MESOS-2224
>     https://issues.apache.org/jira/browse/MESOS-2224
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Add explanatory comments for Allocator interface
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt c746ba51c9985365428e3caaae1327a4f9bfd7d3 
>   3rdparty/libprocess/3rdparty/Makefile.am e64e3d9e958fab3c7c25fad17dcc2b279d255500 
>   3rdparty/libprocess/3rdparty/picojson-1.3.0.tar.gz 0ade1be899f63a8749e06344f2b30b960648d05e 
>   3rdparty/libprocess/3rdparty/picojson-4f93734.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp e36f4b9aed50a75767d29306cb8d4974c4995156 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9eaba668d5215c108849055fe92163da4d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp bc167f1fa9e64f89138f131e726e7733c66da29c 
>   3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 88b6147fa47ebfcb6000de5b2f0891d4438d26bf 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 6f154cd5ab12e1d1dd64e56574d2186ae53ee66e 
>   3rdparty/libprocess/3rdparty/versions.am 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   3rdparty/libprocess/Makefile.am c764717d447da39f78a9c74a756e611b63a814e1 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 8f15ee7e89e5346d306df71eb06a7161da13af48 
>   docs/mesos-c++-style-guide.md 570ea6873ffc2cc8ec190ec3aa81986e3f3cbc95 
>   include/mesos/master/allocator.hpp fb09e2a6502bc8c78ddcc8a595bcd9320da136ea 
>   include/mesos/mesos.proto 4a16be1f570769f3ce42a50a9da9f4fb1c227999 
>   include/mesos/v1/mesos.proto 58e5a6b7912d6f96d495a34d0913b9d27ad65fb5 
>   src/Makefile.am 834f69afe87a093c6d275916f7781d470535a728 
>   src/common/protobuf_utils.hpp 3817c6a3374b2e8f333784261a0c8edabf854fd9 
>   src/common/protobuf_utils.cpp 4dc58fed315d99fd9cdde49e91eab1f4947ef046 
>   src/docker/docker.cpp afcedf1f1a309bd0626c33ee25694ac1b43bdec7 
>   src/master/http.cpp 8bb59355e3852617faccad8cc65e1f9dbd7988b4 
>   src/master/maintenance.hpp a37412ff928b0313752ca45364c0ac9e14a0f70c 
>   src/master/maintenance.cpp 5fe935886a029c7959e07b82ec20cb31b0e7055c 
>   src/master/master.cpp ca4d5876dcd427964111428edc22d567ddaede0b 
>   src/slave/containerizer/provisioner/docker/token_manager.cpp 18b29c3d0ddf25fd3f4713c5de10aeec3b381df0 
>   src/tests/fault_tolerance_tests.cpp c97bc4691f9bac4a8677e6d2247be96ee9674b57 
>   src/tests/hierarchical_allocator_tests.cpp 505b9de3d8d888c296f6103c80fe9f0ef1c2ca16 
>   src/tests/master_maintenance_tests.cpp 9892bc329ff850f638e9f5e79d047efaf2fcb037 
>   src/tests/master_tests.cpp a477794df37c658b80d79dea8555b001415f7b6a 
>   src/tests/monitor_tests.cpp 583e71183e7bc80608047bd8c323170ebfd95646 
>   src/tests/rate_limiting_tests.cpp e512aa690413ee871b56011a9bb1292fa8ab8c33 
>   src/tests/registrar_tests.cpp 5131b570e1b94431dfaea74d5f504a29e520b1f1 
>   src/tests/slave_tests.cpp dccdbb0ec5352df5da63d5ef7261bfc7fd599acb 
> 
> Diff: https://reviews.apache.org/r/37993/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>