-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65106/
-----------------------------------------------------------
(Updated Jan. 23, 2018, 12:11 a.m.)
Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
Changes
-------
Updated per vinod's review.
Bugs: MESOS-8389
https://issues.apache.org/jira/browse/MESOS-8389
Repository: mesos
Description
-------
In the past, the notion of a "removable" task meant: the task is
terminal and acknowledged. However, the `isRemovable` helper only
defines removability by the task state (terminal or unreachable)
but not whether the terminal update is acknowledged.
As a result, the code that is calling `isRemovable` is unintuitive.
One example of a confusing piece of code is within updateTask. Here,
we have logic which says, if the task is removable, recover the
resources but don't remove it. This seems more intuitive if directly
described as: "if the task is no longer consuming resources (e.g.
transitioned to terminal or unreachable) then recover the resources".
If one looks up the documentation of `isRemovable`, it says "When a
task becomes removable, it is erased from the master's primary task
data structures", but that isn't accurate since this function doesn't
say whether the terminal task has been acknowledged, which is
required for a task to be removable.
To make an immediate improvement, this patch removes `isRemovable`
(no pun intended) in favor of directly checking the states we care
about. In the future, we may want to introduce some helpers like
`isAllocatedResources` to describe whether the task's resources are
considered allocated.
If we do introduce a notion of `isRemovable` in the future, it seems
it should take the `Task*` so that it can say whether the task could
be "removed" from the master, which includes checking that terminal
tasks have been acknowledged. However, "remove" is somewhat of a
confusing term, since what we're actually doing is "completing" the
task.
Diffs (updated)
-----
src/master/master.hpp 05136781b6b1539f37c283e8127e4bafb187a0d1
src/master/master.cpp 3af96b1d2024ab1b951537ebc6bbc225cfa9cc88
Diff: https://reviews.apache.org/r/65106/diff/3/
Changes: https://reviews.apache.org/r/65106/diff/2-3/
Testing
-------
make check
Thanks,
Benjamin Mahler
