You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@mesos.apache.org by "Yan Xu (JIRA)" <ji...@apache.org> on 2016/04/26 01:20:13 UTC

[jira] [Created] (MESOS-5280) Inconsistent error checking in DRF sorter.

Yan Xu created MESOS-5280:
-----------------------------

             Summary: Inconsistent error checking in DRF sorter.
                 Key: MESOS-5280
                 URL: https://issues.apache.org/jira/browse/MESOS-5280
             Project: Mesos
          Issue Type: Bug
          Components: allocation
            Reporter: Yan Xu
            Assignee: Yan Xu


There exist a few different error handling styles in the sorter.

h2. Hard checks
e.g., [DRFSorter::update|https://github.com/apache/mesos/blob/c530deb3050d862fd894a9c4ed0a8ddca8714a63/src/master/allocator/sorter/drf/sorter.cpp#L62]
{code}
CHECK(weights.contains(name));
{code}

h2. No-op if it results in an error condition.
e.g., [DRFSorter::allocated|https://github.com/apache/mesos/blob/c530deb3050d862fd894a9c4ed0a8ddca8714a63/src/master/allocator/sorter/drf/sorter.cpp#L116]:
{code}
set<Client, DRFComparator>::iterator it = find(name);

if (it != clients.end()) { // TODO(benh): This should really be a CHECK.
...
}
{code}

IMO there should never be silent no-ops. Short of CHECK, we should return an error if it's indeed an error. If either path of the branch is valid and one is a  noop, we should log the noop branch or return a 'bool' so the caller can distinguish the two.

Implicitness makes it hard to debug things and we have run into one instance of this.

My proposal is to use CHECKs consistently in sorter.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)