Re: Review Request 24576: Some Master cleanups.
On Aug. 13, 2014, 2:42 a.m., Niklas Nielsen wrote: src/master/master.cpp, lines 2252-2253 https://reviews.apache.org/r/24576/diff/1/?file=658113#file658113line2252 It this comment still relevant? Good catch, I think vinod meant stack instead of heap, since they have always been on the heap. I will update it! - Ben --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/#review50396 --- On Aug. 11, 2014, 11:32 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/ --- (Updated Aug. 11, 2014, 11:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Repository: mesos-git Description --- There were a few cleanups here that I did on the way to MESOS-1620. (1) Remove the need for Offer/Task Visitor memory cleanup by using Owned. (2) Restructured the launch task code to be easier to read and understand. Diffs - src/master/master.cpp e688b41b9f2e555acd8fe0da5d3eb4e8bce32211 Diff: https://reviews.apache.org/r/24576/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 24576: Some Master cleanups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/ --- (Updated Aug. 13, 2014, 6:51 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Changes --- Rebased and updated per Vinod and Niklas' reviews. Repository: mesos-git Description --- There were a few cleanups here that I did on the way to MESOS-1620. (1) Remove the need for Offer/Task Visitor memory cleanup by using Owned. (2) Restructured the launch task code to be easier to read and understand. Diffs (updated) - src/master/master.cpp a8cf9ba07b041a770416ee70a0cff9ef51e0a844 Diff: https://reviews.apache.org/r/24576/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 24576: Some Master cleanups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/ --- (Updated Aug. 13, 2014, 6:52 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Changes --- Added missing dependency. Repository: mesos-git Description --- There were a few cleanups here that I did on the way to MESOS-1620. (1) Remove the need for Offer/Task Visitor memory cleanup by using Owned. (2) Restructured the launch task code to be easier to read and understand. Diffs - src/master/master.cpp a8cf9ba07b041a770416ee70a0cff9ef51e0a844 Diff: https://reviews.apache.org/r/24576/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 24576: Some Master cleanups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/#review50366 --- Ship it! src/master/master.cpp https://reviews.apache.org/r/24576/#comment88125 Why loop through all validators and offers when one of them returns error? Short circuting as did earlier seems fine to me, though a bit verbose. src/master/master.cpp https://reviews.apache.org/r/24576/#comment88126 CHECK_SOME? - Vinod Kone On Aug. 11, 2014, 11:32 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/ --- (Updated Aug. 11, 2014, 11:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Repository: mesos-git Description --- There were a few cleanups here that I did on the way to MESOS-1620. (1) Remove the need for Offer/Task Visitor memory cleanup by using Owned. (2) Restructured the launch task code to be easier to read and understand. Diffs - src/master/master.cpp e688b41b9f2e555acd8fe0da5d3eb4e8bce32211 Diff: https://reviews.apache.org/r/24576/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 24576: Some Master cleanups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/#review50396 --- Ship it! Modulo Vinod's comments. Looks good otherwise! src/master/master.cpp https://reviews.apache.org/r/24576/#comment88172 It this comment still relevant? - Niklas Nielsen On Aug. 11, 2014, 4:32 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/ --- (Updated Aug. 11, 2014, 4:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Repository: mesos-git Description --- There were a few cleanups here that I did on the way to MESOS-1620. (1) Remove the need for Offer/Task Visitor memory cleanup by using Owned. (2) Restructured the launch task code to be easier to read and understand. Diffs - src/master/master.cpp e688b41b9f2e555acd8fe0da5d3eb4e8bce32211 Diff: https://reviews.apache.org/r/24576/diff/ Testing --- make check Thanks, Ben Mahler
Re: Review Request 24576: Some Master cleanups.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/#review50263 --- src/master/master.cpp https://reviews.apache.org/r/24576/#comment87951 you might want to clear the list or scope it to here. otherwise the pointers remain allocated for the remainder of this method. i doubt it will cause an issue, but i prefer keeping things tightly scoped. btw - https://reviews.apache.org/r/20423 :) - Dominic Hamon On Aug. 11, 2014, 4:32 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24576/ --- (Updated Aug. 11, 2014, 4:32 p.m.) Review request for mesos, Niklas Nielsen and Vinod Kone. Repository: mesos-git Description --- There were a few cleanups here that I did on the way to MESOS-1620. (1) Remove the need for Offer/Task Visitor memory cleanup by using Owned. (2) Restructured the launch task code to be easier to read and understand. Diffs - src/master/master.cpp e688b41b9f2e555acd8fe0da5d3eb4e8bce32211 Diff: https://reviews.apache.org/r/24576/diff/ Testing --- make check Thanks, Ben Mahler
