Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 26, 2015, 10:55 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Changes --- Split out the allocator changes. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have enough resources after resciding all offers, be optimistic and forward the request to the allocator since there may be available resources to satisfy the request. * If the allocator returns a failure, report the error to the user with `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` maybe as well. We'll pick one eventually. This approach is clearly not ideal, since we would prefer to rescind as little offers as possible. The challenges of implementing the ideal solution in the current state is described in the document above. TODO(mpark): Add more comments and test cases. TODO(mpark): Return a `FutureNothing` rather than `FutureTryNothing` and use `Future::repair` to propagate the failure state. Diffs (updated) - src/Makefile.am a064d17a6b62e6e3c8e190135bcc8cbbb0051ed5 src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 26, 2015, 10:55 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Changes --- Finished TODO. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description (updated) --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have enough resources after resciding all offers, be optimistic and forward the request to the allocator since there may be available resources to satisfy the request. * If the allocator returns a failure, report the error to the user with `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` maybe as well. We'll pick one eventually. This approach is clearly not ideal, since we would prefer to rescind as little offers as possible. The challenges of implementing the ideal solution in the current state is described in the document above. TODO(mpark): Add more comments and test cases. Diffs - src/Makefile.am a064d17a6b62e6e3c8e190135bcc8cbbb0051ed5 src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 26, 2015, 4:44 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description (updated) --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have enough resources after resciding all offers, be optimistic and forward the request to the allocator since there may be available resources to satisfy the request. * If the allocator returns a failure, report the error to the user with `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` maybe as well. We'll pick one eventually. This approach is clearly not ideal, since we would prefer to rescind as little offers as possible. The challenges of implementing the ideal solution in the current state is described in the document above. TODO(mpark): Add more comments and test cases. TODO(mpark): Return a `FutureNothing` rather than `FutureTryNothing` and use `Future::repair` to propagate the failure state. Diffs (updated) - include/mesos/master/allocator.hpp 22992c0c77058af4fcd28aa8e4a1191693a16f44 src/Makefile.am a064d17a6b62e6e3c8e190135bcc8cbbb0051ed5 src/master/allocator/mesos/allocator.hpp 72470ec7f56f84a9a9815c09adb88def90ef672f src/master/allocator/mesos/hierarchical.hpp 3264d145d52b48852878abf7ab9be29ab98208cc src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review89471 --- Patch looks great! Reviews applied: [35714, 35702] All tests passed. - Mesos ReviewBot On June 26, 2015, 4:44 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 26, 2015, 4:44 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This involved a lot more challenges than I anticipated, I've captured the various approaches and limitations and deal-breakers of those approaches here: [Master Endpoint Implementation Challenges](https://docs.google.com/document/d/1cwVz4aKiCYP9Y4MOwHYZkyaiuEv7fArCye-vPvB2lAI/edit#) Key points: * This is a stop-gap solution until we shift the offer creation/management logic from the master to the allocator. * `updateAvailable` and `updateSlave` are kept separate because (1) `updateAvailable` is allowed to fail whereas `updateSlave` must not. (2) `updateAvailable` returns a `Future` whereas `updateSlave` does not. (3) `updateAvailable` never leaves the allocator in an over-allocated state and must not, whereas `updateSlave` does, and can. * The algorithm: * Initially, the master pessimistically assume that what seems like available resources will be gone. This is due to the race between the allocator scheduling an `allocate` call to itself vs master's `allocator-updateAvailable` invocation. As such, we first try to satisfy the request only with the offered resources. * We greedily rescind one offer at a time until we've rescinded sufficiently many offers. IMPORTANT: We perform `recoverResources(..., Filters())` rather than `recoverResources(..., None())` so that we can pretty much always win the race against `allocate`. In the case that we lose, no disaster occurs. We simply fail to satisfy the request. * If we still don't have enough resources after resciding all offers, be optimistic and forward the request to the allocator since there may be available resources to satisfy the request. * If the allocator returns a failure, report the error to the user with `PreconditionFailed`. This could be updated to be `Forbidden`, or `Conflict` maybe as well. We'll pick one eventually. This approach is clearly not ideal, since we would prefer to rescind as little offers as possible. The challenges of implementing the ideal solution in the current state is described in the document above. TODO(mpark): Add more comments and test cases. TODO(mpark): Return a `FutureNothing` rather than `FutureTryNothing` and use `Future::repair` to propagate the failure state. Diffs - include/mesos/master/allocator.hpp 22992c0c77058af4fcd28aa8e4a1191693a16f44 src/Makefile.am a064d17a6b62e6e3c8e190135bcc8cbbb0051ed5 src/master/allocator/mesos/allocator.hpp 72470ec7f56f84a9a9815c09adb88def90ef672f src/master/allocator/mesos/hierarchical.hpp 3264d145d52b48852878abf7ab9be29ab98208cc src/master/http.cpp 350383362311cfbc830965e1155a8515f0dfb332 src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 24, 2015, 3:39 p.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description (updated) --- Work currently in progress, incorporating feedback. Please hold off on reviewing this patch for now. (1) Q: I've added `updateAvailable` to the allocator API. Is this necessary or is there maybe a better way? A: I think we shold be able to generalize `updateSlave` to support not only `oversubscription`, but `reserve`, `unreserve`, `create` and `destroy`. (2) Q: To determine the amount of available resources, I used: `available = total - (offered + used)`. Is this still correct with oversubscription in the picture? A: I think this decision needs to be made in the allocator. (3) Q: I'm creating an `Offer.Operation` to perform the necessary updates in the allocator and such. Feels weird to use an offer operation when there's not an actual offer. Is this fine for now? A: I think we'll want to introduce something like `Resource.Operation`. Diffs - include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp 7097482fc0adad1c177c15c35edd51c10754f89c src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
On June 22, 2015, 10:20 a.m., Alexander Rukletsov wrote: Before making a thorough review, let's discuss one high level question. Here is the problem how I understand it: we reserve for roles, but our code works mostly with frameworks (allocator methods, Offer protobuf). To workaround this you decided to drop role from the reservation request at all. Is it right? Don't we want to update at least the role sorter? If you decide to add the role, we can preserve validation function for reservation and split `updateAllocation()` into two parts: role-specific and framework specific and use just role-specific variant for both operator-initiated reservations and framework-initiated reservations. Thoughts? Michael Park wrote: Thanks for the questions! Here is the problem how I understand it: we reserve for roles, but our code works mostly with frameworks (allocator methods, Offer protobuf). To workaround this you decided to drop role from the reservation request at all. Is it right? No, the desired roles are specified in the `resources` query parameter. If you're wondering why the validation for `Reserve` now takes an optional role, that's because the master endpoint doesn't have a `role` to which all `resources` need to match. Recall that a framework always has a `role` to which all specified resources need to match. Don't we want to update at least the role sorter? Yes, you're right about this. I do need to update the role sorter's total. If you decide to add the role, we can preserve validation function for reservation Hopefully my reply above clarifies the reason for the changes made to the `validate` function. ... and split `updateAllocation()` into two parts: role-specific and framework specific and use just role-specific variant for both operator-initiated reservations and framework-initiated reservations. There's a fundamental difference between operator-initiated reservations and framework-initiated reservations, in that operator-initiated reservations modify the available (unallocated) resources, whereas framework-initiated reservations modify the allocated resources. `updateAvailble` and `updateAllocation` handle each of the cases. The following are some of the differing characteristics: `updateAvailable` handles the former case: (1) the `available` resources change (2) `frameworkSorter` is unaffected (3) only the total of `roleSorter` is affected, the allocations are not. `updateAllocation` handles the latter case: (1) the `available` resources do not change. (2) both `frameworkSorter` and `roleSorter` are affected. No, the desired roles are specified in the resources query parameter. If you're wondering why the validation for Reserve now takes an optional role, that's because the master endpoint doesn't have a role to which all resources need to match. Recall that a framework always has a role to which all specified resources need to match. If I understand you correctly, you mean that an operator may reserve resources for multiple roles in one request (while the framework can't). First, you don't have a test for this case : ). Second, if we group resources by roles and call `applyOfferOperation()` per role, can we avoid modifying validation function? I think you'll need to do the grouping at some point in order to update `roleSorter`, so why not doing it earlier and therefore keep framework-initiated and operator-initiated requests closer to each other? You already group them implicitly by `SlaveID`. There's a fundamental difference between operator-initiated reservations and framework-initiated reservations, in that operator-initiated reservations modify the available (unallocated) resources, whereas framework-initiated reservations modify the allocated resources. updateAvailble and updateAllocation handle each of the cases. Let's capture this in the user guide! It also means that if all frameworks in the role are beyond their fare share, they are not able to dynamically reserve (because they are not getting offers), but an operator can always reserve, right? I also try to understand, whether we can refactor the code in a way that we do not introduce one more method on the allocator. Remember, it's a public interface and folks are supposed to write their own implementations, so we should be careful when changing the API. - Alexander --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review88760 --- On June 22, 2015, 5:29 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit:
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
On June 22, 2015, 10:20 a.m., Alexander Rukletsov wrote: Before making a thorough review, let's discuss one high level question. Here is the problem how I understand it: we reserve for roles, but our code works mostly with frameworks (allocator methods, Offer protobuf). To workaround this you decided to drop role from the reservation request at all. Is it right? Don't we want to update at least the role sorter? If you decide to add the role, we can preserve validation function for reservation and split `updateAllocation()` into two parts: role-specific and framework specific and use just role-specific variant for both operator-initiated reservations and framework-initiated reservations. Thoughts? Thanks for the questions! Here is the problem how I understand it: we reserve for roles, but our code works mostly with frameworks (allocator methods, Offer protobuf). To workaround this you decided to drop role from the reservation request at all. Is it right? No, the desired roles are specified in the `resources` query parameter. If you're wondering why the validation for `Reserve` now takes an optional role, that's because the master endpoint doesn't have a `role` to which all `resources` need to match. Recall that a framework always has a `role` to which all specified resources need to match. Don't we want to update at least the role sorter? Yes, you're right about this. I do need to update the role sorter's total. If you decide to add the role, we can preserve validation function for reservation Hopefully my reply above clarifies the reason for the changes made to the `validate` function. ... and split `updateAllocation()` into two parts: role-specific and framework specific and use just role-specific variant for both operator-initiated reservations and framework-initiated reservations. There's a fundamental difference between operator-initiated reservations and framework-initiated reservations, in that operator-initiated reservations modify the available (unallocated) resources, whereas framework-initiated reservations modify the allocated resources. `updateAvailble` and `updateAllocation` handle each of the cases. The following are some of the differing characteristics: `updateAvailable` handles the former case: (1) the `available` resources change (2) `frameworkSorter` is unaffected (3) only the total of `roleSorter` is affected, the allocations are not. `updateAllocation` handles the latter case: (1) the `available` resources do not change. (2) both `frameworkSorter` and `roleSorter` are affected. - Michael --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review88760 --- On June 22, 2015, 5:29 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 22, 2015, 5:29 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This is still a work in progress, but I'm sharing to gather some feedback around: (1) I've added `updateAvailable` to the allocator API. Is this necessary or is there maybe a better way? (2) To determine the amount of available resources, I used: `available = total - (offered + used)`. Is this still correct with oversubscription in the picture? (3) I'm creating an `Offer.Operation` to perform the necessary updates in the allocator and such. Feels weird to use an offer operation when there's not an actual offer. Is this fine for now? Diffs - include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp 7097482fc0adad1c177c15c35edd51c10754f89c src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review88760 --- Before making a thorough review, let's discuss one high level question. Here is the problem how I understand it: we reserve for roles, but our code works mostly with frameworks (allocator methods, Offer protobuf). To workaround this you decided to drop role from the reservation request at all. Is it right? Don't we want to update at least the role sorter? If you decide to add the role, we can preserve validation function for reservation and split `updateAllocation()` into two parts: role-specific and framework specific and use just role-specific variant for both operator-initiated reservations and framework-initiated reservations. Thoughts? src/master/http.cpp (line 553) https://reviews.apache.org/r/35702/#comment141344 s/recinding/rescinding src/master/http.cpp (line 560) https://reviews.apache.org/r/35702/#comment141345 s/Recinding/rescinding/ - Alexander Rukletsov On June 22, 2015, 5:29 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 22, 2015, 5:29 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This is still a work in progress, but I'm sharing to gather some feedback around: (1) I've added `updateAvailable` to the allocator API. Is this necessary or is there maybe a better way? (2) To determine the amount of available resources, I used: `available = total - (offered + used)`. Is this still correct with oversubscription in the picture? (3) I'm creating an `Offer.Operation` to perform the necessary updates in the allocator and such. Feels weird to use an offer operation when there's not an actual offer. Is this fine for now? Diffs - include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp 7097482fc0adad1c177c15c35edd51c10754f89c src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/#review88781 --- src/master/http.cpp (lines 519 - 520) https://reviews.apache.org/r/35702/#comment141370 Some food for thought here. I think the ultimate decision whether to grant a request or not, should be taken by the allocator. Imagine a funky allocator module that is utterly optimistic and prefers granting requests and then killing tasks. With quota (what we have discussed offline) things go even further: a master cannot decide wether a quota can be fulfilled. We should either expose a lot of stats from allocator so that master can make a decision or we should change allocator interface so that it allows querying. src/master/master.cpp (line 749) https://reviews.apache.org/r/35702/#comment141371 I think reserve is too abstract and may collide with future actions (think quota). How about `/dynamic/reserve`? - Alexander Rukletsov On June 22, 2015, 5:29 a.m., Michael Park wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 22, 2015, 5:29 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This is still a work in progress, but I'm sharing to gather some feedback around: (1) I've added `updateAvailable` to the allocator API. Is this necessary or is there maybe a better way? (2) To determine the amount of available resources, I used: `available = total - (offered + used)`. Is this still correct with oversubscription in the picture? (3) I'm creating an `Offer.Operation` to perform the necessary updates in the allocator and such. Feels weird to use an offer operation when there's not an actual offer. Is this fine for now? Diffs - include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp 7097482fc0adad1c177c15c35edd51c10754f89c src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 22, 2015, 2:35 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Changes --- Added tests for the error cases. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This is still a work in progress, but I'm sharing to gather some feedback around: (1) I've added `updateAvailable` to the allocator API. Is this necessary or is there maybe a better way? (2) To determine the amount of available resources, I used: `available = total - (offered + used)`. Is this still correct with oversubscription in the picture? (3) I'm creating an `Offer.Operation` to perform the necessary updates in the allocator and such. Feels weird to use an offer operation when there's not an actual offer. Is this fine for now? Diffs (updated) - include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 src/Makefile.am e7de0f3d1a5efeaef47d5074defe3b40db94f573 src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp 646ee8c1c0fb824e1d17150b4e96e6281c65358f src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp` which still need to be fleshed out for error cases (missing parameters, unauthorized client, etc). Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 22, 2015, 5:29 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This is still a work in progress, but I'm sharing to gather some feedback around: (1) I've added `updateAvailable` to the allocator API. Is this necessary or is there maybe a better way? (2) To determine the amount of available resources, I used: `available = total - (offered + used)`. Is this still correct with oversubscription in the picture? (3) I'm creating an `Offer.Operation` to perform the necessary updates in the allocator and such. Feels weird to use an offer operation when there's not an actual offer. Is this fine for now? Diffs (updated) - include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp 7097482fc0adad1c177c15c35edd51c10754f89c src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
Re: Review Request 35702: [WIP] Added /reserve HTTP endpoint to the master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35702/ --- (Updated June 22, 2015, 5:11 a.m.) Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, Joris Van Remoortere, and Vinod Kone. Changes --- Rebased. Bugs: MESOS-2600 https://issues.apache.org/jira/browse/MESOS-2600 Repository: mesos Description --- This is still a work in progress, but I'm sharing to gather some feedback around: (1) I've added `updateAvailable` to the allocator API. Is this necessary or is there maybe a better way? (2) To determine the amount of available resources, I used: `available = total - (offered + used)`. Is this still correct with oversubscription in the picture? (3) I'm creating an `Offer.Operation` to perform the necessary updates in the allocator and such. Feels weird to use an offer operation when there's not an actual offer. Is this fine for now? Diffs (updated) - include/mesos/master/allocator.hpp 92de1af414321281b00eaa6f129e5e3e2c849448 src/Makefile.am dfebd2b14c9cb45c437509809fdf5ac3b0c8838c src/master/allocator/mesos/allocator.hpp 6cfa04650d91a80211cfbc0809236f9438926c78 src/master/allocator/mesos/hierarchical.hpp 7097482fc0adad1c177c15c35edd51c10754f89c src/master/http.cpp b893013ddd052cb58c520ac0328f4a5f0fed862e src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 src/master/master.cpp 0135c155181546d3cb43e9e05bb874af846d928d src/master/validation.hpp 469d6f56c3de28a34177124aae81ce24cb4ad160 src/master/validation.cpp 9d128aa1b349b018b8e4a1916434d848761ca051 src/tests/mesos.hpp 9157ac079808d2686592e54ea26a26e6a0825ed3 src/tests/reserve_tests.cpp PRE-CREATION Diff: https://reviews.apache.org/r/35702/diff/ Testing --- Added `src/tests/reserve_tests.cpp`. Thanks, Michael Park
