Re: Review Request 54836: Added helpers to allocate / unallocate Resources.

[Michael Park]

> On Jan. 25, 2017, 1:54 a.m., Michael Park wrote:
> > include/mesos/resources.hpp, lines 332-339
> > 
> >
> > I would've expected:
> > ```cpp
> > void allocate(const std::string& role);
> > void unallocate();
> > ```
> > 
> > Could you help me understand what the `bool` from `unallocate` is for?
> > 
> > If it's simply to indicate whether the function had any effects or
> > not, `allocate` could also return a `bool`, which returns `false`
> > when the resources were already allocated for the specified role.
> 
> Guangya Liu wrote:
> For `allocate`, seems the question here is same as mine above: In which 
> case shall we need to overwrite the role for allocation?
> 
> Guangya Liu wrote:
> @mpark, I think that Ben already posted some comments here for why return 
> `bool` for `unallocate` at https://reviews.apache.org/r/54836/#comment230598

@bmahler, Actually, upon looking further in the chain I realized that mutating 
the resources here is new...
You had mentioned this to me as well and explained why (I think), could you 
explain again here
why we didn't do `allocations` (which we already have), `allocated` and 
`unallocated`?
Similar to `reservations`, `reserved` and `unreserved`?


- Michael


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


On Jan. 22, 2017, 5:55 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54836/
> ---
> 
> (Updated Jan. 22, 2017, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helpers to allocate / unallocate Resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412 
>   include/mesos/v1/resources.hpp 98f025b68c818c384a1174ef9b969f5c0776a7f1 
>   src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45 
>   src/tests/resources_tests.cpp 8dfb1be35d9f9c6ff69139d055c6b3d3ec475e68 
>   src/v1/resources.cpp da4701c03020ff9c33ef995cd0af437d8827c267 
> 
> Diff: https://reviews.apache.org/r/54836/diff/
> 
> 
> Testing
> ---
> 
> Updated the existing allocation test to incorporate the new helper.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55863: Introduce a helper for injecting AllocationInfo into offer operations.

[Guangya Liu]

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




src/common/protobuf_utils.cpp (lines 345 - 347)


Since we do not support one `Resources` store a mix of allocated and 
unallocated resources, how about optimize this a bit as:

```
if (resource->has_allocation_info()) {
  break;
}

resource->mutable_allocation_info()->CopyFrom(allocationInfo);
```

Ditto for here and everywhere



src/tests/protobuf_utils_tests.cpp (lines 101 - 103)


How about make this less jagged

```
  // Test the LAUNCH_GROUP case. This should be constructing a valid
  // task and executor, but for now this just sets the resources in
  // order to verify the allocation info injection.
```

Ditto here and following comments.


- Guangya Liu


On 一月 23, 2017, 10:59 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55863/
> ---
> 
> (Updated 一月 23, 2017, 10:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6967
> https://issues.apache.org/jira/browse/MESOS-6967
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `Resource` did not contain `AllocationInfo`. So for
> backwards compatibility with old schedulers and tooling, we must
> allow operations to contain `Resource`s without an `AllocationInfo`.
> 
> This introduces a function which allows the master to inject the
> offer's `AllocationInfo` into the operation's resources.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp faa7e2a759fd2b1ce5def662679f573ec6fefd28 
>   src/common/protobuf_utils.cpp dd20759affe3adf2867a33bf875c9ee82862038d 
>   src/tests/protobuf_utils_tests.cpp bc2a3d0ebe544a58d0de8f2f7174c170113ddf2e 
> 
> Diff: https://reviews.apache.org/r/55863/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55870: Update the allocator to handle frameworks with multiple roles.

[Guangya Liu]

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




include/mesos/allocator/allocator.hpp (line 91)


I think that we also need to reflect this in CHANGELOG to clarify that this 
interface was updated for multi_role support.



src/master/allocator/mesos/hierarchical.hpp (line 232)


How about update the comments a bit?
```
// Remove an offer filter for the specified role of a framework.
```



src/master/allocator/mesos/hierarchical.hpp (lines 251 - 252)


How about update the comments a bit?
```
// Returns true if there is a resource offer filter for the
// speficied role of the framework on this slave.
```



src/master/allocator/mesos/hierarchical.hpp (line 283)


I think that the reason you want to update here is want to keep consistent, 
right?



src/master/allocator/mesos/hierarchical.cpp (line 428)


s/a new 'role'/new 'roles'



src/master/allocator/mesos/hierarchical.cpp (lines 1124 - 1128)


Add `role` here in the log message?



src/master/allocator/mesos/hierarchical.cpp (lines 1124 - 1129)


Add `role` here in the log message?



src/master/allocator/mesos/hierarchical.cpp (lines 1167 - 1168)


A question here: Why not call `resources.unallocate()` at #1130? 

This can make the logic of `allocate` and `recoverResources` as pair: 
`allocate` will set allocation info while `recoverResources` will clear 
allocatio info.



src/master/allocator/mesos/hierarchical.cpp (lines 1219 - 1220)


Nit, how about update the message a bit:

```
Suppressed offers for roles {r1,r2} in framework xx-xx-xx-xx
```

Ditto for the log message in `reviveOffers`.



src/master/allocator/mesos/hierarchical.cpp (line 1440)


s/role/roles



src/master/allocator/mesos/hierarchical.cpp (line 1480)


Why highlight the `unallocate()` here, I think that this was already done 
via `Resources::createStrippedScalarQuantity` and the resources being `flatten` 
here should not have `allocation info`.



src/master/allocator/mesos/hierarchical.cpp (lines 2044 - 2046)


Adding `role` in the log message here to clarify which role on this 
framework is being filtered?



src/master/allocator/mesos/hierarchical.cpp (lines 2044 - 2046)


Add `role` in the log message here to clarify which role on this framework 
is being filtered?


- Guangya Liu


On 一月 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> ---
> 
> (Updated 一月 25, 2017, 9:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
> 
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 558594983beb6ff49c1fddf875ba29c1983b1288 
>   src/master/allocator/mesos/allocator.hpp 
> 8e0f37a5eedd4d71d765991c0039b49c96f0ca53 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9b66c23f26b37c02ed850c06c4518ea99078b02d 
>   src/master/allocator/mesos/hierarchical.cpp 
> c2211be7458755aeb91ef078e4bfe92ac474044a 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
>   src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558 
> 
> Diff: https://reviews.apache.org/r/55870/diff/
> 
> 
> Testing
> ---
> 
> Adjustments to existing tests are split out into a subsequent patch.

Re: Review Request 55901: [WIP] Added support for command health checks to the default executor.

[Gastón Kleiman]

> On Jan. 28, 2017, 1:17 a.m., Vinod Kone wrote:
> > src/checks/health_checker.hpp, line 137
> > 
> >
> > we don't typically do `const` for POD types.
> > 
> > s/_agentSpawnsCommandContainer/

Looks lke the regex was truncated ;-).


- Gastón


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


On Jan. 25, 2017, 12:33 a.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55901/
> ---
> 
> (Updated Jan. 25, 2017, 12:33 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6280
> https://issues.apache.org/jira/browse/MESOS-6280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [WIP] Added support for command health checks to the default executor.
> 
> This is still a work in progress, but I am posting it in order to get some 
> feedback on the general approach.
> 
> Open questions/issues:
> 
>   - The code that handles communication with the Agent is on the 
> `DefaultExecutor`, so the health checker calls `evolve` when serializing the 
> API calls. Should it use the v1 protos instead, and skip the `evolve` call?
>   - CMD health checks for command tasks inherit the env from the executor, 
> but allow overriding env variables. I don't think that inheriting the 
> executor env makes sense for cmd health checks launched by the 
> `DefaultExecutor` - each task can have its own env, so the health check 
> command should inherit the task env and merge it with the one in the check's 
> `CommandInfo`.
> I added some code that takes the task env from `TaskInfo` and merges it 
> check's env, but I don't think that this is an ideal approach, since it will 
> miss env variables set/modified by containerizers/hooks/isolators. Do you 
> think that it would make sense for all Debug Containers to inherit the 
> environment of the parent container?
>   - There's a TODO for stopping/resuming health checking when the 
> `DefaultExecutor` is disconnected from the agent. After talking to AlexR, I 
> believe that we should do this for all types of checks, not just for `CMD`.
>   - The test that I introduced passes on Linux, but not on macOS, I still 
> have to do some more debugging.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp 6e558f2061a9e31157c47d31cb64b3a8568aace3 
>   src/checks/health_checker.cpp 50aa2858e807b27bbab58a3618f5200cfe4eca9e 
>   src/launcher/default_executor.cpp a03794934adb93868734f8cf00b337a1bff9b5ab 
>   src/tests/health_check_tests.cpp debbd3c09b7555145aaf3f62a24d795d1423a269 
> 
> Diff: https://reviews.apache.org/r/55901/diff/
> 
> 
> Testing
> ---
> 
> Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
> passes on Linux, but not on macOS.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55901: Added support for command health checks to the default executor.

[Gastón Kleiman]

> On Jan. 28, 2017, 1:17 a.m., Vinod Kone wrote:
> > src/checks/health_checker.cpp, line 480
> > 
> >
> > launch nested container session returns a streaming response, how come 
> > you are calling `post()` helper here which expects a non-streaming response?
> > 
> > probably one of the reasons why your test is hanging.

The `post()` helper delegates to `process::http::request()`, which takes 
boolean flag (`streamedResponse`). If this flag is set to `false`, libprocess 
will convert a PIPE (streaming) response into a BODY (non-streaming) response. 
This means that the `Future` returned by the helper will not be completed until 
the server closes the connection.

Relevant links:
 - 
https://github.com/apache/mesos/blob/2d0195eed54feac41485fb1503dc4004e5500c81/3rdparty/libprocess/include/process/http.hpp#L953-L955
 - 
https://github.com/apache/mesos/blob/2d0195eed54feac41485fb1503dc4004e5500c81/3rdparty/libprocess/src/http.cpp#L1191-L1197
 - 
https://github.com/apache/mesos/blob/2d0195eed54feac41485fb1503dc4004e5500c81/3rdparty/libprocess/src/http.cpp#L1005-L1022

In these particular cases, it means that the `Future` will not be completed 
until the container exits, which is exactly what we need.


Regarding the test hanging in Linux, some further debugging seem to indicate 
that test hangs on `process::Clock::settle()`, because there's a race + 
deadlock in `RateLimiter` that leaves a process stuck in `RUNNING`. I'll dig 
deeper on Monday, but here's some evidence:

# Snippet from a successful test run that "leaks" a running process

```
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from HealthCheckTest
[ RUN  ] HealthCheckTest.DefaultExecutorCmdHealthCheck

[...]

E0127 11:18:06.607446  8665 limiter.hpp:123]  LIMITER _acquire
E0127 11:18:06.607471  8665 limiter.hpp:129]  LIMITER There are 1 promises
E0127 11:18:06.608064  8665 limiter.hpp:186]  LIMITER destroy

 DEADLOCK DETECTED! 
You are waiting on process __limiter__(1419)@192.99.40.208:41947 that it is 
currently executing.

[...]

[   OK ] HealthCheckTest.DefaultExecutorCmdHealthCheck (592 ms)
[--] 1 test from HealthCheckTest (593 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (603 ms total)
[  PASSED  ] 1 test.

Repeating all tests (iteration 474) . . .
```

# Snippet from a hung run

```
[...]
E0127 11:18:06.878636  8640 cluster.cpp:358] !!! Settling the clock
E0127 11:18:06.878646  8640 process.cpp:3491] !!! Attempting to acquire mutex
E0127 11:18:06.878654  8640 process.cpp:3494] !!! !runq.empty()?
E0127 11:18:06.878659  8640 process.cpp:3502] !!! running.load() > 0?
E0127 11:18:06.878667  8640 process.cpp:3504] !!! 1 processes still running
E0127 11:18:06.878676  8640 process.cpp:3509]  Process: 
__authentication_router__(1) state: 3
E0127 11:18:06.878684  8640 process.cpp:3509]  Process: 
__basic_authenticator__(1893) state: 3
E0127 11:18:06.878691  8640 process.cpp:3509]  Process: 
__basic_authenticator__(1894) state: 3
E0127 11:18:06.878697  8640 process.cpp:3509]  Process: 
__basic_authenticator__(1895) state: 3
E0127 11:18:06.878705  8640 process.cpp:3509]  Process: __gc__ state: 3
E0127 11:18:06.878711  8640 process.cpp:3509]  Process: __limiter__(1419) 
state: 2
E0127 11:18:06.878718  8640 process.cpp:3509]  Process: __processes__ 
state: 3
E0127 11:18:06.878725  8640 process.cpp:3509]  Process: __reaper__(1) 
state: 3
E0127 11:18:06.878731  8640 process.cpp:3509]  Process: 
crammd5-authenticator(474) state: 3
E0127 11:18:06.878738  8640 process.cpp:3509]  Process: files state: 3
E0127 11:18:06.878744  8640 process.cpp:3509]  Process: help state: 3
E0127 11:18:06.878751  8640 process.cpp:3509]  Process: 
hierarchical-allocator(474) state: 3
E0127 11:18:06.878757  8640 process.cpp:3509]  Process: 
in-memory-storage(474) state: 3
E0127 11:18:06.878764  8640 process.cpp:3509]  Process: 
local-authorizer(947) state: 3
E0127 11:18:06.878772  8640 process.cpp:3509]  Process: logging state: 3
E0127 11:18:06.878777  8640 process.cpp:3509]  Process: master state: 3
E0127 11:18:06.878792  8640 process.cpp:3509]  Process: metrics state: 3
E0127 11:18:06.878800  8640 process.cpp:3509]  Process: profiler state: 3
E0127 11:18:06.878806  8640 process.cpp:3509]  Process: registrar(474) 
state: 3
E0127 11:18:06.878813  8640 process.cpp:3509]  Process: 
standalone-master-detector(1420) state: 3
E0127 11:18:06.878820  8640 process.cpp:3509]  Process: system state: 3
E0127 11:18:06.878828  8640 process.cpp:3509]  Process: version state: 3
E0127 11:18:06.878834  8640 process.cpp:3509]  Process: whitelist(474) 
state: 3
E0127 11:18:06.878840  8640 process.cpp:3491] !!! Attempting to acquire mutex
E0127 11:18:06.878846  8640 proces

Re: Review Request 55901: Added support for command health checks to the default executor.

[Gastón Kleiman]

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

(Updated Jan. 28, 2017, 12:39 p.m.)


Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent huang, 
and Vinod Kone.


Changes
---

Addressed Vinod's feedback.


Summary (updated)
-

Added support for command health checks to the default executor.


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


Repository: mesos


Description (updated)
---

Added support for command health checks to the default executor.


Diffs (updated)
-

  src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 
  src/checks/health_checker.cpp e70bd7936752613a4f92c70c4c61cd7cdf7c4ee5 
  src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
  src/tests/health_check_tests.cpp 710cb66eff6c4447caa22772f0cdc97cfa582c50 

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


Testing
---

Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
passes on Linux, but not on macOS.


Thanks,

Gastón Kleiman

Re: Review Request 55967: Update the allocator unit tests to reflect MULTI_ROLE support.

[Guangya Liu]

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




src/tests/hierarchical_allocator_tests.cpp (line 3933)


Need call `allocation.allocate("*");` here to make sure the agent can be 
added successfully.



src/tests/hierarchical_allocator_tests.cpp (line 3934)


Need call `allocation.allocate("*");` here to make sure the agent can be 
added successfully.



src/tests/hierarchical_allocator_tests.cpp (line 3934)


Need call `allocation.allocate("*");` here to make sure the agent can be 
added successfully.



src/tests/hierarchical_allocator_tests.cpp (line 4084)


Need call `allocation.allocate("*");` here to make sure the agent can be 
added successfully.



src/tests/hierarchical_allocator_tests.cpp (lines 4188 - 4189)


Why moving this here?



src/tests/hierarchical_allocator_tests.cpp (line 4243)


Need call `allocation.allocate("*");` here to make sure the agent can be 
added successfully.


- Guangya Liu


On 一月 26, 2017, 12:46 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55967/
> ---
> 
> (Updated 一月 26, 2017, 12:46 a.m.)
> 
> 
> Review request for mesos, Guangya Liu and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update the allocator unit tests to reflect MULTI_ROLE support.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 1edd0ecc8a93cd41532e1cf3641f67c780ab23a5 
> 
> Diff: https://reviews.apache.org/r/55967/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note that the rest of the tests do not pass until after my subsequent patches
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55968: Added TODOs to make the master code safer.

[Guangya Liu]

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


Ship it!




Ship It!

- Guangya Liu


On 一月 26, 2017, 12:55 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55968/
> ---
> 
> (Updated 一月 26, 2017, 12:55 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The executor and tasks fields in the master's structs for agents
> and frameworks are publicly visible, but yet for correctness it
> is required that callers made modifications through the member
> functions which carry additional logic (e.g. tracking allocated
> resources). The TODO here is to make these private and to provide
> public views that are const.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
> 
> Diff: https://reviews.apache.org/r/55968/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55901: Added support for command health checks to the default executor.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55900, 55901]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Jan. 28, 2017, 12:39 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55901/
> ---
> 
> (Updated Jan. 28, 2017, 12:39 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Anand Mazumdar, haosdent 
> huang, and Vinod Kone.
> 
> 
> Bugs: MESOS-6280
> https://issues.apache.org/jira/browse/MESOS-6280
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for command health checks to the default executor.
> 
> 
> Diffs
> -
> 
>   src/checks/health_checker.hpp 95da1ff7dd6b222a93076633eb3757ec9aa43cf6 
>   src/checks/health_checker.cpp e70bd7936752613a4f92c70c4c61cd7cdf7c4ee5 
>   src/launcher/default_executor.cpp 97eee05cac8cb1f62d43e2aecc08a8e54e49eac3 
>   src/tests/health_check_tests.cpp 710cb66eff6c4447caa22772f0cdc97cfa582c50 
> 
> Diff: https://reviews.apache.org/r/55901/diff/
> 
> 
> Testing
> ---
> 
> Introduced a new test: `HealthCheckTest.DefaultExecutorCmdHealthCheck`. It 
> passes on Linux, but not on macOS.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>

Re: Review Request 55870: Update the allocator to handle frameworks with multiple roles.

[Guangya Liu]

> On 一月 28, 2017, 11:45 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1176-1177
> > 
> >
> > A question here: Why not call `resources.unallocate()` at #1130? 
> > 
> > This can make the logic of `allocate` and `recoverResources` as pair: 
> > `allocate` will set allocation info while `recoverResources` will clear 
> > allocatio info.

I see, it is enough to only decrese the `allocated` in `recoverResources`.


- Guangya


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


On 一月 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> ---
> 
> (Updated 一月 25, 2017, 9:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
> 
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 558594983beb6ff49c1fddf875ba29c1983b1288 
>   src/master/allocator/mesos/allocator.hpp 
> 8e0f37a5eedd4d71d765991c0039b49c96f0ca53 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9b66c23f26b37c02ed850c06c4518ea99078b02d 
>   src/master/allocator/mesos/hierarchical.cpp 
> c2211be7458755aeb91ef078e4bfe92ac474044a 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
>   src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558 
> 
> Diff: https://reviews.apache.org/r/55870/diff/
> 
> 
> Testing
> ---
> 
> Adjustments to existing tests are split out into a subsequent patch.
> 
> New tests for frameworks having multiple roles will be added as a follow up.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55870: Update the allocator to handle frameworks with multiple roles.

[Guangya Liu]

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




src/master/allocator/mesos/hierarchical.cpp (lines 820 - 824)


I found that this was used in many places for both master and agent, how 
about put this in resources_utils.cpp?


- Guangya Liu


On 一月 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> ---
> 
> (Updated 一月 25, 2017, 9:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6635
> https://issues.apache.org/jira/browse/MESOS-6635
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
> 
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 558594983beb6ff49c1fddf875ba29c1983b1288 
>   src/master/allocator/mesos/allocator.hpp 
> 8e0f37a5eedd4d71d765991c0039b49c96f0ca53 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9b66c23f26b37c02ed850c06c4518ea99078b02d 
>   src/master/allocator/mesos/hierarchical.cpp 
> c2211be7458755aeb91ef078e4bfe92ac474044a 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
>   src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558 
> 
> Diff: https://reviews.apache.org/r/55870/diff/
> 
> 
> Testing
> ---
> 
> Adjustments to existing tests are split out into a subsequent patch.
> 
> New tests for frameworks having multiple roles will be added as a follow up.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55970: Updated the master's HTTP operations to handle MULTI_ROLE changes.

[Guangya Liu]

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




src/master/http.cpp (lines 4614 - 4615)


I think this can be replaced by the util function in below comments?



src/master/quota_handler.cpp (lines 196 - 200)


Per the comments in /r/55870/ , how about move this to resources_utils.cpp?



src/master/validation.cpp (lines 1654 - 1667)


Did not quite catch up the `TODO` here, can you please elaborate for why 
want such a function?



src/master/validation.cpp (line 1718)


How about 

```
if (unallocated(resources).contains(volume)) {
```

and then kill #1715 ?


- Guangya Liu


On 一月 26, 2017, 1:08 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55970/
> ---
> 
> (Updated 一月 26, 2017, 1:08 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the addition of MULTI_ROLE framework support, allocated
> resources in the master have `Resource.AllocationInfo` set. This
> means that the master's http endpoints that apply operations or
> perform checks between unallocated and allocated resources must
> be updated to continue to function correctly.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a44621f39cb059e654a56f57f75b38947f3a4230 
>   src/master/quota_handler.cpp 6e6e7375219d34e6e8d011a025b5f5d70b87383b 
>   src/master/validation.cpp 5f134b781901f2de6a90fa6a10d42cc7629c27a0 
> 
> Diff: https://reviews.apache.org/r/55970/diff/
> 
> 
> Testing
> ---
> 
> The existing tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55954: Changed 'Environment.Variable.Value' from required to optional.

[Greg Mann]

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

(Updated Jan. 28, 2017, 11:34 p.m.)


Review request for mesos, Jan Schlicht and Vinod Kone.


Changes
---

Addressed comments, added more validation.


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


Repository: mesos


Description
---

To prepare for future work which will enable the
modular fetching of secrets, we should change the
Environment.Variable.Value field from required to
optional. This way, the field can be left empty
and filled in by a secret fetching module.


Diffs (updated)
-

  include/mesos/mesos.proto a08b8fb2edf26ae730467557ae0e33e3d4252593 
  include/mesos/v1/mesos.proto 4a4609e7db659252ef7f1ca61f8ba079b901b7d0 
  src/checks/checker.cpp 8d7285af40d9633608178ec239d3b8d65d3a2725 
  src/checks/health_checker.cpp e70bd7936752613a4f92c70c4c61cd7cdf7c4ee5 
  src/common/validation.hpp fee955139d9699caa651d9bbd03342f2eba8143f 
  src/common/validation.cpp b2548ad87b4227d6e498c49b5694acb362f6281b 
  src/master/validation.cpp e3f71be16ff21a1b4e10ad59846d9b06691bc1b9 
  src/slave/validation.cpp c30a0ebf044ac81b087a8a599c7476bdc16bef18 

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


Testing
---

Testing information at the end of this review chain.


Thanks,

Greg Mann

Re: Review Request 55955: Added validation tests to ensure environment variable value is set.

[Greg Mann]

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

(Updated Jan. 28, 2017, 11:35 p.m.)


Review request for mesos, Jan Schlicht and Vinod Kone.


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


Repository: mesos


Description
---

The `value` field within `Environment::Variable` is being
changed to `optional`, but for the time being we will
enforce that it must be set for backward compatibility.
This patch adds tests to ensure that environment variables
with unset values are correctly rejected.


Diffs (updated)
-

  src/tests/check_tests.cpp c88cd34fd214f111cff62591aa5fc03eb62567e4 
  src/tests/health_check_tests.cpp debbd3c09b7555145aaf3f62a24d795d1423a269 
  src/tests/master_validation_tests.cpp 
edb57407e08cdbd8fbf10a9e1493cab3b4979bb8 
  src/tests/slave_validation_tests.cpp 5de771114982751e7796f55dcacd4384c6989efb 

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


Testing
---

bin/mesos-tests.sh --gtest_filter="*Validation*"


Thanks,

Greg Mann

Review Request 56055: Added validation for 'LAUNCH_NESTED_CONTAINER_SESSION'.

[Greg Mann]

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

Review request for mesos, Jan Schlicht and Vinod Kone.


Repository: mesos


Description
---

This patch adds a validation test for the
`LAUNCH_NESTED_CONTAINER_SESSION` call.


Diffs
-

  src/tests/slave_validation_tests.cpp 5de771114982751e7796f55dcacd4384c6989efb 

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


Testing
---

`bin/mesos-tests.sh --gtest_filter="*Validation*"`


Thanks,

Greg Mann

Re: Review Request 54836: Added helpers to allocate / unallocate Resources.

[Michael Park]

> On Jan. 25, 2017, 1:54 a.m., Michael Park wrote:
> > include/mesos/resources.hpp, lines 332-339
> > 
> >
> > I would've expected:
> > ```cpp
> > void allocate(const std::string& role);
> > void unallocate();
> > ```
> > 
> > Could you help me understand what the `bool` from `unallocate` is for?
> > 
> > If it's simply to indicate whether the function had any effects or
> > not, `allocate` could also return a `bool`, which returns `false`
> > when the resources were already allocated for the specified role.
> 
> Guangya Liu wrote:
> For `allocate`, seems the question here is same as mine above: In which 
> case shall we need to overwrite the role for allocation?
> 
> Guangya Liu wrote:
> @mpark, I think that Ben already posted some comments here for why return 
> `bool` for `unallocate` at https://reviews.apache.org/r/54836/#comment230598
> 
> Michael Park wrote:
> @bmahler, Actually, upon looking further in the chain I realized that 
> mutating the resources here is new...
> You had mentioned this to me as well and explained why (I think), could 
> you explain again here
> why we didn't do `allocations` (which we already have), `allocated` and 
> `unallocated`?
> Similar to `reservations`, `reserved` and `unreserved`?

Never mind, regarding my last comment. I had another realization that we're not 
trying
to filter for `allocated` and `unallocated` resources, but rather want to 
mutate them.


- Michael


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


On Jan. 22, 2017, 5:55 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54836/
> ---
> 
> (Updated Jan. 22, 2017, 5:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helpers to allocate / unallocate Resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp c2e6d6680d9687f0bbfb73e84ffb1457072c4412 
>   include/mesos/v1/resources.hpp 98f025b68c818c384a1174ef9b969f5c0776a7f1 
>   src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45 
>   src/tests/resources_tests.cpp 8dfb1be35d9f9c6ff69139d055c6b3d3ec475e68 
>   src/v1/resources.cpp da4701c03020ff9c33ef995cd0af437d8827c267 
> 
> Diff: https://reviews.apache.org/r/54836/diff/
> 
> 
> Testing
> ---
> 
> Updated the existing allocation test to incorporate the new helper.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 56055: Added validation for 'LAUNCH_NESTED_CONTAINER_SESSION'.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55954, 55955, 56055]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Jan. 28, 2017, 11:39 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56055/
> ---
> 
> (Updated Jan. 28, 2017, 11:39 p.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a validation test for the
> `LAUNCH_NESTED_CONTAINER_SESSION` call.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_validation_tests.cpp 
> 5de771114982751e7796f55dcacd4384c6989efb 
> 
> Diff: https://reviews.apache.org/r/56055/diff/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh --gtest_filter="*Validation*"`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 56052: Added the 'Secret' protobuf message.

[Greg Mann]

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

Review request for mesos, Jan Schlicht and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a new `Secret` protobuf message which
is designed to serve as a generic mechanism for passing
priviledged information within Mesos.


Diffs
-

  include/mesos/mesos.proto a08b8fb2edf26ae730467557ae0e33e3d4252593 
  include/mesos/v1/mesos.proto 4a4609e7db659252ef7f1ca61f8ba079b901b7d0 

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


Testing
---

`make check`


Thanks,

Greg Mann

Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

[Greg Mann]

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

Review request for mesos, Jan Schlicht and Vinod Kone.


Repository: mesos


Description
---

This patch adds a field of type `Secret` to the
`Environment` protobuf message, enabling the passing
of secrets into the environments of executors and
tasks. Additional validation and test code is added
as well.


Diffs
-

  include/mesos/mesos.proto a08b8fb2edf26ae730467557ae0e33e3d4252593 
  include/mesos/v1/mesos.proto 4a4609e7db659252ef7f1ca61f8ba079b901b7d0 
  src/common/validation.cpp b2548ad87b4227d6e498c49b5694acb362f6281b 
  src/tests/slave_validation_tests.cpp 5de771114982751e7796f55dcacd4384c6989efb 

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

[Greg Mann]

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

(Updated Jan. 29, 2017, 5:10 a.m.)


Review request for mesos, Jan Schlicht and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a field of type `Secret` to the
`Environment` protobuf message, enabling the passing
of secrets into the environments of executors and
tasks. Additional validation and test code is added
as well.


Diffs
-

  include/mesos/mesos.proto a08b8fb2edf26ae730467557ae0e33e3d4252593 
  include/mesos/v1/mesos.proto 4a4609e7db659252ef7f1ca61f8ba079b901b7d0 
  src/common/validation.cpp b2548ad87b4227d6e498c49b5694acb362f6281b 
  src/tests/slave_validation_tests.cpp 5de771114982751e7796f55dcacd4384c6989efb 

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 56053: Added a 'SECRET' type to the 'Environment' protobuf message.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55954, 55955, 56055, 56052, 56053]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Jan. 29, 2017, 5:10 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56053/
> ---
> 
> (Updated Jan. 29, 2017, 5:10 a.m.)
> 
> 
> Review request for mesos, Jan Schlicht and Vinod Kone.
> 
> 
> Bugs: MESOS-7009
> https://issues.apache.org/jira/browse/MESOS-7009
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a field of type `Secret` to the
> `Environment` protobuf message, enabling the passing
> of secrets into the environments of executors and
> tasks. Additional validation and test code is added
> as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto a08b8fb2edf26ae730467557ae0e33e3d4252593 
>   include/mesos/v1/mesos.proto 4a4609e7db659252ef7f1ca61f8ba079b901b7d0 
>   src/common/validation.cpp b2548ad87b4227d6e498c49b5694acb362f6281b 
>   src/tests/slave_validation_tests.cpp 
> 5de771114982751e7796f55dcacd4384c6989efb 
> 
> Diff: https://reviews.apache.org/r/56053/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 55030: CMake: Added source groups for libprocess build.

[Alex Clemmer]

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

(Updated Jan. 29, 2017, 7:18 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, 
and Joseph Wu.


Changes
---

Rebase against current HEAD.


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


Repository: mesos


Description
---

Currently in IDEs such as XCode and Visual Studio, all mesos source
files are grouped into one huge folder, with no attempt made to reflect
the hierarchy of folders that exist on disk.

This commit groups libprocess files into relevant source groups so that
they appear in a sensible directory structure in these IDEs.

In addition to beginning to directly address MESOS-3542, this is also
(indirectly) the first step towards addressing the problem of breaking
libmesos up into smaller binaries (MESOS-3542).


Diffs (updated)
-

  3rdparty/libprocess/src/CMakeLists.txt 
ab362aa476e528deef691239a70f0be9d864e6c0 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
b298bbe042cabcb18169da1923562e9956374232 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 55030: CMake: Added source groups for libprocess build.

[Alex Clemmer]

> On Jan. 25, 2017, 12:29 a.m., Joseph Wu wrote:
> > This review is partially un-done by https://reviews.apache.org/r/55604/ .  
> > Can you separate the source-grouping changes into 
> > https://reviews.apache.org/r/55604/ ?  And leave the CMake lists 
> > rearrangement in this review?

We still need this change because we need to add the `.hpp` files to the call 
to `add_executable` in order for them to show up as source groups. I've rebased 
to account for your recent changes.


- Alex


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


On Jan. 29, 2017, 7:18 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55030/
> ---
> 
> (Updated Jan. 29, 2017, 7:18 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, John Kordich, 
> and Joseph Wu.
> 
> 
> Bugs: MESOS-3542
> https://issues.apache.org/jira/browse/MESOS-3542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in IDEs such as XCode and Visual Studio, all mesos source
> files are grouped into one huge folder, with no attempt made to reflect
> the hierarchy of folders that exist on disk.
> 
> This commit groups libprocess files into relevant source groups so that
> they appear in a sensible directory structure in these IDEs.
> 
> In addition to beginning to directly address MESOS-3542, this is also
> (indirectly) the first step towards addressing the problem of breaking
> libmesos up into smaller binaries (MESOS-3542).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/CMakeLists.txt 
> ab362aa476e528deef691239a70f0be9d864e6c0 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> b298bbe042cabcb18169da1923562e9956374232 
> 
> Diff: https://reviews.apache.org/r/55030/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 55162: Stout: Added style fixes and some useful error messages.

[Alex Clemmer]

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

(Updated Jan. 29, 2017, 7:20 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Changes
---

Rearrange dependency.


Repository: mesos


Description
---

Stout: Added style fixes and some useful error messages.


Diffs
-

  3rdparty/stout/include/stout/os/windows/killtree.hpp 
2be2f8c3d58ee64410f87ee4a4b2bb54fe014748 
  3rdparty/stout/include/stout/windows/os.hpp 
5cd92545a49648e39e8eb7cf131895e9cfc97902 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 55543: Fail the build if %PreferredToolArchitecture% is not set to `x64`.

[Alex Clemmer]

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

(Updated Jan. 29, 2017, 7:21 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Changes
---

Rearrange dependencies.


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


Repository: mesos


Description
---

Before building Mesos on a Windows machine, it is necessary to set
`%PreferredToolArchitecture%` to the value `x64`. This is necessary to
work around (at least) two bugs in the MSVC backend: in particular, the
linker can sometimes take hours or days to link `mesos-x.x.x.lib`, and
the build system occasionally finds it self spuriously unable to find
file `mesos-x.x.x.lib` to link against.

These issues are well-known and documented (e.g., in the official Mesos
"getting started" document), but it is better to simply refuse to build
Mesos at all on Windows unless that environment variable is set.

This commit will introduce such a check.


Diffs
-

  cmake/CompilationConfigure.cmake 560935b81603dc58c167918d36e2ae0a4060673d 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 55546: Added platform-independent constants `DEV_NULL` and `TRUE_COMMAND`.

[Alex Clemmer]

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

(Updated Jan. 29, 2017, 7:21 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Changes
---

Rearrange dependencies.


Repository: mesos


Description
---

Added platform-independent constants `DEV_NULL` and `TRUE_COMMAND`.


Diffs
-

  3rdparty/stout/include/stout/gtest.hpp 
a004a378cb467495234d77a0c56fbea6e7bec420 
  3rdparty/stout/include/stout/os/constants.hpp 
c71d52e1bc0433f9922f26a6eb486235bb9880d4 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 55749: Added CMake to standard documentation.

[Alex Clemmer]

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

(Updated Jan. 29, 2017, 7:22 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.


Changes
---

Rearrange dependencies.


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


Repository: mesos


Description
---

Added CMake to standard documentation.


Diffs
-

  docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 

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


Testing
---


Thanks,

Alex Clemmer