You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Brenden Matthews <br...@diddyinc.com> on 2013/10/02 02:18:22 UTC

Review Request 14434: Added timestamps to TaskInfo.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

Review request for mesos.


Repository: mesos-git


Description
-------

Added timestamps to TaskInfo.

TaskInfo now includes a start/finish timestamp for each task.  This is
particularly for debugging framework problems.


Diffs
-----

  include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp a49b17ef43fca5b385a89731ca8776a26b61399a 
  src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Oct. 31, 2013, 7:09 p.m., Ben Mahler wrote:
> > Looking good!
> > 
> > Can you also add the status setting code to the slave?
> > 
> > With the current patch if a Master fails over the transitions are lost.
> > If the slave also stores the transitions, the master will know the transitions from the Task sent by the slave during re-registration.
> > 
> > May be good to add Ross to this review for the webui related bits.

One issue is that the column sorting broke with this patch, and my javascript skills are not quite epic enough to figure it out.


> On Oct. 31, 2013, 7:09 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1427-1435
> > <https://reviews.apache.org/r/14434/diff/5/?file=371838#file371838line1427>
> >
> >     We should wipe the 'data' and 'message' as they may be arbitrarily large.
> >     
> >     s/TaskStatus */TaskStatus* /

Ben Hindman suggested we keep this.  I'll add him on the review.


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review27939
-----------------------------------------------------------


On Oct. 26, 2013, 1:48 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2013, 1:48 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Oct. 31, 2013, 7:09 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 1427-1435
> > <https://reviews.apache.org/r/14434/diff/5/?file=371838#file371838line1427>
> >
> >     We should wipe the 'data' and 'message' as they may be arbitrarily large.
> >     
> >     s/TaskStatus */TaskStatus* /
> 
> Brenden Matthews wrote:
>     Ben Hindman suggested we keep this.  I'll add him on the review.

Yeah, I liked the idea of possibly being able to expose this information for debugging! IMHO, if these are super large structures than they are likely impacting the performance of other components first and foremost (like the master). How about adding a TODO for now?


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review27939
-----------------------------------------------------------


On Oct. 31, 2013, 8 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2013, 8 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review27939
-----------------------------------------------------------


Looking good!

Can you also add the status setting code to the slave?

With the current patch if a Master fails over the transitions are lost.
If the slave also stores the transitions, the master will know the transitions from the Task sent by the slave during re-registration.

May be good to add Ross to this review for the webui related bits.


src/master/master.cpp
<https://reviews.apache.org/r/14434/#comment54376>

    We should wipe the 'data' and 'message' as they may be arbitrarily large.
    
    s/TaskStatus */TaskStatus* /


- Ben Mahler


On Oct. 26, 2013, 1:48 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 26, 2013, 1:48 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review27997
-----------------------------------------------------------

Ship it!


This LGTM barring the tooltip comments from Ross!


src/master/master.cpp
<https://reviews.apache.org/r/14434/#comment54456>

    How about:
    
    task->add_statuses()->CopyFrom(status);



src/master/master.cpp
<https://reviews.apache.org/r/14434/#comment54457>

    Likewise, what about:
    
    task->mutable_statuses(task->statuses_size())->CopyFrom(status);
    
    Or perhaps we're even more explicit that we're removing the last one? What about: 
    
    task->mutable_statuses()->RemoveLast();
    task->add_statuses()->CopyFrom(status);


- Benjamin Hindman


On Oct. 31, 2013, 8 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2013, 8 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28850
-----------------------------------------------------------


I'll hold off on merging this until I hear from some of the others.

- Brenden Matthews


On Nov. 13, 2013, 10:17 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 10:17 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Nov. 14, 2013, 7:06 p.m., Vinod Kone wrote:
> > src/exec/exec.cpp, line 498
> > <https://reviews.apache.org/r/14434/diff/6/?file=383873#file383873line498>
> >
> >     May be only set this if it's not already set by the executor? Silently overwriting is a bit concerning.
> 
> Brenden Matthews wrote:
>     I discussed this with Ben H at the hackathon, and we decided this was the best way to do it.
> 
> Vinod Kone wrote:
>     What was the reasoning?

