----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39792/#review104622 -----------------------------------------------------------
Hm.. we're still relying on the update uuid, shouldn't we be trying to move off of it onto the status uuid? src/master/master.cpp (line 4365) <https://reviews.apache.org/r/39792/#comment162847> Just a side note, it would be great to have a status update benchmark for throughput, since we're introducing an extra copy of the status update here (which might be expensive for large updates). Ideally libprocess could move construct this 'update' field (but it doesn't support this currently). src/sched/sched.cpp (line 903) <https://reviews.apache.org/r/39792/#comment162848> Mind adding a newline to separate the TODO from the rest of the comment? I find that clearer to read, especially when the TODO is at the top and it becomes ambiguous where a multi-line TODO ends and the comment starts. src/sched/sched.cpp (lines 903 - 904) <https://reviews.apache.org/r/39792/#comment162849> Hm.. why wasn't it enough that the slave was setting it? I'm guessing the concern was due to the old checkpointed updates per your change below? Seems helpful to spell out the slave side of this in the comment. src/slave/slave.cpp (lines 3025 - 3027) <https://reviews.apache.org/r/39792/#comment162850> Hm.. could you reduce jaggedness here? I like how you formatted your comment in master.cpp above, easy on my eyes. src/slave/slave.cpp (lines 3028 - 3029) <https://reviews.apache.org/r/39792/#comment162851> Hm.. not immediately obvious to me why we set the status uuid from the update uuid, should we spell that out here? - Ben Mahler On Oct. 30, 2015, 12:23 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39792/ > ----------------------------------------------------------- > > (Updated Oct. 30, 2015, 12:23 a.m.) > > > Review request for mesos, Anand Mazumdar and Ben Mahler. > > > Repository: mesos > > > Description > ------- > > This just makes sure master and slave properly set the uuid in task status to > setup the stage for deprecating the messy logic in scheduler driver in a > future release. > > > Diffs > ----- > > src/master/master.hpp b76d30197b3decda0a742e03ce01a17a64b633ac > src/master/master.cpp 9f4586e668a2141f4937497d42853fbdea7751a5 > src/sched/sched.cpp 9c5e3b8e42605f4647d897ba02ea3a17e494f355 > src/slave/slave.cpp 5f9b52b41eaab0c24965f28e192074340e00bde5 > > Diff: https://reviews.apache.org/r/39792/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
