[
https://issues.apache.org/jira/browse/MESOS-8?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150127#comment-13150127
]
[email protected] commented on MESOS-8:
---------------------------------------------------
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/http.cpp, line 80
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56809#file56809line80>
bq. >
bq. > I know this isn't your stuff, but let's change this to
registered_time (update everywhere else as appropriate) and let's also add
reregistered_time (which will only be the last reregister, but that is okay).
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/http.cpp, line 81
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56809#file56809line81>
bq. >
bq. > Now let's change this to unregistered_time. And as long as we are
here, let's go ahead and add whether or not this framework is active too
(framework.active).
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/http.cpp, line 97
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56809#file56809line97>
bq. >
bq. > Kill tab.
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/http.cpp, line 100
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56809#file56809line100>
bq. >
bq. > Let's use snake case here ( s/completedTasks/completed_tasks) just
to be consistent with the rest of the JSON we are sending. If there is some
reason to be using camel case instead (JSON standards, best practices, etc) let
us know but then let's change it all!
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/http.cpp, line 259
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56809#file56809line259>
bq. >
bq. > Kill tab.
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/http.cpp, line 262
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56809#file56809line262>
bq. >
bq. > s/completedFrameworks/completed_frameworks
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.hpp, line 218
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56810#file56810line218>
bq. >
bq. > Since this is effectively a constant for now, lets stick it in
master/constants.hpp as MAX_COMPLETED_FRAMEWORKS. You can add a TODO there to
make it configurable later.
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.hpp, line 402
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56810#file56810line402>
bq. >
bq. > Space between 'if' and '('.
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/webui/master/index.tpl, line 101
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56813#file56813line101>
bq. >
bq. > Why was this necessary?
I think this was an artifact left over from the fact that I started working on
an old version. Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/webui/master/index.tpl, line 240
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56813#file56813line240>
bq. >
bq. > Again, what's going on here? If I look above at model(const Offer&
offer) I don't see any code that is adding a 'slaves' field.
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/webui/master/index.tpl, lines 281-282
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56813#file56813line281>
bq. >
bq. > Probably don't need that right?
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.hpp, line 478
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56810#file56810line478>
bq. >
bq. > s/completedTime/unregisteredTime
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.hpp, line 483
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56810#file56810line483>
bq. >
bq. > Let's stick this in master/constants.hpp too.
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.cpp, line 1765
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56811#file56811line1765>
bq. >
bq. > What happens if we ever set MAX_COMPLETED_TASKS to be 0? Will the
task get deleted after calling removeTask suck that we'll segfault when we try
and use the task? Try and think about the ownership here. It was more explicit
with the delete below, but now the ownership is getting transferred. Maybe it
would be easier to store a copy of the completed Task?
Andy and I agreed that the best solution to this (and to the memory leak
problems) was to make a copy of the frameworks and tasks before storing them,
and store the new objects themselves rather than pointers.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.cpp, line 1578
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56811#file56811line1578>
bq. >
bq. > Update this comment since you are no longer deallocating the memory.
We're making a copy and deleting it now, so this isn't applicable.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.cpp, line 1575
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56811#file56811line1575>
bq. >
bq. > Memory leak.
Done.
bq. On 2011-11-14 20:19:11, Benjamin Hindman wrote:
bq. > src/master/master.hpp, line 403
bq. > <https://reviews.apache.org/r/2763/diff/1/?file=56810#file56810line403>
bq. >
bq. > Probably a memory leak here ... this is where the Task object is
supposed to get deallocated right?
Done.
- Thomas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2763/#review3214
-----------------------------------------------------------
On 2011-11-14 23:03:35, Thomas Marshall wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2763/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-14 23:03:35)
bq.
bq.
bq. Review request for mesos and Andy Konwinski.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. Frameworks are no longer deleted when they are done executing; they are
saved in a list in the master, which is then displayed on the webui. Tasks are
similarly stored in their framework.
bq.
bq.
bq. This addresses bug MESOS-8.
bq. https://issues.apache.org/jira/browse/MESOS-8
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. src/master/http.cpp 47caf48
bq. src/master/master.hpp fdacf36
bq. src/master/master.cpp b6cfde7
bq. src/webui/master/framework.tpl 0f41349
bq. src/webui/master/index.tpl 16e3446
bq.
bq. Diff: https://reviews.apache.org/r/2763/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Thomas
bq.
bq.
> Maintain a history of executed frameworks/tasks and show it on the web UI
> -------------------------------------------------------------------------
>
> Key: MESOS-8
> URL: https://issues.apache.org/jira/browse/MESOS-8
> Project: Mesos
> Issue Type: Improvement
> Reporter: Matei Zaharia
> Assignee: Thomas Marshall
>
> This is kind of a big task but ultimately important for cluster management.
> See https://github.com/mesos/mesos/issues#issue/39 for previous discussion on
> GitHub.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
