Re: Review Request 55973: Update the tests to handle MULTI_ROLE support.

[Guangya Liu]

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




src/examples/dynamic_reservation_framework.cpp (line 123)


A question here, why need `apply` resources here? What is the problem is we 
keeps check if the `unreserved` contains `Allocated TASK RESOURCES`?



src/tests/master_allocator_tests.cpp (line 266)


kill this



src/tests/master_allocator_tests.cpp (line 273)


kill this



src/tests/master_tests.cpp (lines 73 - 74)


switch the order here


- Guangya Liu


On 一月 27, 2017, 12:34 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55973/
> ---
> 
> (Updated 一月 27, 2017, 12:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A number of tests and example frameworks assume that allocated
> resources did not look different from unallocated resources.
> This updates the tests to reflect the presence of
> `Resource.AllocationInfo`.
> 
> 
> Diffs
> -
> 
>   src/examples/disk_full_framework.cpp 
> e13d4c8a427905793dda9bb01c52b6d372c19150 
>   src/examples/dynamic_reservation_framework.cpp 
> 4d3db965e18d79ed3e1759bb5f6cb41104562f47 
>   src/examples/no_executor_framework.cpp 
> e82ae9a9ea23c7d181b071f0e3f5071b3856d8a4 
>   src/examples/persistent_volume_framework.cpp 
> 9d45bb496c4cf378af429b5aa970bf81450e482a 
>   src/examples/test_framework.cpp 068b85d551012a390ba2a68bc88807103d3c31f3 
>   src/examples/test_http_framework.cpp 
> 258cb512803d1ed07c7a5ce1465e5663e77b0977 
>   src/tests/api_tests.cpp 400ac6fcd420508509291249cad50679fbe9807d 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> ba268b6918be0e7226a975c38eff5620b076083f 
>   src/tests/hook_tests.cpp f4ef629768beabc014e14472c5818d117d51abe7 
>   src/tests/master_allocator_tests.cpp 
> 996762f25453f7a8a5e0b7b97006ee2a603cf8c4 
>   src/tests/master_tests.cpp da7094dbbafbb0ab1153a0a4a6fcabd63888d67a 
>   src/tests/mesos.hpp 71e919702bccd31bf1e14205cd1e30d473883d48 
>   src/tests/oversubscription_tests.cpp 
> 22ae069ab71c82c6a6e2f5783b13ebd74a9ccf25 
>   src/tests/partition_tests.cpp f03c5bd96861b6643a4ecd9e37775447f86c3710 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 695029fe0cb91d668d6a8495f3ccd0c69c883cd2 
>   src/tests/persistent_volume_tests.cpp 
> 468a85b4a6ce09592341afd07ce12a03f5fc4f73 
>   src/tests/reservation_endpoints_tests.cpp 
> 7bc59c2ded9f7e6f1ccaa37456f654d18efcb2e2 
>   src/tests/reservation_tests.cpp b7061de0dfcc79548d507c7d70376c82d5d647ec 
>   src/tests/resource_offers_tests.cpp 
> 72ceb8696a7277f9add6d89adb55aa08665bdeb5 
>   src/tests/role_tests.cpp 0d0f1a94ff9aaec6ea04280282a52ac88dc6b273 
>   src/tests/slave_recovery_tests.cpp 9cf1d461cf5c6ac1f51ac963360a17e37f558505 
> 
> Diff: https://reviews.apache.org/r/55973/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 56004: Fixed MULTI_ROLE related bugs when updating framework info.

[Guangya Liu]

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


Fix it, then Ship it!




Ship It!


src/master/master.hpp (lines 2431 - 2440)


Can you please add some comments here to clarify that this is used to 
handle upgrade/downgrade case to/from multi role framework?


- Guangya Liu


On 一月 27, 2017, 12:30 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56004/
> ---
> 
> (Updated 一月 27, 2017, 12:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The first issue is that we need to update the capabilities member
> to reflect the new capabilities.
> 
> The second issue is that when we allow an upgrade or downgrade
> to or from MULTI_ROLE, we need to update the `role` and `roles`
> fields of `FrameworkInfo`.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 
> 
> Diff: https://reviews.apache.org/r/56004/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

[Guangya Liu]

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




src/master/master.cpp (lines 5620 - 5628)


Same comments as /r/55971/ here


- Guangya Liu


On 一月 27, 2017, 12:29 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55972/
> ---
> 
> (Updated 一月 27, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6968
> https://issues.apache.org/jira/browse/MESOS-6968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the addition of MULTI_ROLE framweork support, allocated
> resources have a `Resource.AllocationInfo` set. While the new
> MULTI_ROLE capable agents will be sending allocated resources,
> the old agents may not be since they may not preserve the
> unknown fields.
> 
> To cope with this, the master ensures the `Resource.AllocationInfo`
> is set for non-MULTI_ROLE capable agents, by injecting it. Note
> that we can only do this so long as it is unambiguous! This will
> be prevented by not allowing frameworks with multiple to use
> agents without the MULTI_ROLE support, see: MESOS-6940.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 
>   src/master/master.cpp 0f2c7cd32161576fad742ef350dff64874b80854 
> 
> Diff: https://reviews.apache.org/r/55972/diff/
> 
> 
> Testing
> ---
> 
> The tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55971: Updated the agent to be MULTI_ROLE capable.

[Guangya Liu]

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




src/slave/slave.cpp (lines 1874 - 1878)


Ditto as comments in previous patches, move this to a resources util.



src/slave/slave.cpp (lines 5241 - 5249)


Since we do not support store mix allocated and unallocated resources, how 
about optimize the logic a big as following?

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

if (roles.size() != 1) {
  LOG(FATAL) << "Missing 'Resource.AllocationInfo' for resources"
 << " allocated to MULTI_ROLE framework"
 << " '" << frameworkInfo.name() << "'";
}

resource->mutable_allocation_info()->set_role(*roles.begin());
```



src/slave/slave.cpp (line 5244)


How about include `resources` in the log message?


- Guangya Liu


On 一月 27, 2017, 12:27 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55971/
> ---
> 
> (Updated 一月 27, 2017, 12:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6964
> https://issues.apache.org/jira/browse/MESOS-6964
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the addition of MULTI_ROLE support, the agent needs to ensure
> that allocated resources reported to the master have the
> `Resource.AllocationInfo` set. The approach taken here is to set
> it only in memory upon recovering tasks and executors. Note that
> once we allow a framework to modify its roles, we need to update
> the agent to re-persist the resources with injected allocation
> info (rather than just setting it in memory).
> 
> 
> Diffs
> -
> 
>   src/slave/resource_estimators/fixed.cpp 
> 2c1268c3423091c6a419020a3af97255de55db0a 
>   src/slave/slave.cpp 0548b04073c0ba4adfc4433d75fd18c2ba79d891 
> 
> Diff: https://reviews.apache.org/r/55971/diff/
> 
> 
> Testing
> ---
> 
> The tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55964: Improved executor, scheduler V1 API docs.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On Jan. 29, 2017, 11:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55964/
> ---
> 
> (Updated Jan. 29, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These APIs are no longer in beta. Also make various fixes and cleanups
> for grammar and writing style, and fix typos in a few other doc pages.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md a3b8d6f7cff4f182c8fb77dc216d82eca1d30e7c 
>   docs/fetcher.md d4ffd523a0c689bdb462e934c431a38a9d8c350b 
>   docs/operator-http-api.md 41ebdb51644677369f0d96d6982d8441be855bc3 
>   docs/quota.md 0abb9ebe8e9a40c6b7201a1f0e8071b7b249a6ed 
>   docs/scheduler-http-api.md 958cfc52496b1585564e019f9ce8b06a623f2d5c 
> 
> Diff: https://reviews.apache.org/r/55964/diff/
> 
> 
> Testing
> ---
> 
> Visual inspection on GH.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 55964: Improved executor, scheduler V1 API docs.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [55964]

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, 11:41 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55964/
> ---
> 
> (Updated Jan. 29, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These APIs are no longer in beta. Also make various fixes and cleanups
> for grammar and writing style, and fix typos in a few other doc pages.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md a3b8d6f7cff4f182c8fb77dc216d82eca1d30e7c 
>   docs/fetcher.md d4ffd523a0c689bdb462e934c431a38a9d8c350b 
>   docs/operator-http-api.md 41ebdb51644677369f0d96d6982d8441be855bc3 
>   docs/quota.md 0abb9ebe8e9a40c6b7201a1f0e8071b7b249a6ed 
>   docs/scheduler-http-api.md 958cfc52496b1585564e019f9ce8b06a623f2d5c 
> 
> Diff: https://reviews.apache.org/r/55964/diff/
> 
> 
> Testing
> ---
> 
> Visual inspection on GH.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

[Michael Park]

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


Fix it, then Ship it!





src/common/protobuf_utils.cpp (lines 332 - 335)


Is there a reason why we don't just mutate it in-place?
```cpp
void adjustOfferOperation(
Offer::Operation* operation,
const Resource::AllocationInfo& allocationInfo);
```



src/common/protobuf_utils.cpp (lines 340 - 341)


I think it'd be clearer for us to write:

```cpp
foreach (TaskInfo& task, *result.launch().mutable_task_infos()) {
  // ...
}
```



src/common/protobuf_utils.cpp (lines 343 - 348)


We basically do this for every repeated resources field within 
`Offer::Operation`. Can we introduce a function similar to 
`adjustOfferOperation`?

```cpp
void adjustResources(
RepeatedPtrField* resources,
const Resource::AllocationInfo& allocationInfo);
```

The `injectAllocationInfo` helper which operates on `FrameworkInfo`
I think should just be:

```cpp
  auto injectAllocationInfo = [](
  RepeatedPtrField* resources,
  const FrameworkInfo& frameworkInfo)
  {
if (protobuf::frameworkHasCapability(
  frameworkInfo,
  FrameworkInfo::Capability::MULTI_ROLE) {
  return;  
}

Resource::AllocationInfo allocationInfo;
allocationInfo.set_role(frameworkInfo.role);
adjustResources(resources, allocationInfo);
  };
```


- Michael Park


On Jan. 23, 2017, 2:59 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55863/
> ---
> 
> (Updated Jan. 23, 2017, 2: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 55825: Augmented printing of Resources to include AllocationInfo.

[Benjamin Mahler]

> On Jan. 25, 2017, 10:19 a.m., Michael Park wrote:
> > src/common/resources.cpp, line 1942
> > 
> >
> > Labeling `allocated:` seems okay to me, but also seems to be new to 
> > this format.
> > 
> > It's not clear to me what direction we're going with this, seeing as 
> > it's gotten (and getting) more complicated.
> > 
> > I think the eventual plan is to move to using JSON for this 
> > representation?
> > 
> > Just would like your quick thoughts on this.

I originally went without the label, and found it quite confusing (note that 
the reservation info currently shows principal without a label and that's also 
fairly confusing). The difficulty in moving to JSON for logging is that the 
JSON is fairly verbose compared to the current short string format.


- Benjamin


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


On Jan. 23, 2017, 2:04 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55825/
> ---
> 
> (Updated Jan. 23, 2017, 2:04 a.m.)
> 
> 
> Review request for mesos, Jay Guo, Guangya Liu, and Michael Park.
> 
> 
> Bugs: MESOS-6970
> https://issues.apache.org/jira/browse/MESOS-6970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignoring whether there is a disk info, the format is as follows:
> name(reservedrole[, principal[, labels]])(allocatedrole):value
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45 
>   src/tests/resources_tests.cpp 8dfb1be35d9f9c6ff69139d055c6b3d3ec475e68 
>   src/v1/resources.cpp da4701c03020ff9c33ef995cd0af437d8827c267 
> 
> Diff: https://reviews.apache.org/r/55825/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55825: Augmented printing of Resources to include AllocationInfo.

[Benjamin Mahler]

> On Jan. 25, 2017, 11:10 a.m., Guangya Liu wrote:
> > src/tests/resources_tests.cpp, line 822
> > 
> >
> > How about not clear this but keep the `allocated` label for all of the 
> > test or just add this to the end of this test after line 883? This can 
> > convine us that the `allocated` label works for all kind of resources.

Well, why do we need to be convinced of that? I think we can leverage a bit of 
internal knowledge here to simplify the test: that the printing of allocation 
info is orthogonal to other properties of the resource.


- Benjamin


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


On Jan. 23, 2017, 2:04 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55825/
> ---
> 
> (Updated Jan. 23, 2017, 2:04 a.m.)
> 
> 
> Review request for mesos, Jay Guo, Guangya Liu, and Michael Park.
> 
> 
> Bugs: MESOS-6970
> https://issues.apache.org/jira/browse/MESOS-6970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignoring whether there is a disk info, the format is as follows:
> name(reservedrole[, principal[, labels]])(allocatedrole):value
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45 
>   src/tests/resources_tests.cpp 8dfb1be35d9f9c6ff69139d055c6b3d3ec475e68 
>   src/v1/resources.cpp da4701c03020ff9c33ef995cd0af437d8827c267 
> 
> Diff: https://reviews.apache.org/r/55825/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benjamin Mahler]

> On Dec. 18, 2016, 1:10 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 1062
> > 
> >
> > Shall we add a `CHECK` here to make sure this resource was not 
> > allocated to any role?
> 
> Benjamin Mahler wrote:
> We could, the alternative that I went for was to have callers (the 
> allocator) CHECK this invariant if they care about it, since it seems 
> specific to the way the allocator works.
> 
> Guangya Liu wrote:
> I saw that you have updated the comments of `allocate` by allowing 
> existing allocation info be overwitten by a new role, why do we allow this? 
> Which secnario need this operation?
> 
> Benjamin Mahler wrote:
> It's not clear if we have a scenario that needs the ability to overwrite, 
> it's just the behavior I would intuit, much like set_role overwrites if there 
> is a role already set.
> 
> Guangya Liu wrote:
> Still have one question: For which case shall we need to overwrite the 
> role? Ideally, the role info should be cleared when `recoverResources`, so if 
> there are still roles when `allocate`, we should report an error?

There shouldn't be any use cases in mesos for over-writing a role. Currently 
there is little checking that unallocated resources are unallocated and 
allocated resources are allocated, but I'd like to make this less brittle at 
some point with better types.


- Benjamin


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


On Jan. 23, 2017, 1:55 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54836/
> ---
> 
> (Updated Jan. 23, 2017, 1:55 a.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 54836: Added helpers to allocate / unallocate Resources.

[Benjamin Mahler]

> On Jan. 25, 2017, 10:40 a.m., Guangya Liu wrote:
> > src/tests/resources_tests.cpp, line 2774
> > 
> >
> > How about 
> > 
> > ```
> > Resources cpus2 = cpus1;
> > ```
> > 
> > Ditto for others.

Hm.. I found this makes the test a bit harder to follow, since you have to look 
at the other definition to see the value. Note that we don't care here that 
cpus2 is equal to cpus1, just that cpus2 is unreserved.


- Benjamin


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


On Jan. 23, 2017, 1:55 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54836/
> ---
> 
> (Updated Jan. 23, 2017, 1:55 a.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 55828: Updated Resources::apply to handle Resource.AllocationInfo.

[Michael Park]

> On Jan. 29, 2017, 12:02 a.m., Michael Park wrote:
> > It took me a while trying to figure out what the invariants of `Resources` 
> > and `Offer::Operation` are, but I think I have a grasp of it.
> > 
> > - We have `recovered.apply(operation);` where we have `recovered` and 
> > `operation` both with unallocated resources. (after `recovered` adjustment)
> > - We have a few `offeredResources.apply(operation);` where we have 
> > `offeredResources` and `operation` both with resources allocated to a 
> > single role. (after `operation` adjustment)
> > - We also have `totalResources.apply(operation);` where we have 
> > `totalResources` with unallocated resources, and `operation` with resources 
> > allocated to a single role. (case 2 helps us with this)
> > 
> > It seems to me like we really end up with resources allocated to 
> > __multiple__ roles if we were to allow accepting multiple offers from 
> > multiple roles.
> > That is, currently the resources in `Resources` and `Offer::Operation` are 
> > either all unallocated, or all allocated to one role.
> > 
> > Is this accurate?

Ah, I was wrong about `Resources`. It can hold resources that are allocated to 
multiple roles.
I was correct about `Offer::Operation`, however. In that they can only be 
unallocated or allocated to one role.


- Michael


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


On Jan. 23, 2017, 2:47 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55828/
> ---
> 
> (Updated Jan. 23, 2017, 2:47 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 allocation role.
> The two interesting cases for adjusting the operation's resource are:
> 
> (1) The operation `Resource` does not contain an `AllocationInfo`
> but is being applied to an allocated `Resources`. We allow this
> only if the operation is unambiguous, that is, the allocated
> `Resources` are only allocated to a single role.
> 
> (2) The operation `Resource` contains an `AllocationInfo` but is
> being applied to an unallocated `Resources`. In this case we
> simply ignore the `AllocationInfo` of the `Resource`.
> 
> Note that we assume no `Resources` store a mix of allocated and
> unallocated resources. This is a brittle assumption that we should
> have enforcement for.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45 
>   src/tests/resources_tests.cpp 8dfb1be35d9f9c6ff69139d055c6b3d3ec475e68 
>   src/tests/resources_utils.hpp 18dcca7f171102df8fe88f10785f70c5d1cf5b32 
>   src/v1/resources.cpp da4701c03020ff9c33ef995cd0af437d8827c267 
> 
> Diff: https://reviews.apache.org/r/55828/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 56005: Added a safety CHECK when accessing activeRoles in the master.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On Jan. 27, 2017, 12:31 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56005/
> ---
> 
> (Updated Jan. 27, 2017, 12:31 a.m.)
> 
> 
> Review request for mesos, Michael Park and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently there is a use of the [] operator that is not preceded
> by a CHECK that the key is contained in the map. This leads to a
> hard to diagnose issue if there are bugs that violate this
> assumption.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 0f2c7cd32161576fad742ef350dff64874b80854 
> 
> Diff: https://reviews.apache.org/r/56005/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55964: Improved executor, scheduler V1 API docs.

[Neil Conway]

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

(Updated Jan. 29, 2017, 11:41 p.m.)


Review request for mesos and Anand Mazumdar.


Changes
---

Address review comments.


Summary (updated)
-

Improved executor, scheduler V1 API docs.


Repository: mesos


Description (updated)
---

These APIs are no longer in beta. Also make various fixes and cleanups
for grammar and writing style, and fix typos in a few other doc pages.


Diffs (updated)
-

  docs/executor-http-api.md a3b8d6f7cff4f182c8fb77dc216d82eca1d30e7c 
  docs/fetcher.md d4ffd523a0c689bdb462e934c431a38a9d8c350b 
  docs/operator-http-api.md 41ebdb51644677369f0d96d6982d8441be855bc3 
  docs/quota.md 0abb9ebe8e9a40c6b7201a1f0e8071b7b249a6ed 
  docs/scheduler-http-api.md 958cfc52496b1585564e019f9ce8b06a623f2d5c 

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


Testing
---

Visual inspection on GH.


Thanks,

Neil Conway

Re: Review Request 50871: Supported auto backend in Unified Containerizer.

[Jie Yu]

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


Fix it, then Ship it!




I'll fix the issues below for you.


src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 50 
- 51)


No needed anymore?



src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp (lines 
231 - 257)


Looks like this missing layer check is best effort now. I'd suggest we 
simply remove it. The 'storedImage' will simply be a layer id cache.



src/slave/containerizer/mesos/provisioner/docker/paths.hpp (line 22)


Why this?



src/slave/containerizer/mesos/provisioner/provisioner.hpp (line 117)


Do you still need this for ProvisionerProcess?



src/slave/containerizer/mesos/provisioner/provisioner.cpp (lines 68 - 69)


It's not host filesystem, but the underlying filesystem for a specific 
directory.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 78)


I'd rename this to 'validateBackend'



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 121)


I would kill this helper as it's only used once. I'll just inline the 
implementation in Provisioner::create.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 122)


I would use `const Option& backend` here.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 127)


We should not only check 'provisionerDir' but also all the store dir as 
well. I'll leave a TODO here at the moment. Please create a ticket to track.

I think what we might need to track a per store default backend because 
each store might have a different store dir.



src/slave/containerizer/mesos/provisioner/provisioner.cpp (line 176)


What if copy backend is not supported? I know it is not possible at the 
moment, but would prefer writing the code a bit more defensively in case 
there're future changes.


- Jie Yu


On Jan. 26, 2017, 1:37 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50871/
> ---
> 
> (Updated Jan. 26, 2017, 1:37 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5931
> https://issues.apache.org/jira/browse/MESOS-5931
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Select a backend smartly. Currently, the logic is really
> simple:
> 1. Use overlayfs backend if it exists.
> 2. Use aufs backend if the overlayfs does not exists.
> 3. Use copy backend of both overlayfs and aufs do not exist.
> Please note that the bind backend needs to be specified explicitly
> through the agent flag '--image_provisioner_backend' since it
> requires the sandbox already existed.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2113d06f58dddc0a28ae1241a24096266fe39801 
>   src/slave/containerizer/mesos/provisioner/appc/store.hpp 
> 26cdd2fba142874ab1d3eca61222bb22bfbb2e13 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> e63ae419e24212b887edddeb5cae114cd39b39c8 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.hpp 
> abb8e7e48422896f207a475661ced0530fc75e68 
>   src/slave/containerizer/mesos/provisioner/docker/local_puller.cpp 
> 817e30c5d6d6a4b011193e3209301fc3cdf88b06 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> 395c36b0f6b8f8e037681ec5f2df99e83a7cf155 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/puller.hpp 
> 191f3fc8cd1e0f706b5b1e6afddf856e8d53ae6e 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.hpp 
> bbd6005317bed3fff3d86e2527ca3ead839d49e3 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> cecf34a23329a64fdbce7de4b83827a30975e9a4 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1c2b149823e83dc5a3feb0af599d651d1dc05682 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> 9dccd0673dbc0c61abfd4b88097f86e7d7131c46 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 0a48617d6f9ade928993e1d5205de6486ef657c7 
>   src/slave/containerizer/mesos/pr

Re: Review Request 55964: Updates for executor, scheduler V1 API docs.

[Anand Mazumdar]

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



Looks good and thanks for the cleanup! 

Can you update the summary/description given that the diff also contains minor 
typo fixes in the fetcher/quota docs.


docs/executor-http-api.md (line 36)


hmm, wondering why the change from s/connection(s)/connection?

We wanted to outline the fact that a scheduler can either send all 
non-subscribe calls on one or more connections or use HTTP pipelining to send 
all the calls on the same connection. This change gives the impression that we 
always want a client to use the latter.



docs/scheduler-http-api.md (line 37)


Ditto as above.


- Anand Mazumdar


On Jan. 25, 2017, 11:58 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55964/
> ---
> 
> (Updated Jan. 25, 2017, 11:58 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These APIs are no longer in beta. Also make various fixes and cleanups
> for grammar and writing style.
> 
> 
> Diffs
> -
> 
>   docs/executor-http-api.md a3b8d6f7cff4f182c8fb77dc216d82eca1d30e7c 
>   docs/fetcher.md d4ffd523a0c689bdb462e934c431a38a9d8c350b 
>   docs/quota.md 0abb9ebe8e9a40c6b7201a1f0e8071b7b249a6ed 
>   docs/scheduler-http-api.md 958cfc52496b1585564e019f9ce8b06a623f2d5c 
> 
> Diff: https://reviews.apache.org/r/55964/diff/
> 
> 
> Testing
> ---
> 
> Visual inspection on GH.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 53741: Display maintainance info in the webui.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [53741]

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, 2:48 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53741/
> ---
> 
> (Updated Jan. 29, 2017, 2:48 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Joseph Wu.
> 
> 
> Bugs: MESOS-6443
> https://issues.apache.org/jira/browse/MESOS-6443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create new page with Maintenance schedule. Schedule is downloaded on
> page refresh. Schedule is not live like stats and tasks so there is
> no need to poll it periodically.
> Diable sorting when data-key is not defined in table header.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 7c6a8ad56437c2315fc563d469ed0426286b48ce 
>   src/webui/master/static/js/app.js 558a67ddf59a39db43023b4f7aa26b7e319924b0 
>   src/webui/master/static/js/controllers.js 
> 07bc612a4d7a6b4b418de964303e8fb7083b5d31 
>   src/webui/master/static/maintenance.html PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53741/diff/
> 
> 
> Testing
> ---
> 
> [Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)
> 
> Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
> entires schedule generated with 
> [generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 48291: Use varint comparator in replica log.

[Tomasz Janiszewski]

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

(Updated Jan. 29, 2017, 2:56 p.m.)


Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, and haosdent 
huang.


Changes
---

rebase


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


Repository: mesos


Description
---

Since bug discussed at
https://groups.google.com/forum/#!topic/leveldb/F-rDkWiQm6c
was fixed. After upgrading leveldb to 1.18 we could replace
default byte-wise comparator with varint comparator.


Diffs (updated)
-

  src/log/leveldb.cpp 5310a123b0fb25f240429722b676fe46174cb2ce 

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


Testing
---

`make check dist` on Ubuntu x64


Thanks,

Tomasz Janiszewski

Re: Review Request 48291: Use varint comparator in replica log.

[Tomasz Janiszewski]

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

(Updated Jan. 29, 2017, 2:54 p.m.)


Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, and haosdent 
huang.


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


Repository: mesos


Description
---

Since bug discussed at
https://groups.google.com/forum/#!topic/leveldb/F-rDkWiQm6c
was fixed. After upgrading leveldb to 1.18 we could replace
default byte-wise comparator with varint comparator.


Diffs
-

  src/log/leveldb.cpp f389d74b123574665c611b46cb52e3dc7042b331 

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


Testing
---

`make check dist` on Ubuntu x64


Thanks,

Tomasz Janiszewski

Re: Review Request 51053: Update leveldb to 1.19.

[Tomasz Janiszewski]

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

(Updated Jan. 29, 2017, 2:52 p.m.)


Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, haosdent 
huang, and Vinod Kone.


Changes
---

Rebase


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


Repository: mesos


Description (updated)
---

Leveldb in modern version is required to support s390x and arm64.
It's also required to replace default byte-wise comparator
with varint comparator in \`src/log/leveldb.cpp\`.


Diffs (updated)
-

  3rdparty/Makefile.am bbf9cfe7125b193003115f440a00c91e2e68f404 
  3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
eeb27860f6f95d297ccfe273ed76de5355b50ff8 
  3rdparty/cmake/Versions.cmake ad23f3895b49d623709c0939d63a0529654788ef 
  3rdparty/leveldb-1.19.patch PRE-CREATION 
  3rdparty/leveldb-1.19.tar.gz PRE-CREATION 
  3rdparty/leveldb-1.4.patch b899f0141d633b1ffb2321e573395256fc893b16 
  3rdparty/leveldb-1.4.tar.gz 2ddbc0c2e02054406ff0ea43ddc10d14979de8d8 
  3rdparty/versions.am 26f839cd8d454de91f10c2e616e78decc30eff1b 
  LICENSE 7946d1d46174ff30459a33b3d0b44ab62672af0a 
  src/Makefile.am 6c9be54fe0ce3faaa26e2f090773d1b77bb6e430 
  src/python/native_common/ext_modules.py.in 
2d4a45efa224b32f80ace4542a00062c5ccb06d5 

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


Testing
---

make -j 8 distcheck
benchamrks: https://gist.github.com/janisz/c0cd248a796cd7f10254ddbc02e91872


Thanks,

Tomasz Janiszewski

Re: Review Request 53741: Display maintainance info in the webui.

[Tomasz Janiszewski]

> On Jan. 25, 2017, 6:16 p.m., haosdent huang wrote:
> > Hi, may you mind rebase this.

Done.


- Tomasz


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


On Jan. 29, 2017, 2:48 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53741/
> ---
> 
> (Updated Jan. 29, 2017, 2:48 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Joseph Wu.
> 
> 
> Bugs: MESOS-6443
> https://issues.apache.org/jira/browse/MESOS-6443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Create new page with Maintenance schedule. Schedule is downloaded on
> page refresh. Schedule is not live like stats and tasks so there is
> no need to poll it periodically.
> Diable sorting when data-key is not defined in table header.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html 7c6a8ad56437c2315fc563d469ed0426286b48ce 
>   src/webui/master/static/js/app.js 558a67ddf59a39db43023b4f7aa26b7e319924b0 
>   src/webui/master/static/js/controllers.js 
> 07bc612a4d7a6b4b418de964303e8fb7083b5d31 
>   src/webui/master/static/maintenance.html PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/53741/diff/
> 
> 
> Testing
> ---
> 
> [Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)
> 
> Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
> entires schedule generated with 
> [generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 53741: Display maintainance info in the webui.

[Tomasz Janiszewski]

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

(Updated Jan. 29, 2017, 2:48 p.m.)


Review request for mesos, haosdent huang and Joseph Wu.


Changes
---

Rebase and retest


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


Repository: mesos


Description
---

Create new page with Maintenance schedule. Schedule is downloaded on
page refresh. Schedule is not live like stats and tasks so there is
no need to poll it periodically.
Diable sorting when data-key is not defined in table header.


Diffs (updated)
-

  src/webui/master/static/index.html 7c6a8ad56437c2315fc563d469ed0426286b48ce 
  src/webui/master/static/js/app.js 558a67ddf59a39db43023b4f7aa26b7e319924b0 
  src/webui/master/static/js/controllers.js 
07bc612a4d7a6b4b418de964303e8fb7083b5d31 
  src/webui/master/static/maintenance.html PRE-CREATION 

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


Testing
---

[Screenshot](https://issues.apache.org/jira/secure/attachment/12838845/mesos_webui_maintenance_schedule.png)

Testing done maually on Ubuntu/Chrome. Perfomrance testing done with 500 
entires schedule generated with 
[generate_schedule.py](https://gist.github.com/janisz/e4dcb001f19aa4b466f9112a6dd16853)


Thanks,

Tomasz Janiszewski

Re: Review Request 55828: Updated Resources::apply to handle Resource.AllocationInfo.

[Michael Park]

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


Fix it, then Ship it!




It took me a while trying to figure out what the invariants of `Resources` and 
`Offer::Operation` are, but I think I have a grasp of it.

- We have `recovered.apply(operation);` where we have `recovered` and 
`operation` both with unallocated resources. (after `recovered` adjustment)
- We have a few `offeredResources.apply(operation);` where we have 
`offeredResources` and `operation` both with resources allocated to a single 
role. (after `operation` adjustment)
- We also have `totalResources.apply(operation);` where we have 
`totalResources` with unallocated resources, and `operation` with resources 
allocated to a single role. (case 2 helps us with this)

It seems to me like we really end up with resources allocated to __multiple__ 
roles if we were to allow accepting multiple offers from multiple roles.
That is, currently the resources in `Resources` and `Offer::Operation` are 
either all unallocated, or all allocated to one role.

Is this accurate?


src/common/resources.cpp (lines 1257 - 1261)


We talked about how this condition probably isn't necessary since we set 
the `AllocationInfo` in
`Offer::Operation`s in the master (and redundantly in the allocator).

Please let me know how that turned out!



src/common/resources.cpp (line 1339)


Shouldn't we print `unreserved` as opposed to `adjustedReservation` here?


- Michael Park


On Jan. 23, 2017, 2:47 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55828/
> ---
> 
> (Updated Jan. 23, 2017, 2:47 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 allocation role.
> The two interesting cases for adjusting the operation's resource are:
> 
> (1) The operation `Resource` does not contain an `AllocationInfo`
> but is being applied to an allocated `Resources`. We allow this
> only if the operation is unambiguous, that is, the allocated
> `Resources` are only allocated to a single role.
> 
> (2) The operation `Resource` contains an `AllocationInfo` but is
> being applied to an unallocated `Resources`. In this case we
> simply ignore the `AllocationInfo` of the `Resource`.
> 
> Note that we assume no `Resources` store a mix of allocated and
> unallocated resources. This is a brittle assumption that we should
> have enforcement for.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp be9bca2063e9f0e60c5faa0142077bea56272e45 
>   src/tests/resources_tests.cpp 8dfb1be35d9f9c6ff69139d055c6b3d3ec475e68 
>   src/tests/resources_utils.hpp 18dcca7f171102df8fe88f10785f70c5d1cf5b32 
>   src/v1/resources.cpp da4701c03020ff9c33ef995cd0af437d8827c267 
> 
> Diff: https://reviews.apache.org/r/55828/diff/
> 
> 
> Testing
> ---
> 
> Added a test.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>