You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2014/10/14 20:07:59 UTC
Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/
-----------------------------------------------------------
Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
Bugs: MESOS-1799 and MESOS-1817
https://issues.apache.org/jira/browse/MESOS-1799
https://issues.apache.org/jira/browse/MESOS-1817
Repository: mesos-git
Description
-------
Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
Diffs
-----
src/master/master.cpp cb46cec0674b3aa031450c5b4f48f4f8bb92767d
src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
Diff: https://reviews.apache.org/r/26702/diff/
Testing
-------
make check
Ran new test 1000 times.
Thanks,
Vinod Kone
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review56559
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26697, 26698, 26699, 26700, 26701, 26702]
All tests passed.
- Mesos ReviewBot
On Oct. 14, 2014, 6:07 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2014, 6:07 p.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp cb46cec0674b3aa031450c5b4f48f4f8bb92767d
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review56849
-----------------------------------------------------------
Bad patch!
Reviews applied: [26697, 26698, 26699, 26700, 26701]
Failed command: git apply --index 26701.patch
Error:
error: patch failed: src/master/master.cpp:4475
error: src/master/master.cpp: patch does not apply
- Mesos ReviewBot
On Oct. 15, 2014, 9:32 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2014, 9:32 p.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp 1b1ce0de3065ced45890777d42c69ef1091f5699
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review57495
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26846, 26957, 26699, 26700, 26701, 26702]
All tests passed.
- Mesos ReviewBot
On Oct. 20, 2014, 11:58 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 11:58 p.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp 0a5c9a374062a241c90ea238725fbb8dd2408ef4
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review57711
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26846, 26957, 26699, 26700, 26701, 26702]
All tests passed.
- Mesos ReviewBot
On Oct. 21, 2014, 10:26 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 10:26 p.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp be910d9f02405e25e5ad6ae3bf13d770a0c2b417
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 10:26 p.m.)
Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
Changes
-------
rebased. NNFR.
Bugs: MESOS-1799 and MESOS-1817
https://issues.apache.org/jira/browse/MESOS-1799
https://issues.apache.org/jira/browse/MESOS-1817
Repository: mesos-git
Description
-------
Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
Diffs (updated)
-----
src/master/master.cpp be910d9f02405e25e5ad6ae3bf13d770a0c2b417
src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
Diff: https://reviews.apache.org/r/26702/diff/
Testing
-------
make check
Ran new test 1000 times.
Thanks,
Vinod Kone
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/
-----------------------------------------------------------
(Updated Oct. 20, 2014, 11:58 p.m.)
Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
Changes
-------
addressed comments.
Bugs: MESOS-1799 and MESOS-1817
https://issues.apache.org/jira/browse/MESOS-1799
https://issues.apache.org/jira/browse/MESOS-1817
Repository: mesos-git
Description
-------
Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
Diffs (updated)
-----
src/master/master.cpp 0a5c9a374062a241c90ea238725fbb8dd2408ef4
src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
Diff: https://reviews.apache.org/r/26702/diff/
Testing
-------
make check
Ran new test 1000 times.
Thanks,
Vinod Kone
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 19, 2014, 8:51 p.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 3916-3918
> > <https://reviews.apache.org/r/26702/diff/3/?file=723867#file723867line3916>
> >
> > Hm.. since we don't look at this state on the slave side, maybe a comment? For now there is no difference.
done.
> On Oct. 19, 2014, 8:51 p.m., Ben Mahler wrote:
> > src/tests/reconciliation_tests.cpp, lines 748-751
> > <https://reviews.apache.org/r/26702/diff/3/?file=723868#file723868line748>
> >
> > Could you restructure this really long sentence?
done.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review57295
-----------------------------------------------------------
On Oct. 17, 2014, 12:31 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2014, 12:31 a.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp 0a5c9a374062a241c90ea238725fbb8dd2408ef4
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review57295
-----------------------------------------------------------
Ship it!
src/master/master.cpp
<https://reviews.apache.org/r/26702/#comment97911>
Hm.. since we don't look at this state on the slave side, maybe a comment? For now there is no difference.
src/tests/reconciliation_tests.cpp
<https://reviews.apache.org/r/26702/#comment97912>
Could you restructure this really long sentence?
- Ben Mahler
On Oct. 17, 2014, 12:31 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2014, 12:31 a.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp 0a5c9a374062a241c90ea238725fbb8dd2408ef4
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review57093
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26846, 26699, 26700, 26701, 26702]
All tests passed.
- Mesos ReviewBot
On Oct. 17, 2014, 12:31 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2014, 12:31 a.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp 0a5c9a374062a241c90ea238725fbb8dd2408ef4
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/
-----------------------------------------------------------
(Updated Oct. 17, 2014, 12:31 a.m.)
Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
Changes
-------
Rebased.
Bugs: MESOS-1799 and MESOS-1817
https://issues.apache.org/jira/browse/MESOS-1799
https://issues.apache.org/jira/browse/MESOS-1817
Repository: mesos-git
Description
-------
Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
Diffs (updated)
-----
src/master/master.cpp 0a5c9a374062a241c90ea238725fbb8dd2408ef4
src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
Diff: https://reviews.apache.org/r/26702/diff/
Testing
-------
make check
Ran new test 1000 times.
Thanks,
Vinod Kone
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/
-----------------------------------------------------------
(Updated Oct. 15, 2014, 9:32 p.m.)
Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
Changes
-------
Addressed comments.
Bugs: MESOS-1799 and MESOS-1817
https://issues.apache.org/jira/browse/MESOS-1799
https://issues.apache.org/jira/browse/MESOS-1817
Repository: mesos-git
Description
-------
Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
Diffs (updated)
-----
src/master/master.cpp 1b1ce0de3065ced45890777d42c69ef1091f5699
src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
Diff: https://reviews.apache.org/r/26702/diff/
Testing
-------
make check
Ran new test 1000 times.
Thanks,
Vinod Kone
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 15, 2014, 7:41 a.m., Adam B wrote:
> > src/master/master.cpp, lines 3920-3927
> > <https://reviews.apache.org/r/26702/diff/1/?file=720980#file720980line3920>
> >
> > Should reconciliation on slave reregistration return to the slave each task's latest state, or its latest unacknowledged state? Would it ever matter?
> > Looking at Slave::reregistered(), it seems it doesn't even look at the state. It just checks to see if the slave knows that taskID and sends back TASK_LOST if the task is unknown.
For consistency, reconciliation requests should always include the latest unacknowledged state, because reconciliation is always based on unacknowledged state not latest state (to avoid out of order updates).
Here, technically it doesn't matter because slave ignores it. But that needs non-local reasoning to understand. I would rather we keep the semantics consistent.
> On Oct. 15, 2014, 7:41 a.m., Adam B wrote:
> > src/tests/reconciliation_tests.cpp, line 851
> > <https://reviews.apache.org/r/26702/diff/1/?file=720981#file720981line851>
> >
> > Validate uuid?
Reconciliation updates generate new uuids.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review56674
-----------------------------------------------------------
On Oct. 14, 2014, 6:07 p.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2014, 6:07 p.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp cb46cec0674b3aa031450c5b4f48f4f8bb92767d
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>
Re: Review Request 26702: Updated reconciliation semantics to take the
task's unacknowledged state into account.
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26702/#review56674
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/26702/#comment97078>
Should reconciliation on slave reregistration return to the slave each task's latest state, or its latest unacknowledged state? Would it ever matter?
Looking at Slave::reregistered(), it seems it doesn't even look at the state. It just checks to see if the slave knows that taskID and sends back TASK_LOST if the task is unknown.
src/tests/reconciliation_tests.cpp
<https://reviews.apache.org/r/26702/#comment97079>
Times(1) is implicit, unnecessary.
src/tests/reconciliation_tests.cpp
<https://reviews.apache.org/r/26702/#comment97080>
Validate uuid?
- Adam B
On Oct. 14, 2014, 11:07 a.m., Vinod Kone wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26702/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2014, 11:07 a.m.)
>
>
> Review request for mesos, Adam B, Ben Mahler, and Niklas Nielsen.
>
>
> Bugs: MESOS-1799 and MESOS-1817
> https://issues.apache.org/jira/browse/MESOS-1799
> https://issues.apache.org/jira/browse/MESOS-1817
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Master responds to reconciliation requests with unacknowledged state of task instead of latest state.
>
>
> Diffs
> -----
>
> src/master/master.cpp cb46cec0674b3aa031450c5b4f48f4f8bb92767d
> src/tests/reconciliation_tests.cpp 400c5c035a2cadd408636021aafb3888546f0081
>
> Diff: https://reviews.apache.org/r/26702/diff/
>
>
> Testing
> -------
>
> make check
>
> Ran new test 1000 times.
>
>
> Thanks,
>
> Vinod Kone
>
>