You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <an...@apache.org> on 2017/09/07 05:54:19 UTC
Review Request 62148: Made the `stop()` logic in the scheduler library
simpler.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62148/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
Repository: mesos
Description
-------
Instead of relying on `Clock::pause()/settle()` to ensure that no
callbacks on the scheduler were in flight, the test
helper can wait on the future returned by `stop()` now. This is
possible as we acquire the mutex explicitly in `stop()` to ensure
that there can't be any queued callbacks that get executed after
`TestMesos` is destroyed.
Diffs
-----
include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661
src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744
src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554
Diff: https://reviews.apache.org/r/62148/diff/1/
Testing
-------
Thanks,
Anand Mazumdar
Re: Review Request 62148: Made the `stop()` logic in the scheduler
library simpler.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62148/#review184820
-----------------------------------------------------------
FAIL: Some Mesos tests failed. Please check http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stdout.log and http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs/mesos-tests-stderr.log for any relevant errors
Reviews applied: [62148]
Logs available here: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62148/logs
- Mesos Reviewbot Windows
On Sept. 7, 2017, 5:54 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62148/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 5:54 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of relying on `Clock::pause()/settle()` to ensure that no
> callbacks on the scheduler were in flight, the test
> helper can wait on the future returned by `stop()` now. This is
> possible as we acquire the mutex explicitly in `stop()` to ensure
> that there can't be any queued callbacks that get executed after
> `TestMesos` is destroyed.
>
>
> Diffs
> -----
>
> include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661
> src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744
> src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554
>
>
> Diff: https://reviews.apache.org/r/62148/diff/1/
>
>
> Testing
> -------
>
> make check (Ensured tests still work when run in a loop)
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 62148: Made the `stop()` logic in the scheduler
library simpler.
Posted by Anand Mazumdar <an...@apache.org>.
> On Sept. 7, 2017, 3:33 p.m., Benjamin Hindman wrote:
> > include/mesos/v1/scheduler.hpp
> > Lines 25 (patched)
> > <https://reviews.apache.org/r/62148/diff/1/?file=1817001#file1817001line25>
> >
> > Have we pulled in libprocess dependencies in public headers in the past?
hmm, most of our module/hooks interfaces already had libprocess dependency (future.hpp) in their headers. So, I assumed that it should be fine to add it. That being said; both the C++ driver/v1 scheduler/executor library haven't had a dependency in the past.
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62148/#review184823
-----------------------------------------------------------
On Sept. 7, 2017, 5:54 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62148/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 5:54 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of relying on `Clock::pause()/settle()` to ensure that no
> callbacks on the scheduler were in flight, the test
> helper can wait on the future returned by `stop()` now. This is
> possible as we acquire the mutex explicitly in `stop()` to ensure
> that there can't be any queued callbacks that get executed after
> `TestMesos` is destroyed.
>
>
> Diffs
> -----
>
> include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661
> src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744
> src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554
>
>
> Diff: https://reviews.apache.org/r/62148/diff/1/
>
>
> Testing
> -------
>
> make check (Ensured tests still work when run in a loop)
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 62148: Made the `stop()` logic in the scheduler
library simpler.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62148/#review184823
-----------------------------------------------------------
include/mesos/v1/scheduler.hpp
Lines 25 (patched)
<https://reviews.apache.org/r/62148/#comment261015>
Have we pulled in libprocess dependencies in public headers in the past?
- Benjamin Hindman
On Sept. 7, 2017, 5:54 a.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62148/
> -----------------------------------------------------------
>
> (Updated Sept. 7, 2017, 5:54 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Instead of relying on `Clock::pause()/settle()` to ensure that no
> callbacks on the scheduler were in flight, the test
> helper can wait on the future returned by `stop()` now. This is
> possible as we acquire the mutex explicitly in `stop()` to ensure
> that there can't be any queued callbacks that get executed after
> `TestMesos` is destroyed.
>
>
> Diffs
> -----
>
> include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661
> src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744
> src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554
>
>
> Diff: https://reviews.apache.org/r/62148/diff/1/
>
>
> Testing
> -------
>
> make check (Ensured tests still work when run in a loop)
>
>
> Thanks,
>
> Anand Mazumdar
>
>
Re: Review Request 62148: Made the `stop()` logic in the scheduler
library simpler.
Posted by Anand Mazumdar <an...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62148/
-----------------------------------------------------------
(Updated Sept. 7, 2017, 5:54 a.m.)
Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Vinod Kone.
Changes
-------
Edited testing done
Repository: mesos
Description
-------
Instead of relying on `Clock::pause()/settle()` to ensure that no
callbacks on the scheduler were in flight, the test
helper can wait on the future returned by `stop()` now. This is
possible as we acquire the mutex explicitly in `stop()` to ensure
that there can't be any queued callbacks that get executed after
`TestMesos` is destroyed.
Diffs
-----
include/mesos/v1/scheduler.hpp d56e08861d5190ef66992d383dc5710d8f6ce661
src/scheduler/scheduler.cpp ce69258027ed50867569374d2d827fc3cc651744
src/tests/mesos.hpp 444c75763aea995708db0b17eafee2d22c912554
Diff: https://reviews.apache.org/r/62148/diff/1/
Testing (updated)
-------
make check (Ensured tests still work when run in a loop)
Thanks,
Anand Mazumdar