You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2013/08/22 20:34:08 UTC
Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
Bugs: MESOS-655
https://issues.apache.org/jira/browse/MESOS-655
Repository: mesos-git
Description
-------
- In production, Masters exit by either LOG(FATAL) or exit().
- Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
Diffs
-----
src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
Diff: https://reviews.apache.org/r/13746/diff/
Testing
-------
make check on Linux and OSX
Thanks,
Jiang Yan Xu
Re: Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On Aug. 22, 2013, 6:39 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 212
> > <https://reviews.apache.org/r/13746/diff/1/?file=343857#file343857line212>
> >
> > What about adding a boolean to removeFramework which indicates what kind of removal to do, i.e., whether or not to send messages to schedulers or slaves?
This is certainly achievable but I am afraid not in a clean way. This requires us to put boolean flags on both removeFramework and removeSlave, which do three things:
1) Send multiple messages
2) Update data structures for continuous running
3) Free up resources
We only need the third kind in the destruction process, which means the first two kinds, which are 80% of the lines, have to be put in multiple branches which reduces the readability of the logic flow in these methods. Thoughts?
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/#review25420
-----------------------------------------------------------
On Aug. 22, 2013, 6:34 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13746/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2013, 6:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-655
> https://issues.apache.org/jira/browse/MESOS-655
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> - In production, Masters exit by either LOG(FATAL) or exit().
> - Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
>
>
> Diffs
> -----
>
> src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
> src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
>
> Diff: https://reviews.apache.org/r/13746/diff/
>
>
> Testing
> -------
>
> make check on Linux and OSX
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
Posted by Vinod Kone <vi...@gmail.com>.
> On Aug. 22, 2013, 6:39 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 212
> > <https://reviews.apache.org/r/13746/diff/1/?file=343857#file343857line212>
> >
> > What about adding a boolean to removeFramework which indicates what kind of removal to do, i.e., whether or not to send messages to schedulers or slaves?
>
> Jiang Yan Xu wrote:
> This is certainly achievable but I am afraid not in a clean way. This requires us to put boolean flags on both removeFramework and removeSlave, which do three things:
> 1) Send multiple messages
> 2) Update data structures for continuous running
> 3) Free up resources
>
> We only need the third kind in the destruction process, which means the first two kinds, which are 80% of the lines, have to be put in multiple branches which reduces the readability of the logic flow in these methods. Thoughts?
>
+1 to what yan said. i think the code is going to be messy if we want to plumb the boolean flag.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/#review25420
-----------------------------------------------------------
On Aug. 22, 2013, 7:36 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13746/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2013, 7:36 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-655
> https://issues.apache.org/jira/browse/MESOS-655
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> - In production, Masters exit by either LOG(FATAL) or exit().
> - Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
>
>
> Diffs
> -----
>
> src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
> src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
>
> Diff: https://reviews.apache.org/r/13746/diff/
>
>
> Testing
> -------
>
> make check on Linux and OSX
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/#review25420
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/13746/#comment49832>
What about adding a boolean to removeFramework which indicates what kind of removal to do, i.e., whether or not to send messages to schedulers or slaves?
- Benjamin Hindman
On Aug. 22, 2013, 6:34 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13746/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2013, 6:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-655
> https://issues.apache.org/jira/browse/MESOS-655
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> - In production, Masters exit by either LOG(FATAL) or exit().
> - Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
>
>
> Diffs
> -----
>
> src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
> src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
>
> Diff: https://reviews.apache.org/r/13746/diff/
>
>
> Testing
> -------
>
> make check on Linux and OSX
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/#review25490
-----------------------------------------------------------
committed. pls mark as submitted.
- Vinod Kone
On Aug. 22, 2013, 7:36 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13746/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2013, 7:36 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-655
> https://issues.apache.org/jira/browse/MESOS-655
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> - In production, Masters exit by either LOG(FATAL) or exit().
> - Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
>
>
> Diffs
> -----
>
> src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
> src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
>
> Diff: https://reviews.apache.org/r/13746/diff/
>
>
> Testing
> -------
>
> make check on Linux and OSX
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/#review25446
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Aug. 22, 2013, 7:36 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13746/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2013, 7:36 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-655
> https://issues.apache.org/jira/browse/MESOS-655
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> - In production, Masters exit by either LOG(FATAL) or exit().
> - Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
>
>
> Diffs
> -----
>
> src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
> src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
>
> Diff: https://reviews.apache.org/r/13746/diff/
>
>
> Testing
> -------
>
> make check on Linux and OSX
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/
-----------------------------------------------------------
(Updated Aug. 22, 2013, 7:36 p.m.)
Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
Changes
-------
Addressed Vinod's comments.
Bugs: MESOS-655
https://issues.apache.org/jira/browse/MESOS-655
Repository: mesos-git
Description
-------
- In production, Masters exit by either LOG(FATAL) or exit().
- Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
Diffs (updated)
-----
src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
Diff: https://reviews.apache.org/r/13746/diff/
Testing
-------
make check on Linux and OSX
Thanks,
Jiang Yan Xu
Re: Review Request 13746: Fixed a bug that caused Master::~Master() to send
messages to the schedulers and slaves. It was not the way Masters exit in
production.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13746/#review25422
-----------------------------------------------------------
src/master/master.cpp
<https://reviews.apache.org/r/13746/#comment49834>
Make sure to kill any remaining tasks in the slave that were not in the framework map (e.g, framework hasnt reregistered yet)
- Vinod Kone
On Aug. 22, 2013, 6:34 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13746/
> -----------------------------------------------------------
>
> (Updated Aug. 22, 2013, 6:34 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Ian Downes, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-655
> https://issues.apache.org/jira/browse/MESOS-655
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> - In production, Masters exit by either LOG(FATAL) or exit().
> - Also fixed tests that incorrectly relied on the messages sent by Master::~Master().
>
>
> Diffs
> -----
>
> src/master/master.cpp d53b8bb97da45834790cca6e04b70b969a8d3453
> src/tests/allocator_zookeeper_tests.cpp b84fa86274c80cc85e2b2eeadd6eb08da34433db
>
> Diff: https://reviews.apache.org/r/13746/diff/
>
>
> Testing
> -------
>
> make check on Linux and OSX
>
>
> Thanks,
>
> Jiang Yan Xu
>
>