You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2015/04/23 04:54:16 UTC
Review Request 33465: Removed 'uuid' field from UPDATE call.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33465/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Bugs: MESOS-1127
https://issues.apache.org/jira/browse/MESOS-1127
Repository: mesos
Description
-------
Now that TaskStatus has 'uuid', we don't need it in UPDATE. It will also make the ACKNOWLEDGE semantics easier.
Diffs
-----
include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
src/examples/low_level_scheduler_libprocess.cpp 63d34eefb60d13fe2b82905c1cec9b762340e997
src/examples/low_level_scheduler_pthread.cpp 6d1f938660c02db75bfcbf7c8de0d941cff1920d
src/scheduler/scheduler.cpp bd9fced0f58aa3bc0ff147dbefb77cea4734a79e
Diff: https://reviews.apache.org/r/33465/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 33465: Removed 'uuid' field from UPDATE call.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33465/
-----------------------------------------------------------
(Updated April 25, 2015, 9:32 p.m.)
Review request for mesos and Ben Mahler.
Changes
-------
benm's comments.
Bugs: MESOS-1127
https://issues.apache.org/jira/browse/MESOS-1127
Repository: mesos
Description
-------
Now that TaskStatus has 'uuid', we don't need it in UPDATE. It will also make the ACKNOWLEDGE semantics easier.
Diffs (updated)
-----
include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
src/examples/low_level_scheduler_libprocess.cpp 63d34eefb60d13fe2b82905c1cec9b762340e997
src/examples/low_level_scheduler_pthread.cpp 6d1f938660c02db75bfcbf7c8de0d941cff1920d
src/sched/sched.cpp 66fd2b3146568900356cc48e0f17c6720665ae80
src/scheduler/scheduler.cpp bd9fced0f58aa3bc0ff147dbefb77cea4734a79e
src/tests/scheduler_tests.cpp 4a89a7a88b50bb8c254f5076661ce07ac9fc7657
Diff: https://reviews.apache.org/r/33465/diff/
Testing
-------
make check
Thanks,
Vinod Kone
Re: Review Request 33465: Removed 'uuid' field from UPDATE call.
Posted by Vinod Kone <vi...@gmail.com>.
> On April 23, 2015, 7:10 p.m., Ben Mahler wrote:
> > src/scheduler/scheduler.cpp, lines 646-652
> > <https://reviews.apache.org/r/33465/diff/2/?file=940239#file940239line646>
> >
> > I realize this hack is necessary to accomplish this in 1 version, but should we have a TODO here for the master to not set the 'uuid' for non-retried updates? Similarly, if there are other cases (e.g. in the slave) where non-retried updates are generated, can we omit the 'uuid'?
> >
> > Even better, why not make that change now in 0.23.0 and leave the TODO here for 0.24.0 to rely on uuid directly?
> > That will also help us move away from the similar hack in sched.cpp `statusUpdate` (see my TODO). :)
> >
> > Also, how did you test the non-acknowledgement case? :)
updated tests.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33465/#review81372
-----------------------------------------------------------
On April 23, 2015, 5:48 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33465/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 5:48 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1127
> https://issues.apache.org/jira/browse/MESOS-1127
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Now that TaskStatus has 'uuid', we don't need it in UPDATE. It will also make the ACKNOWLEDGE semantics easier.
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
> src/examples/low_level_scheduler_libprocess.cpp 63d34eefb60d13fe2b82905c1cec9b762340e997
> src/examples/low_level_scheduler_pthread.cpp 6d1f938660c02db75bfcbf7c8de0d941cff1920d
> src/scheduler/scheduler.cpp bd9fced0f58aa3bc0ff147dbefb77cea4734a79e
>
> Diff: https://reviews.apache.org/r/33465/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 33465: Removed 'uuid' field from UPDATE call.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33465/#review81372
-----------------------------------------------------------
src/scheduler/scheduler.cpp
<https://reviews.apache.org/r/33465/#comment131717>
I realize this hack is necessary to accomplish this in 1 version, but should we have a TODO here for the master to not set the 'uuid' for non-retried updates? Similarly, if there are other cases (e.g. in the slave) where non-retried updates are generated, can we omit the 'uuid'?
Even better, why not make that change now in 0.23.0 and leave the TODO here for 0.24.0 to rely on uuid directly?
That will also help us move away from the similar hack in sched.cpp `statusUpdate` (see my TODO). :)
Also, how did you test the non-acknowledgement case? :)
- Ben Mahler
On April 23, 2015, 5:48 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33465/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 5:48 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1127
> https://issues.apache.org/jira/browse/MESOS-1127
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Now that TaskStatus has 'uuid', we don't need it in UPDATE. It will also make the ACKNOWLEDGE semantics easier.
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
> src/examples/low_level_scheduler_libprocess.cpp 63d34eefb60d13fe2b82905c1cec9b762340e997
> src/examples/low_level_scheduler_pthread.cpp 6d1f938660c02db75bfcbf7c8de0d941cff1920d
> src/scheduler/scheduler.cpp bd9fced0f58aa3bc0ff147dbefb77cea4734a79e
>
> Diff: https://reviews.apache.org/r/33465/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 33465: Removed 'uuid' field from UPDATE call.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33465/#review81544
-----------------------------------------------------------
Ship it!
Can you file the follow-up tickets for correctly setting 'uuid' and add a TODO on the existing hacks here an in sched.cpp?
Would love to see the hacks removed in a future version!
Also, looks like we need a new test for this case, yes? Mind sending a separate review for that before committing this one?
- Ben Mahler
On April 23, 2015, 5:48 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33465/
> -----------------------------------------------------------
>
> (Updated April 23, 2015, 5:48 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1127
> https://issues.apache.org/jira/browse/MESOS-1127
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Now that TaskStatus has 'uuid', we don't need it in UPDATE. It will also make the ACKNOWLEDGE semantics easier.
>
>
> Diffs
> -----
>
> include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
> src/examples/low_level_scheduler_libprocess.cpp 63d34eefb60d13fe2b82905c1cec9b762340e997
> src/examples/low_level_scheduler_pthread.cpp 6d1f938660c02db75bfcbf7c8de0d941cff1920d
> src/scheduler/scheduler.cpp bd9fced0f58aa3bc0ff147dbefb77cea4734a79e
>
> Diff: https://reviews.apache.org/r/33465/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 33465: Removed 'uuid' field from UPDATE call.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33465/
-----------------------------------------------------------
(Updated April 23, 2015, 5:48 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
minor fix. updated tag id.
Bugs: MESOS-1127
https://issues.apache.org/jira/browse/MESOS-1127
Repository: mesos
Description
-------
Now that TaskStatus has 'uuid', we don't need it in UPDATE. It will also make the ACKNOWLEDGE semantics easier.
Diffs (updated)
-----
include/mesos/scheduler/scheduler.proto 783a63ad1cc0edd86605d638046fb959cb6e97e8
src/examples/low_level_scheduler_libprocess.cpp 63d34eefb60d13fe2b82905c1cec9b762340e997
src/examples/low_level_scheduler_pthread.cpp 6d1f938660c02db75bfcbf7c8de0d941cff1920d
src/scheduler/scheduler.cpp bd9fced0f58aa3bc0ff147dbefb77cea4734a79e
Diff: https://reviews.apache.org/r/33465/diff/
Testing
-------
make check
Thanks,
Vinod Kone