You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anindya Sinha <an...@apple.com> on 2017/04/17 23:14:25 UTC
Review Request 58486: Fixed a race in `updateAllocation()` on DESTORY
of a shared volume.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
Review request for mesos, James Peach and Jiang Yan Xu.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description
-------
In `updateAllocation()`, it is possible for the flattened scalar
quantites of original framework allocation to differ from the updated
framework allocation. This can happen when an offer with a shared
persistent volume is sent out after a DESTROY has been received. If
that is the case, we rescind the pending offers from all frameworks
which contain the volumes to be destroyed.
Diffs
-----
include/mesos/allocator/allocator.hpp 6eda1b8619269c1501a935045b18b1deaf845b33
src/master/allocator/mesos/allocator.hpp 57b54b86c43c7731e64d422d285c4b8ca7e27a60
src/master/allocator/mesos/hierarchical.hpp f84b0574ce9a392c9528c87b04b01dbb2053cff7
src/master/allocator/mesos/hierarchical.cpp 051f749dd5921a322ca930a042c31814616d38f9
src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad
src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d
src/tests/hierarchical_allocator_tests.cpp 33e7b455f8664858eb4f03727b076a10c80cd6e0
Diff: https://reviews.apache.org/r/58486/diff/1/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Anindya Sinha <an...@apple.com>.
> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > I feel the following may be more clear and general at high level.
> >
> > - In `_accept()` all operations are validated, for resource operations we call `updateAllocation` for each individual operation [like we used to do](https://github.com/apache/mesos/blob/e7fcc4fbc09122480c8f2549a8af099769e888c0/src/master/master.cpp#L3770).
> > - We fail `updateAllocate` if the operation cannot be applied but leave the allocator in a good state.
> > - Individual resource operations are sent to the agent only when `updateAllocate` succeeds, otherwise the operation is dropped with no bad side effects.
> >
> > It'll be also great if we can come up with a test?
The overall approach is as follows (based on your comments and F2F discussions):
1) `Resources::apply()` fails if a `DESTROY` is applied on a `Resources` object with shared count != 1 (https://reviews.apache.org/r/59194/, https://issues.apache.org/jira/browse/MESOS-7403).
2) `updateAllocation()` returns a failed future if any operation cannot be applied to `slave.allocated` (https://reviews.apache.org/r/59195/).
3) All valid operations received as a part of `ACCEPT` call `updateAllocation()` individually. Since `updateAllocation()` now returns a `Future<Nothing>`, we proceed to the next step in processing of `ACCEPT` once we get back the futures from `updateAllocation()` for each of the valid operations.
3a) For each of the non-failed futures (from step 3):
i) For `LAUNCH` and `LAUNCH_GROUP`: We send the `RunTaskMessage` or `RunTaskGroupMessage` to the agent.
ii) For all other operations, we send the `CheckpointedResourcesMessage` to the agent.
3b) For each of failed futures, we drop that operation.
4) Recover resources as applicable based on the successful allocation updates for the valid operations.
I will add tests in subsequent review in this chain.
> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 746-749 (original), 746-749 (patched)
> > <https://reviews.apache.org/r/58486/diff/2/?file=1693990#file1693990line746>
> >
> > Below here we can use the same mechanism as done in `updateAvailable`:
> >
> > ```
> > // In addition to asserting the operation can be applied to the offered
> > // resources above, here we also check if it can be applied to the agent's
> > // current allocated resources. We do this because certain operations
> > // require that they are applicable to the agent's allocated resources
> > // as a whole. One such example is that a shared persistent volume can
> > // only be destroyed by a framework when it is not allocated to other
> > // frameworks (see `Resources::apply` for details). The master already does
> > // best-effort validation but this check may still fail due to additional
> > // `allocate` runs. In a race condition an `allocate` could have been
> > // enqueued by the allocator itself just before master's request to
> > // enqueue `updateAllocation` arrives to the allocator. This is similar
> > // to the siutation in `updateAvailable`, see the diagram there.
> > Try<Resources> updatedAllocated = slave.allocated.apply(operation);
> > if (updatedAllocated.isError()) {
> > return Failure(updatedAllocated.error());
> > }
> > ```
> >
> > We need to comment clearly on why we need this. Above I tried to state the general idea but also gave an concrete example but this is just a first draft.
> >
> >
> > For this to work we need to make sure `Resources::apply` fails in such a case: https://issues.apache.org/jira/browse/MESOS-7403
> >
> > But now the failure would occur in the right place in this method (not applicable to the current allocated resources) and the error message returned by `Resources::apply` would be able to explain what exactly happened.
Fixed in https://reviews.apache.org/r/59195/.
> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 848-851 (original)
> > <https://reviews.apache.org/r/58486/diff/2/?file=1693990#file1693990line848>
> >
> > This being at the bottom of the method was intended to validate the post-condition. If this doesn't hold, ideally we shouldn't capture it here because we don't know how it has failed. We should capture the failure at the individual step and construct an error message/log that's explicit about it.
Updated in https://reviews.apache.org/r/59195/.
> On April 20, 2017, 6:17 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp
> > Lines 4774-4775 (patched)
> > <https://reviews.apache.org/r/58486/diff/2/?file=1693992#file1693992line4775>
> >
> > So if the failure has occurred, the allocation in the allocator is already wrong. e.g., it can have both a volume of 1MB (which didn't get removed) and the plain disk of 1MB (which got added assuming the volume was removed).
> >
> > Ideally when a failure happens it leaves the allocator in the previous good state but here we are making an attempt to asynchronously compensate that and to correct it. Many events can be executed on the allocator between the two.
> >
> > While it's true that no new supposedly destroyed volumes can be offered out due to being removed from `slave.total` in the allocator, the allocator is itself running and the framework's incorrect allocation can cause issue, e.g., fairness, although there are probably more severe problems.
Updated based on the description of the new revision.
- Anindya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/#review172448
-----------------------------------------------------------
On May 11, 2017, 8:21 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58486/
> -----------------------------------------------------------
>
> (Updated May 11, 2017, 8:21 p.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator is updated for each offer operation in the order specified
> in the `ACCEPT` call. Once the allocator is updated successfully, we
> handle subsequent operations as follows:
>
> 1) Launch or Launch Group: We launch the tasks or task group on the
> agent.
> 2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
> the `CheckPointResourcesMessage` to the agent.
>
> If allocation for any of the operations fail, those resources are not
> recovered from the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
>
>
> Diff: https://reviews.apache.org/r/58486/diff/4/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 58486: Fixed a race in `updateAllocation()` on
DESTORY of a shared volume.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/#review172448
-----------------------------------------------------------
I feel the following may be more clear and general at high level.
- In `_accept()` all operations are validated, for resource operations we call `updateAllocation` for each individual operation [like we used to do](https://github.com/apache/mesos/blob/e7fcc4fbc09122480c8f2549a8af099769e888c0/src/master/master.cpp#L3770).
- We fail `updateAllocate` if the operation cannot be applied but leave the allocator in a good state.
- Individual resource operations are sent to the agent only when `updateAllocate` succeeds, otherwise the operation is dropped with no bad side effects.
It'll be also great if we can come up with a test?
src/master/allocator/mesos/hierarchical.cpp
Lines 746-749 (original), 746-749 (patched)
<https://reviews.apache.org/r/58486/#comment245633>
Below here we can use the same mechanism as done in `updateAvailable`:
```
// In addition to asserting the operation can be applied to the offered
// resources above, here we also check if it can be applied to the agent's
// current allocated resources. We do this because certain operations
// require that they are applicable to the agent's allocated resources
// as a whole. One such example is that a shared persistent volume can
// only be destroyed by a framework when it is not allocated to other
// frameworks (see `Resources::apply` for details). The master already does
// best-effort validation but this check may still fail due to additional
// `allocate` runs. In a race condition an `allocate` could have been
// enqueued by the allocator itself just before master's request to
// enqueue `updateAllocation` arrives to the allocator. This is similar
// to the siutation in `updateAvailable`, see the diagram there.
Try<Resources> updatedAllocated = slave.allocated.apply(operation);
if (updatedAllocated.isError()) {
return Failure(updatedAllocated.error());
}
```
We need to comment clearly on why we need this. Above I tried to state the general idea but also gave an concrete example but this is just a first draft.
For this to work we need to make sure `Resources::apply` fails in such a case: https://issues.apache.org/jira/browse/MESOS-7403
But now the failure would occur in the right place in this method (not applicable to the current allocated resources) and the error message returned by `Resources::apply` would be able to explain what exactly happened.
src/master/allocator/mesos/hierarchical.cpp
Lines 848-851 (original)
<https://reviews.apache.org/r/58486/#comment245570>
This being at the bottom of the method was intended to validate the post-condition. If this doesn't hold, ideally we shouldn't capture it here because we don't know how it has failed. We should capture the failure at the individual step and construct an error message/log that's explicit about it.
src/master/master.cpp
Line 4355 (original), 4355 (patched)
<https://reviews.apache.org/r/58486/#comment245636>
We should proably resurrect the previous
```
void Master::apply(
Framework* framework,
Slave* slave,
const Offer::Operation& operation)
{
CHECK_NOTNULL(framework);
CHECK_NOTNULL(slave);
allocator->updateAllocation(framework->id(), slave->id, {operation});
_apply(slave, operation);
}
```
https://github.com/apache/mesos/blob/57a7e7d0283aa08455d6572ade75a11d914c6962/src/master/master.cpp#L5729-L5740
but make it async so we only call `_apply` when the result from `updateAllocation` is successful.
Then we call `updateAllocation` with just this single operation so when it fails it's due to this single operation.
src/master/master.cpp
Lines 4774-4775 (patched)
<https://reviews.apache.org/r/58486/#comment245574>
So if the failure has occurred, the allocation in the allocator is already wrong. e.g., it can have both a volume of 1MB (which didn't get removed) and the plain disk of 1MB (which got added assuming the volume was removed).
Ideally when a failure happens it leaves the allocator in the previous good state but here we are making an attempt to asynchronously compensate that and to correct it. Many events can be executed on the allocator between the two.
While it's true that no new supposedly destroyed volumes can be offered out due to being removed from `slave.total` in the allocator, the allocator is itself running and the framework's incorrect allocation can cause issue, e.g., fairness, although there are probably more severe problems.
- Jiang Yan Xu
On April 18, 2017, 7:50 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58486/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 7:50 p.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
>
>
> Repository: mesos
>
>
> Description
> -------
>
> In `updateAllocation()`, it is possible for the flattened scalar
> quantites of original framework allocation to differ from the updated
> framework allocation. This can happen when an offer with a shared
> persistent volume is sent out after a DESTROY has been received. If
> that is the case, we rescind the pending offers from all frameworks
> which contain the volumes to be destroyed.
>
>
> Diffs
> -----
>
> include/mesos/allocator/allocator.hpp 6eda1b8619269c1501a935045b18b1deaf845b33
> src/master/allocator/mesos/allocator.hpp 57b54b86c43c7731e64d422d285c4b8ca7e27a60
> src/master/allocator/mesos/hierarchical.hpp f84b0574ce9a392c9528c87b04b01dbb2053cff7
> src/master/allocator/mesos/hierarchical.cpp 051f749dd5921a322ca930a042c31814616d38f9
> src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
> src/master/master.cpp 52de2f91bdacf46f913c27382ad50b4f278ad297
> src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d
> src/tests/hierarchical_allocator_tests.cpp 33e7b455f8664858eb4f03727b076a10c80cd6e0
>
>
> Diff: https://reviews.apache.org/r/58486/diff/2/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/#review175227
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59194, 59195, 58486]
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 May 17, 2017, 2 a.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58486/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 2 a.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator is updated for each offer operation in the order specified
> in the `ACCEPT` call. Once the allocator is updated successfully, we
> handle subsequent operations as follows:
>
> 1) Launch or Launch Group: We launch the tasks or task group on the
> agent.
> 2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
> the `CheckPointResourcesMessage` to the agent.
>
> If allocation for any of the operations fail, those resources are not
> recovered from the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
>
>
> Diff: https://reviews.apache.org/r/58486/diff/7/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/#review175297
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59194, 59195, 58486]
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 May 17, 2017, 4:50 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58486/
> -----------------------------------------------------------
>
> (Updated May 17, 2017, 4:50 p.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator is updated for each offer operation in the order specified
> in the `ACCEPT` call. Once the allocator is updated successfully, we
> handle subsequent operations as follows:
>
> 1) Launch or Launch Group: We launch the tasks or task group on the
> agent.
> 2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
> the `CheckPointResourcesMessage` to the agent.
>
> If allocation for any of the operations fail, those resources are not
> recovered from the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
>
>
> Diff: https://reviews.apache.org/r/58486/diff/8/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
(Updated May 17, 2017, 4:50 p.m.)
Review request for mesos, James Peach and Jiang Yan Xu.
Changes
-------
Do not recalculate resources to be recovered if there is no error in `updateAllocation()`.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description
-------
The allocator is updated for each offer operation in the order specified
in the `ACCEPT` call. Once the allocator is updated successfully, we
handle subsequent operations as follows:
1) Launch or Launch Group: We launch the tasks or task group on the
agent.
2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
the `CheckPointResourcesMessage` to the agent.
If allocation for any of the operations fail, those resources are not
recovered from the allocator.
Diffs (updated)
-----
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
Diff: https://reviews.apache.org/r/58486/diff/8/
Changes: https://reviews.apache.org/r/58486/diff/7-8/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
(Updated May 17, 2017, 2 a.m.)
Review request for mesos, James Peach and Jiang Yan Xu.
Changes
-------
Validate that the valid operations can be processed in `Master::__accept()`.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description
-------
The allocator is updated for each offer operation in the order specified
in the `ACCEPT` call. Once the allocator is updated successfully, we
handle subsequent operations as follows:
1) Launch or Launch Group: We launch the tasks or task group on the
agent.
2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
the `CheckPointResourcesMessage` to the agent.
If allocation for any of the operations fail, those resources are not
recovered from the allocator.
Diffs (updated)
-----
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
Diff: https://reviews.apache.org/r/58486/diff/7/
Changes: https://reviews.apache.org/r/58486/diff/6-7/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/#review175055
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59194, 59195, 58486]
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 May 15, 2017, 6:01 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58486/
> -----------------------------------------------------------
>
> (Updated May 15, 2017, 6:01 p.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator is updated for each offer operation in the order specified
> in the `ACCEPT` call. Once the allocator is updated successfully, we
> handle subsequent operations as follows:
>
> 1) Launch or Launch Group: We launch the tasks or task group on the
> agent.
> 2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
> the `CheckPointResourcesMessage` to the agent.
>
> If allocation for any of the operations fail, those resources are not
> recovered from the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
>
>
> Diff: https://reviews.apache.org/r/58486/diff/6/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
(Updated May 15, 2017, 6:01 p.m.)
Review request for mesos, James Peach and Jiang Yan Xu.
Changes
-------
Rebased.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description
-------
The allocator is updated for each offer operation in the order specified
in the `ACCEPT` call. Once the allocator is updated successfully, we
handle subsequent operations as follows:
1) Launch or Launch Group: We launch the tasks or task group on the
agent.
2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
the `CheckPointResourcesMessage` to the agent.
If allocation for any of the operations fail, those resources are not
recovered from the allocator.
Diffs (updated)
-----
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
Diff: https://reviews.apache.org/r/58486/diff/6/
Changes: https://reviews.apache.org/r/58486/diff/5-6/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/#review174892
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59194, 59195, 58486]
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 May 13, 2017, 4:51 a.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58486/
> -----------------------------------------------------------
>
> (Updated May 13, 2017, 4:51 a.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator is updated for each offer operation in the order specified
> in the `ACCEPT` call. Once the allocator is updated successfully, we
> handle subsequent operations as follows:
>
> 1) Launch or Launch Group: We launch the tasks or task group on the
> agent.
> 2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
> the `CheckPointResourcesMessage` to the agent.
>
> If allocation for any of the operations fail, those resources are not
> recovered from the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
>
>
> Diff: https://reviews.apache.org/r/58486/diff/5/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
(Updated May 13, 2017, 4:51 a.m.)
Review request for mesos, James Peach and Jiang Yan Xu.
Changes
-------
Rebased.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description
-------
The allocator is updated for each offer operation in the order specified
in the `ACCEPT` call. Once the allocator is updated successfully, we
handle subsequent operations as follows:
1) Launch or Launch Group: We launch the tasks or task group on the
agent.
2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
the `CheckPointResourcesMessage` to the agent.
If allocation for any of the operations fail, those resources are not
recovered from the allocator.
Diffs (updated)
-----
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
Diff: https://reviews.apache.org/r/58486/diff/5/
Changes: https://reviews.apache.org/r/58486/diff/4-5/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/#review174780
-----------------------------------------------------------
Patch looks great!
Reviews applied: [59194, 59195, 58486]
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 May 11, 2017, 8:21 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58486/
> -----------------------------------------------------------
>
> (Updated May 11, 2017, 8:21 p.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The allocator is updated for each offer operation in the order specified
> in the `ACCEPT` call. Once the allocator is updated successfully, we
> handle subsequent operations as follows:
>
> 1) Launch or Launch Group: We launch the tasks or task group on the
> agent.
> 2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
> the `CheckPointResourcesMessage` to the agent.
>
> If allocation for any of the operations fail, those resources are not
> recovered from the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
> src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
>
>
> Diff: https://reviews.apache.org/r/58486/diff/4/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 58486: Update the allocator on a per offer
operation.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
(Updated May 11, 2017, 8:21 p.m.)
Review request for mesos, James Peach and Jiang Yan Xu.
Summary (updated)
-----------------
Update the allocator on a per offer operation.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description (updated)
-------
The allocator is updated for each offer operation in the order specified
in the `ACCEPT` call. Once the allocator is updated successfully, we
handle subsequent operations as follows:
1) Launch or Launch Group: We launch the tasks or task group on the
agent.
2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
the `CheckPointResourcesMessage` to the agent.
If allocation for any of the operations fail, those resources are not
recovered from the allocator.
Diffs (updated)
-----
src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732
src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad
Diff: https://reviews.apache.org/r/58486/diff/4/
Changes: https://reviews.apache.org/r/58486/diff/3-4/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 58486: Fixed a race in `updateAllocation()` on
DESTORY of a shared volume.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
(Updated April 28, 2017, 3:42 p.m.)
Review request for mesos, James Peach and Jiang Yan Xu.
Changes
-------
Rebased. Yet to address the review comments.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description
-------
In `updateAllocation()`, it is possible for the flattened scalar
quantites of original framework allocation to differ from the updated
framework allocation. This can happen when an offer with a shared
persistent volume is sent out after a DESTROY has been received. If
that is the case, we rescind the pending offers from all frameworks
which contain the volumes to be destroyed.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp 6eda1b8619269c1501a935045b18b1deaf845b33
src/master/allocator/mesos/allocator.hpp 57b54b86c43c7731e64d422d285c4b8ca7e27a60
src/master/allocator/mesos/hierarchical.hpp 219f508dacb3b372c12da3f15e07a3bf5f27e6e6
src/master/allocator/mesos/hierarchical.cpp 84dc31dd6e567e9316a73c61e03a1daf6f556829
src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
src/master/master.cpp e8c2a96ff3407fb429e60cd9e66a8c4dc52b391b
src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d
src/tests/hierarchical_allocator_tests.cpp 84bb6f302f6ff6f8a93fa891795df9ef7380629f
Diff: https://reviews.apache.org/r/58486/diff/3/
Changes: https://reviews.apache.org/r/58486/diff/2-3/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 58486: Fixed a race in `updateAllocation()` on
DESTORY of a shared volume.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58486/
-----------------------------------------------------------
(Updated April 19, 2017, 2:50 a.m.)
Review request for mesos, James Peach and Jiang Yan Xu.
Bugs: MESOS-7308
https://issues.apache.org/jira/browse/MESOS-7308
Repository: mesos
Description
-------
In `updateAllocation()`, it is possible for the flattened scalar
quantites of original framework allocation to differ from the updated
framework allocation. This can happen when an offer with a shared
persistent volume is sent out after a DESTROY has been received. If
that is the case, we rescind the pending offers from all frameworks
which contain the volumes to be destroyed.
Diffs (updated)
-----
include/mesos/allocator/allocator.hpp 6eda1b8619269c1501a935045b18b1deaf845b33
src/master/allocator/mesos/allocator.hpp 57b54b86c43c7731e64d422d285c4b8ca7e27a60
src/master/allocator/mesos/hierarchical.hpp f84b0574ce9a392c9528c87b04b01dbb2053cff7
src/master/allocator/mesos/hierarchical.cpp 051f749dd5921a322ca930a042c31814616d38f9
src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034
src/master/master.cpp 52de2f91bdacf46f913c27382ad50b4f278ad297
src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d
src/tests/hierarchical_allocator_tests.cpp 33e7b455f8664858eb4f03727b076a10c80cd6e0
Diff: https://reviews.apache.org/r/58486/diff/2/
Changes: https://reviews.apache.org/r/58486/diff/1-2/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha