Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/#review86783 --- Patch looks great! Reviews applied: [32961, 33159] All tests passed. - Mesos ReviewBot On June 5, 2015, 8:53 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated June 5, 2015, 8:53 a.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs - include/mesos/master/allocator.hpp 8347cfaf6abce155a777484970e595e6b141bb87 src/master/allocator/mesos/allocator.hpp b57b03daf992082ec7c73199ab2574bf7993 src/master/allocator/mesos/hierarchical.hpp 44fbccaf72b65251095f69b68627d0efa58707d4 src/master/master.cpp be0db42da3c59761aa154439653d715556465256 src/tests/mesos.hpp 86660ac4d7476bc6cc077fc2d474c4a9ac81c031 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/#review86781 --- Ship it! src/master/allocator/mesos/hierarchical.hpp https://reviews.apache.org/r/33159/#comment138859 CHECK_EQ - Benjamin Hindman On June 5, 2015, 8:53 a.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated June 5, 2015, 8:53 a.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs - include/mesos/master/allocator.hpp 8347cfaf6abce155a777484970e595e6b141bb87 src/master/allocator/mesos/allocator.hpp b57b03daf992082ec7c73199ab2574bf7993 src/master/allocator/mesos/hierarchical.hpp 44fbccaf72b65251095f69b68627d0efa58707d4 src/master/master.cpp be0db42da3c59761aa154439653d715556465256 src/tests/mesos.hpp 86660ac4d7476bc6cc077fc2d474c4a9ac81c031 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated June 5, 2015, 8:53 a.m.) Review request for mesos and Benjamin Hindman. Changes --- rebased. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs (updated) - include/mesos/master/allocator.hpp 8347cfaf6abce155a777484970e595e6b141bb87 src/master/allocator/mesos/allocator.hpp b57b03daf992082ec7c73199ab2574bf7993 src/master/allocator/mesos/hierarchical.hpp 44fbccaf72b65251095f69b68627d0efa58707d4 src/master/master.cpp be0db42da3c59761aa154439653d715556465256 src/tests/mesos.hpp 86660ac4d7476bc6cc077fc2d474c4a9ac81c031 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated June 5, 2015, 1:35 p.m.) Review request for mesos and Benjamin Hindman. Changes --- switched CHECK to CHECK_EQ Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs (updated) - include/mesos/master/allocator.hpp 8347cfaf6abce155a777484970e595e6b141bb87 src/master/allocator/mesos/allocator.hpp b57b03daf992082ec7c73199ab2574bf7993 src/master/allocator/mesos/hierarchical.hpp 44fbccaf72b65251095f69b68627d0efa58707d4 src/master/master.cpp be0db42da3c59761aa154439653d715556465256 src/tests/mesos.hpp 86660ac4d7476bc6cc077fc2d474c4a9ac81c031 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
On June 5, 2015, 5:42 p.m., Vinod Kone wrote: src/master/allocator/mesos/hierarchical.hpp, lines 455-456 https://reviews.apache.org/r/33159/diff/4/?file=979945#file979945line455 I don't follow. Why are these CHECKs? Is there currently code in the master that guarantees that these checks won't fail? actually nm. looks like this is being called with framework-info in the master and not with the frameworkinfo that a framework re-registers with. - Vinod --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/#review86818 --- On June 5, 2015, 1:35 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated June 5, 2015, 1:35 p.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs - include/mesos/master/allocator.hpp 8347cfaf6abce155a777484970e595e6b141bb87 src/master/allocator/mesos/allocator.hpp b57b03daf992082ec7c73199ab2574bf7993 src/master/allocator/mesos/hierarchical.hpp 44fbccaf72b65251095f69b68627d0efa58707d4 src/master/master.cpp be0db42da3c59761aa154439653d715556465256 src/tests/mesos.hpp 86660ac4d7476bc6cc077fc2d474c4a9ac81c031 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/#review86818 --- src/master/allocator/mesos/hierarchical.hpp https://reviews.apache.org/r/33159/#comment138910 I don't follow. Why are these CHECKs? Is there currently code in the master that guarantees that these checks won't fail? - Vinod Kone On June 5, 2015, 1:35 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated June 5, 2015, 1:35 p.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs - include/mesos/master/allocator.hpp 8347cfaf6abce155a777484970e595e6b141bb87 src/master/allocator/mesos/allocator.hpp b57b03daf992082ec7c73199ab2574bf7993 src/master/allocator/mesos/hierarchical.hpp 44fbccaf72b65251095f69b68627d0efa58707d4 src/master/master.cpp be0db42da3c59761aa154439653d715556465256 src/tests/mesos.hpp 86660ac4d7476bc6cc077fc2d474c4a9ac81c031 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated May 29, 2015, 10:41 p.m.) Review request for mesos and Benjamin Hindman. Changes --- Rebased. Addressed comments. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs (updated) - include/mesos/master/allocator.hpp 8347cfaf6abce155a777484970e595e6b141bb87 src/master/allocator/mesos/allocator.hpp b57b03daf992082ec7c73199ab2574bf7993 src/master/allocator/mesos/hierarchical.hpp 44fbccaf72b65251095f69b68627d0efa58707d4 src/master/master.cpp 710b8149c9d855d0f47cb2952366be10bc78c74d src/tests/mesos.hpp ac986a0687a576a0bd5693c82b3c7d543aaa956b Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/#review84882 --- Ship it! src/master/master.cpp https://reviews.apache.org/r/33159/#comment136310 I don't think this comment adds any value since the following line is pretty self explanatory (though the comment 4 lines before suffers of the same ailment) - Alexander Rojas On April 20, 2015, 8:46 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated April 20, 2015, 8:46 p.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs - src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964 src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f src/master/allocator/mesos/hierarchical.hpp 9f9a631ee52a3ab194e678f9be139e8dabc7f3db src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
Re: Review Request 33159: Pump updateFramework through Allocator from Master.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/#review84893 --- src/master/allocator/mesos/hierarchical.hpp https://reviews.apache.org/r/33159/#comment136338 According to this comment, I assume that now we do not allow frameworks to re-register with a new role or checkpoint flag. If this is the case, does it make sense to verify and therefore document these assumptions? Something like ``` CHECK(frameworks[frameworkId].role == frameworkInfo.role()) ``` Without this, the patch looks like harness code for future changes. On the same page, we do allow some of the `FrameworkInfo`'s fields to change on the re-registration. IIRC, they are all irrelevant to allocation, but do you mind throwing a comment about that? - Alexander Rukletsov On April 20, 2015, 6:46 p.m., Joris Van Remoortere wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33159/ --- (Updated April 20, 2015, 6:46 p.m.) Review request for mesos and Benjamin Hindman. Bugs: MESOS-2615 and MESOS-703 https://issues.apache.org/jira/browse/MESOS-2615 https://issues.apache.org/jira/browse/MESOS-703 Repository: mesos Description --- Follow up of 32961 where we add the 'updateFramework' function to the allocator. Diffs - src/master/allocator/allocator.hpp 91f80501aa9bc733fd53f9cb1ac0f15949a74964 src/master/allocator/mesos/allocator.hpp fb898f1175b61b442204e6e38c69ccc2838a646f src/master/allocator/mesos/hierarchical.hpp 9f9a631ee52a3ab194e678f9be139e8dabc7f3db src/master/master.cpp 44b0a0147f5354824d86332a67b30018634c9a36 src/tests/mesos.hpp 42e42ac425a448fcc5e93db1cef1112cbf5e67c4 Diff: https://reviews.apache.org/r/33159/diff/ Testing --- make check Thanks, Joris Van Remoortere