I think the rationale was that both timestamps should be the same, and be set from the same place.


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28884
-----------------------------------------------------------


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Nov. 14, 2013, 7:06 p.m., Vinod Kone wrote:
> > src/slave/slave.hpp, lines 369-370
> > <https://reviews.apache.org/r/14434/diff/6/?file=383877#file383877line369>
> >
> >     Can we get rid of 'taskId' param since we can get that from 'status' ?

I was thinking the same thing, but I wasn't sure if it was that way for a reason.


> On Nov. 14, 2013, 7:06 p.m., Vinod Kone wrote:
> > src/exec/exec.cpp, line 498
> > <https://reviews.apache.org/r/14434/diff/6/?file=383873#file383873line498>
> >
> >     May be only set this if it's not already set by the executor? Silently overwriting is a bit concerning.

I discussed this with Ben H at the hackathon, and we decided this was the best way to do it.


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28884
-----------------------------------------------------------


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Vinod Kone <vi...@gmail.com>.

> On Nov. 14, 2013, 7:06 p.m., Vinod Kone wrote:
> > src/exec/exec.cpp, line 498
> > <https://reviews.apache.org/r/14434/diff/6/?file=383873#file383873line498>
> >
> >     May be only set this if it's not already set by the executor? Silently overwriting is a bit concerning.
> 
> Brenden Matthews wrote:
>     I discussed this with Ben H at the hackathon, and we decided this was the best way to do it.

What was the reasoning?


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28884
-----------------------------------------------------------


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28884
-----------------------------------------------------------

Ship it!



src/exec/exec.cpp
<https://reviews.apache.org/r/14434/#comment55949>

    May be only set this if it's not already set by the executor? Silently overwriting is a bit concerning.



src/master/master.cpp
<https://reviews.apache.org/r/14434/#comment55950>

    s/)/):/



src/slave/slave.hpp
<https://reviews.apache.org/r/14434/#comment55951>

    Can we get rid of 'taskId' param since we can get that from 'status' ?


- Vinod Kone


On Nov. 13, 2013, 10:17 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 10:17 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Ben Mahler <be...@gmail.com>.

> On Dec. 5, 2013, 10:32 p.m., Ben Mahler wrote:
> > When we create Tasks (e.g. in Master::launchTask) we should add a corresponding status for the Task, can you take that up in a subsequent patch?
> > 
> > There are a few spots in the master/slave where a task is created with no corresponding status. Would potentially be easiest if we delegated task creation to a protobuf_utils:: function that always adds a status.

We've got https://issues.apache.org/jira/browse/MESOS-869 targeted for 0.17.0 if you'd like to take it :)


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review29842
-----------------------------------------------------------


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review29842
-----------------------------------------------------------


When we create Tasks (e.g. in Master::launchTask) we should add a corresponding status for the Task, can you take that up in a subsequent patch?

There are a few spots in the master/slave where a task is created with no corresponding status. Would potentially be easiest if we delegated task creation to a protobuf_utils:: function that always adds a status.

- Ben Mahler


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Nov. 14, 2013, 7:19 p.m., Ross Allen wrote:
> > src/webui/master/static/js/controllers.js, line 224
> > <https://reviews.apache.org/r/14434/diff/7/?file=384705#file384705line224>
> >
> >     All other timestamps in JS are unmodified, stored as seconds, and then the '* 1000' is done in the view. Let's stick to that here too.
> >     
> >     The '* 1000' in the view is gnarly, but we can move that into the 'isoDate' filter afterward.
> 
> Brenden Matthews wrote:
>     Oh no, just a few moments too late :)
>     
>     I'll open another review for this.

