> On Jan. 27, 2014, 8:54 p.m., Benjamin Hindman wrote: > > src/master/master.cpp, line 2657 > > <https://reviews.apache.org/r/16724/diff/3/?file=425217#file425217line2657> > > > > Let's use the same parameter name here and in the declaration please > > (same for Master::readdCompletedFramework too).
Changing to 'completedFrameworks_' since Master.completedFrameworks exists; updated the declarations as well. > On Jan. 27, 2014, 8:54 p.m., Benjamin Hindman wrote: > > src/master/master.cpp, line 2723 > > <https://reviews.apache.org/r/16724/diff/3/?file=425217#file425217line2723> > > > > We align our wrapped "lists" (parameter lists, argument lists, etc). Moot since this now fits on one line after the rename, but I understand your point. > On Jan. 27, 2014, 8:54 p.m., Benjamin Hindman wrote: > > src/master/master.cpp, line 2739 > > <https://reviews.apache.org/r/16724/diff/3/?file=425217#file425217line2739> > > > > s/knownFramework/completedFramework/ Changed 'knownFramework' to 'completedFramework'. The alternate naming came from a desire to distinguish (a) the completedFramework_ being re-added by the slave from (b) the completedFramework iterator into the master's collection of known completedFrameworks, from (c) the framework object we end up adding the completedTasks to. > On Jan. 27, 2014, 8:54 p.m., Benjamin Hindman wrote: > > src/master/master.cpp, line 2748 > > <https://reviews.apache.org/r/16724/diff/3/?file=425217#file425217line2748> > > > > Let's make this a TODO to make registration and reregistration time be > > optional in Framework. No need for a TODO. Made time an optional parameter to Master::Framework(). (Slave::Framework does not track time.) > On Jan. 27, 2014, 8:54 p.m., Benjamin Hindman wrote: > > src/messages/messages.proto, line 371 > > <https://reviews.apache.org/r/16724/diff/3/?file=425218#file425218line371> > > > > I was thinking we'd put this in the 'archive' package since we'll > > likely call a class 'Archive', but this is fine for now and we can always > > rename it later without causing upgrade/compatibility issues. Added a TODO. > On Jan. 27, 2014, 8:54 p.m., Benjamin Hindman wrote: > > src/slave/slave.cpp, line 708 > > <https://reviews.apache.org/r/16724/diff/3/?file=425219#file425219line708> > > > > Please kill all the 'msg' prefixes here, they're not necessary and not > > with convention. s/msgCompletedFramework/completedFramework_/ (since we already have a 'completedFramework' iterator into Slave.completedFrameworks) s/msgFrameworkInfo/frameworkInfo/ - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16724/#review32949 ----------------------------------------------------------- On Jan. 16, 2014, 4:24 p.m., Adam B wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16724/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2014, 4:24 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and > Vinod Kone. > > > Bugs: MESOS-767 > https://issues.apache.org/jira/browse/MESOS-767 > > > Repository: mesos-git > > > Description > ------- > > Added completed frameworks/tasks to slave re-registration. > Fixes MESOS-767. > > Additional issues discovered during investigation: > - MESOS-905: Remove Framework.id in favor of FrameworkInfo.id > - MESOS-906: Last task in Completed Framework never graduates from > terminatedTasks to completedTasks. > - Completed frameworks/executors/tasks are stored in circular buffers, > and these may overflow in different orders on different slaves. > BenH proposes an archive to replace these circular buffers. > > > Diffs > ----- > > src/master/master.hpp 95b9cec > src/master/master.cpp 38c5532 > src/messages/messages.proto 1f264d5 > src/slave/slave.cpp 396293b > > Diff: https://reviews.apache.org/r/16724/diff/ > > > Testing > ------- > > make check; manually failed-over a master, watched the slave reregister its > completed frameworks, web UI shows completed tasks and stdout/stderr. > > > Thanks, > > Adam B > >
