> On May 7, 2015, 8:37 a.m., Adam B wrote: > > src/master/framework.cpp, lines 59-60 > > <https://reviews.apache.org/r/33490/diff/2/?file=946470#file946470line59> > > > > Given that MAX_COMPLETED_TASKS_PER_FRAMEWORK = 1000, should we worry > > about the performance hit of a linear scan through completedTasks each time > > a task completes? For very short-lived tasks, and/or frameworks that launch > > thousands of simultaneous tasks, this could be happening a lot. > > Maybe completedTasks should be a combination of a circular buffer and a > > set. > > > > Taking a step further back, we've discussed replacing this circular > > buffer with an "Archive" abstraction, that might involve persisting the > > completed tasks/frameworks to disk/network, so that we can keep a longer > > history. If/when this happens, the duplicate checking should be pushed into > > the Archive::contains() implementation, rather than doing a find_if at the > > call-site. > > I originally created this TODO because boost::circular_buffer doesn't > > have a contains().
Agreed. Not sure how to conduct a performance to test to really verify the impact of this change, though: do you have any suggestions? If performance is identified as an issue and we still think duplicating Tasks into the completedTask vector is an issue (is it?) we could add a hash of the Task to improve the lookup efficiency. - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33490/#review82809 ----------------------------------------------------------- On May 7, 2015, 8:26 a.m., Marco Massenzio wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33490/ > ----------------------------------------------------------- > > (Updated May 7, 2015, 8:26 a.m.) > > > Review request for mesos, Adam B and Joris Van Remoortere. > > > Bugs: MESOS-2633 > https://issues.apache.org/jira/browse/MESOS-2633 > > > Repository: mesos > > > Description > ------- > > In Framework::addCompletedTask(const Task& task) we did not check > for duplicated tasks, so they could be added more than once. > > A simple check has now been added (there still is the issue > of whether the `operator ==` on `Task` is too strict, so as > never to cause a match). > > > Diffs > ----- > > src/master/framework.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/33490/diff/ > > > Testing > ------- > > buildbot make check pass > http://build.mesosphere.com:5555/builders/dev_test/builds/13 > > > Thanks, > > Marco Massenzio > >