https://reviews.apache.org/r/15539/


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28886
-----------------------------------------------------------


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Nov. 14, 2013, 7:19 p.m., Ross Allen wrote:
> > src/webui/master/static/js/controllers.js, line 224
> > <https://reviews.apache.org/r/14434/diff/7/?file=384705#file384705line224>
> >
> >     All other timestamps in JS are unmodified, stored as seconds, and then the '* 1000' is done in the view. Let's stick to that here too.
> >     
> >     The '* 1000' in the view is gnarly, but we can move that into the 'isoDate' filter afterward.

Oh no, just a few moments too late :)

I'll open another review for this.


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28886
-----------------------------------------------------------


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Ross Allen <ro...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28886
-----------------------------------------------------------



src/webui/master/static/js/controllers.js
<https://reviews.apache.org/r/14434/#comment55954>

    All other timestamps in JS are unmodified, stored as seconds, and then the '* 1000' is done in the view. Let's stick to that here too.
    
    The '* 1000' in the view is gnarly, but we can move that into the 'isoDate' filter afterward.


- Ross Allen


On Nov. 14, 2013, 7:16 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2013, 7:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

(Updated Nov. 14, 2013, 7:16 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Updated as per Vinod's suggestions.


Repository: mesos-git


Description
-------

Added list of TaskStatuses to Task message.

Task now includes repeated task status with timestamps.  This permits
caltulating the start/finish time for tasks (among other things).

To support this, a timestamp was added to the TaskStatus message which
is also passed to frameworks when there are state transitions.

Review: https://reviews.apache.org/r/14434


Diffs (updated)
-----

  include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
  src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
  src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
  src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
  src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
  src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
  src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
  src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review28844
-----------------------------------------------------------

Ship it!


Ship It!

- Niklas Nielsen


On Nov. 13, 2013, 10:17 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2013, 10:17 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
>   src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
>   src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

(Updated Nov. 13, 2013, 10:17 p.m.)


Review request for mesos and Benjamin Hindman.


Changes
-------

Updated as per Ben M's, Ross A's, and Ben H's comments.


Repository: mesos-git


Description
-------

Added list of TaskStatuses to Task message.

Task now includes repeated task status with timestamps.  This permits
caltulating the start/finish time for tasks (among other things).

To support this, a timestamp was added to the TaskStatus message which
is also passed to frameworks when there are state transitions.

Review: https://reviews.apache.org/r/14434


Diffs (updated)
-----

  include/mesos/mesos.proto ad4ba5bb618d4c9e107e288566852f54da6ceea5 
  src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
  src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp 8e14a070e87ebe579b54d05fb1e8b286edb5e459 
  src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
  src/slave/slave.hpp 68526f3fb01be6dc19d7e3c8e4ba8220b2c56ccb 
  src/slave/slave.cpp 13cb41828298a725ee94a900a63dd8626f315e7e 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
  src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
  src/webui/master/static/js/controllers.js 8f65679ff4f82f570cf7d632e45ca7dbcfab3376 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Ross Allen <ro...@mesosphere.io>.

> On Oct. 31, 2013, 8:06 p.m., Ross Allen wrote:
> > src/webui/master/static/framework.html, line 98
> > <https://reviews.apache.org/r/14434/diff/5/?file=371840#file371840line98>
> >
> >     Knowing that statuses[0] is the start time and statuses[statuses.length - 1] should stay either in Mesos or in the controller in the UI. Can this logic move to one of those places, and then this could become task.start_time?
> 
> Benjamin Hindman wrote:
>     I'd prefer not to put this in the master since all the information you need is already in the JSON. How about we push this off for now and think about some abstractions in the controllers that could make this cleaner?

Sounds good to me.


- Ross


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review27947
-----------------------------------------------------------


On Oct. 31, 2013, 8 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2013, 8 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Benjamin Hindman <be...@berkeley.edu>.

> On Oct. 31, 2013, 8:06 p.m., Ross Allen wrote:
> > src/webui/master/static/framework.html, line 98
> > <https://reviews.apache.org/r/14434/diff/5/?file=371840#file371840line98>
> >
> >     Knowing that statuses[0] is the start time and statuses[statuses.length - 1] should stay either in Mesos or in the controller in the UI. Can this logic move to one of those places, and then this could become task.start_time?

I'd prefer not to put this in the master since all the information you need is already in the JSON. How about we push this off for now and think about some abstractions in the controllers that could make this cleaner?


> On Oct. 31, 2013, 8:06 p.m., Ross Allen wrote:
> > src/webui/master/static/framework.html, line 103
> > <https://reviews.apache.org/r/14434/diff/5/?file=371840#file371840line103>
> >
> >     "tooltip" is the text shown on hover and should be human-readable. Can this be "Copy start time" without the underscore?

+1 to killing the underscore here and below.


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review27947
-----------------------------------------------------------


On Oct. 31, 2013, 8 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2013, 8 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Ross Allen <ro...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review27947
-----------------------------------------------------------



src/webui/master/static/framework.html
<https://reviews.apache.org/r/14434/#comment54398>

    Knowing that statuses[0] is the start time and statuses[statuses.length - 1] should stay either in Mesos or in the controller in the UI. Can this logic move to one of those places, and then this could become task.start_time?



src/webui/master/static/framework.html
<https://reviews.apache.org/r/14434/#comment54395>

    "tooltip" is the text shown on hover and should be human-readable. Can this be "Copy start time" without the underscore?



src/webui/master/static/framework.html
<https://reviews.apache.org/r/14434/#comment54397>

    "Copy start time" here too.


- Ross Allen


On Oct. 31, 2013, 8 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2013, 8 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added list of TaskStatuses to Task message.
> 
> Task now includes repeated task status with timestamps.  This permits
> caltulating the start/finish time for tasks (among other things).
> 
> To support this, a timestamp was added to the TaskStatus message which
> is also passed to frameworks when there are state transitions.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
>   src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
>   src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
>   src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

(Updated Oct. 31, 2013, 8 p.m.)


Review request for mesos and Benjamin Hindman.


Repository: mesos-git


Description
-------

Added list of TaskStatuses to Task message.

Task now includes repeated task status with timestamps.  This permits
caltulating the start/finish time for tasks (among other things).

To support this, a timestamp was added to the TaskStatus message which
is also passed to frameworks when there are state transitions.

Review: https://reviews.apache.org/r/14434


Diffs
-----

  include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
  src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
  src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
  src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
  src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added list of TaskStatuses to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

(Updated Oct. 26, 2013, 1:48 a.m.)


Review request for mesos.


Changes
-------

Updated as per Ben Hindman's suggestions.


Summary (updated)
-----------------

Added list of TaskStatuses to Task message.


Repository: mesos-git


Description (updated)
-------

Added list of TaskStatuses to Task message.

Task now includes repeated task status with timestamps.  This permits
caltulating the start/finish time for tasks (among other things).

To support this, a timestamp was added to the TaskStatus message which
is also passed to frameworks when there are state transitions.

Review: https://reviews.apache.org/r/14434


Diffs (updated)
-----

  include/mesos/mesos.proto d45ee5ea1287f925332968b7247b21f72ba38b13 
  src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
  src/exec/exec.cpp 4a598f52bd9f97eeb14d9700e05281ccc1650c14 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp 1147cc6ebd9a2d18c3b58fa103e6348f9d623438 
  src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
  src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added StateTransition to Task message.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

(Updated Oct. 14, 2013, 9:40 p.m.)


Review request for mesos.


Changes
-------

Updated as per Ben M's & Ben H's comments.


Summary (updated)
-----------------

Added StateTransition to Task message.


Repository: mesos-git


Description (updated)
-------

Added StateTransition to Task message.

Task now includes repeated state transitions with timestamps.  This
allows the master to perform some basic task accounting, like displaying
the start/finish time for tasks.

To support this, a timestamp was added to the TaskStatus message which
is also passed to frameworks when there are state transitions.

Review: https://reviews.apache.org/r/14434


Diffs (updated)
-----

  include/mesos/mesos.proto fe1d82bf3580201f2aff2ca775f9d77aef45736e 
  src/common/protobuf_utils.hpp 19a49ab1cb212b454394b78ffedeb9a7eb6b8964 
  src/exec/exec.cpp 7ca21fa205ee8cb851dc8d826d75d8d438c36860 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp 1bf5d47b2894a896ef7ee0cec05170dff86b62e8 
  src/messages/messages.proto a5dded2c0f3f88a5f7d789ccde6e59f68880374b 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
  src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added timestamps to TaskInfo.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

(Updated Oct. 4, 2013, 7:35 p.m.)


Review request for mesos.


Changes
-------

The last patch wasn't quite right.  This one seems to work well, however.


Repository: mesos-git


Description
-------

Added timestamps to TaskInfo.

TaskInfo now includes a start/finish timestamp for each task.  This is
particularly for debugging framework problems.

Review: https://reviews.apache.org/r/14434


Diffs (updated)
-----

  include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp ce8365f082a5f96ef64e33e526cb5047dff52127 
  src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
  src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added timestamps to TaskInfo.

Posted by Brenden Matthews <br...@diddyinc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/
-----------------------------------------------------------

(Updated Oct. 2, 2013, 9:17 p.m.)


Review request for mesos.


Changes
-------

Updated to reflect comments from Ben Hindman & Ben Mahler.


Repository: mesos-git


Description (updated)
-------

Added timestamps to TaskInfo.

TaskInfo now includes a start/finish timestamp for each task.  This is
particularly for debugging framework problems.

Review: https://reviews.apache.org/r/14434


Diffs (updated)
-----

  include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
  src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
  src/master/master.cpp ce8365f082a5f96ef64e33e526cb5047dff52127 
  src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f 
  src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
  src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 

Diff: https://reviews.apache.org/r/14434/diff/


Testing
-------

`make check`, tested in staging, and manual web UI testing.


Thanks,

Brenden Matthews


Re: Review Request 14434: Added timestamps to TaskInfo.

Posted by Benjamin Hindman <be...@eecs.berkeley.edu>.
How about adding an optional timestamp to TaskStatus?


On Wed, Oct 2, 2013 at 10:11 AM, Brenden Matthews <br...@diddyinc.com>wrote:

>
>
> > On Oct. 2, 2013, 5:54 a.m., Ben Mahler wrote:
> > > include/mesos/mesos.proto, lines 372-373
> > > <
> https://reviews.apache.org/r/14434/diff/1/?file=360207#file360207line372>
> > >
> > >     I believe adding mutable fields to TaskInfo may be problematic as
> in general the various components assume that _Info protobufs are immutable.
> > >
> > >     A while back I had filed MESOS-296 in the same spirit: what if we
> added timestamps to status updates? Status updates would still remain
> immutable in the system. However, it's likely a bit trickier to show them
> in the webui since the master does not expose status updates. One approach
> is to have the master / slave track the {start,finish}_times in memory in
> their respective Task structs. Thoughts?
> > >
> > >     Even better would be if the webui could how a history of state
> transitions (rather than status updates given how status updates can be
> used as a messaging mechanism).
>
> I see, that was indeed a while back.
>
> If we add the same values to the in-memory structs, how do we report the
> data back to the web UI?  Currently the code in src/master/http.cpp just
> pulls the values from the `Task` message.  Is it safe to treat `Task` as
> mutable?
>
>
> - Brenden
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/#review26598
> -----------------------------------------------------------
>
>
> On Oct. 2, 2013, 12:18 a.m., Brenden Matthews wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/14434/
> > -----------------------------------------------------------
> >
> > (Updated Oct. 2, 2013, 12:18 a.m.)
> >
> >
> > Review request for mesos.
> >
> >
> > Repository: mesos-git
> >
> >
> > Description
> > -------
> >
> > Added timestamps to TaskInfo.
> >
> > TaskInfo now includes a start/finish timestamp for each task.  This is
> > particularly for debugging framework problems.
> >
> >
> > Diffs
> > -----
> >
> >   include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616
> >   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327
> >   src/master/master.cpp a49b17ef43fca5b385a89731ca8776a26b61399a
> >   src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f
> >   src/webui/master/static/framework.html
> 6e5cd9f9e48597c7894d6381377c8a291014e8f3
> >
> > Diff: https://reviews.apache.org/r/14434/diff/
> >
> >
> > Testing
> > -------
> >
> > `make check`, tested in staging, and manual web UI testing.
> >
> >
> > Thanks,
> >
> > Brenden Matthews
> >
> >
>
>

Re: Review Request 14434: Added timestamps to TaskInfo.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Oct. 2, 2013, 5:54 a.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 372-373
> > <https://reviews.apache.org/r/14434/diff/1/?file=360207#file360207line372>
> >
> >     I believe adding mutable fields to TaskInfo may be problematic as in general the various components assume that _Info protobufs are immutable.
> >     
> >     A while back I had filed MESOS-296 in the same spirit: what if we added timestamps to status updates? Status updates would still remain immutable in the system. However, it's likely a bit trickier to show them in the webui since the master does not expose status updates. One approach is to have the master / slave track the {start,finish}_times in memory in their respective Task structs. Thoughts?
> >     
> >     Even better would be if the webui could how a history of state transitions (rather than status updates given how status updates can be used as a messaging mechanism).

I see, that was indeed a while back.

If we add the same values to the in-memory structs, how do we report the data back to the web UI?  Currently the code in src/master/http.cpp just pulls the values from the `Task` message.  Is it safe to treat `Task` as mutable?


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review26598
-----------------------------------------------------------


On Oct. 2, 2013, 12:18 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:18 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added timestamps to TaskInfo.
> 
> TaskInfo now includes a start/finish timestamp for each task.  This is
> particularly for debugging framework problems.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp a49b17ef43fca5b385a89731ca8776a26b61399a 
>   src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added timestamps to TaskInfo.

Posted by Ben Mahler <be...@gmail.com>.

> On Oct. 2, 2013, 5:54 a.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 372-373
> > <https://reviews.apache.org/r/14434/diff/1/?file=360207#file360207line372>
> >
> >     I believe adding mutable fields to TaskInfo may be problematic as in general the various components assume that _Info protobufs are immutable.
> >     
> >     A while back I had filed MESOS-296 in the same spirit: what if we added timestamps to status updates? Status updates would still remain immutable in the system. However, it's likely a bit trickier to show them in the webui since the master does not expose status updates. One approach is to have the master / slave track the {start,finish}_times in memory in their respective Task structs. Thoughts?
> >     
> >     Even better would be if the webui could how a history of state transitions (rather than status updates given how status updates can be used as a messaging mechanism).
> 
> Brenden Matthews wrote:
>     I see, that was indeed a while back.
>     
>     If we add the same values to the in-memory structs, how do we report the data back to the web UI?  Currently the code in src/master/http.cpp just pulls the values from the `Task` message.  Is it safe to treat `Task` as mutable?

+1 to benh's comment on the email thread, I think adding timestamp to TaskStatus would be a good first step here.


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review26598
-----------------------------------------------------------


On Oct. 2, 2013, 9:17 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 9:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added timestamps to TaskInfo.
> 
> TaskInfo now includes a start/finish timestamp for each task.  This is
> particularly for debugging framework problems.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp ce8365f082a5f96ef64e33e526cb5047dff52127 
>   src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added timestamps to TaskInfo.

Posted by Brenden Matthews <br...@diddyinc.com>.

> On Oct. 2, 2013, 5:54 a.m., Ben Mahler wrote:
> > include/mesos/mesos.proto, lines 372-373
> > <https://reviews.apache.org/r/14434/diff/1/?file=360207#file360207line372>
> >
> >     I believe adding mutable fields to TaskInfo may be problematic as in general the various components assume that _Info protobufs are immutable.
> >     
> >     A while back I had filed MESOS-296 in the same spirit: what if we added timestamps to status updates? Status updates would still remain immutable in the system. However, it's likely a bit trickier to show them in the webui since the master does not expose status updates. One approach is to have the master / slave track the {start,finish}_times in memory in their respective Task structs. Thoughts?
> >     
> >     Even better would be if the webui could how a history of state transitions (rather than status updates given how status updates can be used as a messaging mechanism).
> 
> Brenden Matthews wrote:
>     I see, that was indeed a while back.
>     
>     If we add the same values to the in-memory structs, how do we report the data back to the web UI?  Currently the code in src/master/http.cpp just pulls the values from the `Task` message.  Is it safe to treat `Task` as mutable?
> 
> Ben Mahler wrote:
>     +1 to benh's comment on the email thread, I think adding timestamp to TaskStatus would be a good first step here.

After looking at the code, it seems unnecessary to add a timestamp to TaskStatus, since the StatusUpdate message already includes a timestamp (which I've used in my latest revision).


- Brenden


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review26598
-----------------------------------------------------------


On Oct. 2, 2013, 9:17 p.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 9:17 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added timestamps to TaskInfo.
> 
> TaskInfo now includes a start/finish timestamp for each task.  This is
> particularly for debugging framework problems.
> 
> Review: https://reviews.apache.org/r/14434
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp ce8365f082a5f96ef64e33e526cb5047dff52127 
>   src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
>   src/webui/master/static/frameworks.html 7c243d47d80e7c74fcac938d012b91b67b995490 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>


Re: Review Request 14434: Added timestamps to TaskInfo.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14434/#review26598
-----------------------------------------------------------



include/mesos/mesos.proto
<https://reviews.apache.org/r/14434/#comment51842>

    I believe adding mutable fields to TaskInfo may be problematic as in general the various components assume that _Info protobufs are immutable.
    
    A while back I had filed MESOS-296 in the same spirit: what if we added timestamps to status updates? Status updates would still remain immutable in the system. However, it's likely a bit trickier to show them in the webui since the master does not expose status updates. One approach is to have the master / slave track the {start,finish}_times in memory in their respective Task structs. Thoughts?
    
    Even better would be if the webui could how a history of state transitions (rather than status updates given how status updates can be used as a messaging mechanism).


- Ben Mahler


On Oct. 2, 2013, 12:18 a.m., Brenden Matthews wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14434/
> -----------------------------------------------------------
> 
> (Updated Oct. 2, 2013, 12:18 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added timestamps to TaskInfo.
> 
> TaskInfo now includes a start/finish timestamp for each task.  This is
> particularly for debugging framework problems.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
>   src/master/http.cpp f2a535a002f07784db724f6fdedfb26b271d6327 
>   src/master/master.cpp a49b17ef43fca5b385a89731ca8776a26b61399a 
>   src/messages/messages.proto c599eb2f1105baf5253ab8c982f48f30e798b94f 
>   src/webui/master/static/framework.html 6e5cd9f9e48597c7894d6381377c8a291014e8f3 
> 
> Diff: https://reviews.apache.org/r/14434/diff/
> 
> 
> Testing
> -------
> 
> `make check`, tested in staging, and manual web UI testing.
> 
> 
> Thanks,
> 
> Brenden Matthews
> 
>