This is an automated email from the ASF dual-hosted git repository.
chhsiao pushed a commit to branch 1.8.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 7e43f1fcfab8983f102ba79419a81f62fba677d8
Author: Chun-Hung Hsiao <chhs...@mesosphere.io>
AuthorDate: Mon Apr 22 15:46:36 2019 -0700
Do not implicitly decline speculatively converted resources.
Currently if a framework accepts an offer with a `RESERVE` operation
without a task consuming the reserved resources, the resources will be
implicitly declined. This is counter to what one would expect (that only
the remaining resources in the offer will be declined):
Offer `cpus:10` -> `ACCEPT` with `RESERVE cpus(role):1`
*Actual* implicit decline: `cpus:9;cpus(role):1`
*Expected* implicit decline: `cpus:9`
The same issue is present with other transformational operations (i.e.,
`UNRESERVE`, `CREATE` and `DESTROY`).
This patch fixes this issue by only implicitly declining the "remaining"
untransformed resources, computed as follows:
Offered = `cpus:10`
Remaining = `cpus:9;cpus(role):1`
Implicitly declined = remaining - (remaining - offered)
= `cpus:9;cpus(role):1` - `cpus:(role):1`
= `cpus:9`
Review: https://reviews.apache.org/r/70132
---
src/master/master.cpp | 49 +++++++++++++++++++++++++++++++++++++++--------
src/tests/slave_tests.cpp | 3 ++-
2 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/src/master/master.cpp b/src/master/master.cpp
index ad54ae2..555136e 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -5959,17 +5959,50 @@ void Master::_accept(
conversions);
}
+ // We now need to compute the amounts of remaining (1) speculatively
converted
+ // resources to recover without a filter and (2) resources that are
implicitly
+ // declined with the filter:
+ //
+ // Speculatively converted resources
+ // = (offered resources).apply(speculative operations)
+ // - resources consumed by non-speculative operations
+ // - offered resources not consumed by any operation
+ // = `_offeredResources` - offered resources not consumed by any operation
+ // = `_offeredResources` - offered resources
+ //
+ // (The last equality holds because resource subtraction yields no
negatives.)
+ //
+ // Implicitly declined resources
+ // = (offered resources).apply(speculative operations)
+ // - resources consumed by non-speculative operations
+ // - speculatively converted resources
+ // = `_offeredResources` - speculatively converted resources
+ //
+ // TODO(zhitao): Right now `GROW_VOLUME` and `SHRINK_VOLUME` are implemented
+ // as speculative operations. Since the plan is to make them non-speculative
+ // in the future, their results are not in `_offeredResources`, so we add
them
+ // back here. Remove this once the operations become non-speculative.
+ Resources speculativelyConverted =
+ _offeredResources + resizedResources - offeredResources;
+ Resources implicitlyDeclined = _offeredResources - speculativelyConverted;
+
+ // Prevent any allocations from occurring during resource recovery below.
+ allocator->pause();
- // TODO(zhitao): Remove `resizedResources` once `GROW_VOLUME` and
- // `SHRINK_VOLUME` become non-speculative.
- if (!_offeredResources.empty() || !resizedResources.empty()) {
- // Tell the allocator about the unused (e.g., refused) resources.
+ // Tell the allocator about the net speculatively converted resources. These
+ // resources should not be implicitly declined.
+ if (!speculativelyConverted.empty()) {
allocator->recoverResources(
- frameworkId,
- slaveId,
- _offeredResources + resizedResources,
- accept.filters());
+ frameworkId, slaveId, speculativelyConverted, None());
+ }
+
+ // Tell the allocator about the implicitly declined resources.
+ if (!implicitlyDeclined.empty()) {
+ allocator->recoverResources(
+ frameworkId, slaveId, implicitlyDeclined, accept.filters());
}
+
+ allocator->resume();
}
diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp
index 019dbd7..50882a5 100644
--- a/src/tests/slave_tests.cpp
+++ b/src/tests/slave_tests.cpp
@@ -6495,7 +6495,8 @@ TEST_F(SlaveTest, UpdateOperationStatusRetry)
Future<v1::scheduler::Event::Offers> offers;
EXPECT_CALL(*scheduler, offers(_, _))
- .WillOnce(FutureArg<1>(&offers));
+ .WillOnce(FutureArg<1>(&offers))
+ .WillRepeatedly(Return()); // Ignore subsequent offers.
ContentType contentType = ContentType::PROTOBUF;
