Re: Review Request 44274: Added the ability to stop running the scheduler library process.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 4, 2016, 12:03 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44274/
> ---
> 
> (Updated March 4, 2016, 12:03 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the ability to stop running the scheduler library
> process so that no future callbacks are delivered to the scheduler.
> 
> This helps us during testing to ensure no further callbacks happen
> to stack allocated mock objects.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44274/diff/
> 
> 
> Testing
> ---
> 
> make check. This is used later in the chain when we move the interface to use 
> the `stop()` function.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44274: Added the ability to stop running the scheduler library process.

[Anand Mazumdar]

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

(Updated March 4, 2016, 12:03 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Made `stop()` explicitly terminate the process based on feedback from Vinod


Bugs: MESOS-4029
https://issues.apache.org/jira/browse/MESOS-4029


Repository: mesos


Description
---

This change adds the ability to stop running the scheduler library
process so that no future callbacks are delivered to the scheduler.

This helps us during testing to ensure no further callbacks happen
to stack allocated mock objects.


Diffs (updated)
-

  include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 

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


Testing
---

make check. This is used later in the chain when we move the interface to use 
the `stop()` function.


Thanks,

Anand Mazumdar

Re: Review Request 44274: Added the ability to stop running the scheduler library process.

[Anand Mazumdar]

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

(Updated March 3, 2016, 1:30 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Vinod


Bugs: MESOS-4029
https://issues.apache.org/jira/browse/MESOS-4029


Repository: mesos


Description
---

This change adds the ability to stop running the scheduler library
process so that no future callbacks are delivered to the scheduler.

This helps us during testing to ensure no further callbacks happen
to stack allocated mock objects.


Diffs (updated)
-

  include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 

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


Testing
---

make check. This is used later in the chain when we move the interface to use 
the `stop()` function.


Thanks,

Anand Mazumdar

Re: Review Request 44274: Added the ability to stop running the scheduler library process.

[Anand Mazumdar]

> On March 2, 2016, 11:59 p.m., Vinod Kone wrote:
> > src/scheduler/scheduler.cpp, line 463
> > 
> >
> > don't you need to guard this with !running.load() check?

To be more precise, I moved all the `running.load()` calls to the `async` 
callbacks.


> On March 2, 2016, 11:59 p.m., Vinod Kone wrote:
> > include/mesos/v1/scheduler.hpp, line 86
> > 
> >
> > s/interface/library/
> > 
> > what about calls? can they make calls after calling stop? looks like no 
> > from the implementation below, but it needs to be commented here.
> > 
> > also here it says "no callbacks" but down below it says "atmost one 
> > callback" ?

+1. My bad, I had tried to follow the driver that punts on adding the "atmost 
one" detail in the header.


- Anand


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


On March 3, 2016, 1:30 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44274/
> ---
> 
> (Updated March 3, 2016, 1:30 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the ability to stop running the scheduler library
> process so that no future callbacks are delivered to the scheduler.
> 
> This helps us during testing to ensure no further callbacks happen
> to stack allocated mock objects.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44274/diff/
> 
> 
> Testing
> ---
> 
> make check. This is used later in the chain when we move the interface to use 
> the `stop()` function.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 44274: Added the ability to stop running the scheduler library process.

[Vinod Kone]

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




include/mesos/v1/scheduler.hpp (line 86)


s/interface/library/

what about calls? can they make calls after calling stop? looks like no 
from the implementation below, but it needs to be commented here.

also here it says "no callbacks" but down below it says "atmost one 
callback" ?



src/scheduler/scheduler.cpp (line 314)


s/connection attempt/connected event/



src/scheduler/scheduler.cpp (line 409)


s/change\/disconnection/detected event/



src/scheduler/scheduler.cpp (line 463)


don't you need to guard this with !running.load() check?


- Vinod Kone


On March 2, 2016, 6:21 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44274/
> ---
> 
> (Updated March 2, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4029
> https://issues.apache.org/jira/browse/MESOS-4029
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the ability to stop running the scheduler library
> process so that no future callbacks are delivered to the scheduler.
> 
> This helps us during testing to ensure no further callbacks happen
> to stack allocated mock objects.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
>   src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 
> 
> Diff: https://reviews.apache.org/r/44274/diff/
> 
> 
> Testing
> ---
> 
> make check. This is used later in the chain when we move the interface to use 
> the `stop()` function.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 44274: Added the ability to stop running the scheduler library process.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Bugs: MESOS-4029
https://issues.apache.org/jira/browse/MESOS-4029


Repository: mesos


Description
---

This change adds the ability to stop running the scheduler library
process so that no future callbacks are delivered to the scheduler.

This helps us during testing to ensure no further callbacks happen
to stack allocated mock objects.


Diffs
-

  include/mesos/v1/scheduler.hpp a0fb73be2178171497dcce5c497ca38c3002f667 
  src/scheduler/scheduler.cpp 7ea1c2567f37a73160bca346a25bb2f0c54e71a0 

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


Testing
---

make check. This is used later in the chain when we move the interface to use 
the `stop()` function.


Thanks,

Anand Mazumdar