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
>
>