Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-28 Thread Jay Guo 


> On June 27, 2016, 8:29 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 2706
> > 
> >
> > I was looking at this code today and realized this might not be safe? 
> > What's the guarantee that a given `Task*` is still valid and not being 
> > accessed by other actor (master) when we are executing the lambda? Is the 
> > lambda guaranteed to be executed in master's context? cc @anand @mcypark
> 
> Anand Mazumdar wrote:
> Good catch! This needs to go inside `defer`.
> 
> Vinod Kone wrote:
> `defer`ing solves the concurrent access problem, but not the `Task*` 
> becoming invalid problem?

hmm...good point... Copying instead of referencing may give us a valid 
snapshot, although it would be too expensive, right? How about using 
`vector` and do a check before return? However this may results in 
incorrect number of tasks requested by user.


- Jay


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


On June 18, 2016, 2:33 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48400/
> ---
> 
> (Updated June 18, 2016, 2:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a helper function _tasks() to collect tasks. This is
> to be used by both tasks() and getTasks() in operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp dca814b070e513420175a2df6db7844fd425c3fa 
>   src/master/master.hpp 2bddfe1c6638281e3ed34146490bc33f66d87d5c 
> 
> Diff: https://reviews.apache.org/r/48400/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-27 Thread Vinod Kone 


> On June 27, 2016, 8:29 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 2706
> > 
> >
> > I was looking at this code today and realized this might not be safe? 
> > What's the guarantee that a given `Task*` is still valid and not being 
> > accessed by other actor (master) when we are executing the lambda? Is the 
> > lambda guaranteed to be executed in master's context? cc @anand @mcypark
> 
> Anand Mazumdar wrote:
> Good catch! This needs to go inside `defer`.

`defer`ing solves the concurrent access problem, but not the `Task*` becoming 
invalid problem?


- Vinod


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


On June 18, 2016, 2:33 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48400/
> ---
> 
> (Updated June 18, 2016, 2:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a helper function _tasks() to collect tasks. This is
> to be used by both tasks() and getTasks() in operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp dca814b070e513420175a2df6db7844fd425c3fa 
>   src/master/master.hpp 2bddfe1c6638281e3ed34146490bc33f66d87d5c 
> 
> Diff: https://reviews.apache.org/r/48400/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-27 Thread Anand Mazumdar 


> On June 27, 2016, 8:29 p.m., Vinod Kone wrote:
> > src/master/http.cpp, line 2706
> > 
> >
> > I was looking at this code today and realized this might not be safe? 
> > What's the guarantee that a given `Task*` is still valid and not being 
> > accessed by other actor (master) when we are executing the lambda? Is the 
> > lambda guaranteed to be executed in master's context? cc @anand @mcypark

Good catch! This needs to go inside `defer`.


- Anand


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


On June 18, 2016, 2:33 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48400/
> ---
> 
> (Updated June 18, 2016, 2:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a helper function _tasks() to collect tasks. This is
> to be used by both tasks() and getTasks() in operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp dca814b070e513420175a2df6db7844fd425c3fa 
>   src/master/master.hpp 2bddfe1c6638281e3ed34146490bc33f66d87d5c 
> 
> Diff: https://reviews.apache.org/r/48400/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-27 Thread Vinod Kone 

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




src/master/http.cpp (line 2706)


I was looking at this code today and realized this might not be safe? 
What's the guarantee that a given `Task*` is still valid and not being accessed 
by other actor (master) when we are executing the lambda? Is the lambda 
guaranteed to be executed in master's context? cc @anand @mcypark


- Vinod Kone


On June 18, 2016, 2:33 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48400/
> ---
> 
> (Updated June 18, 2016, 2:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a helper function _tasks() to collect tasks. This is
> to be used by both tasks() and getTasks() in operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp dca814b070e513420175a2df6db7844fd425c3fa 
>   src/master/master.hpp 2bddfe1c6638281e3ed34146490bc33f66d87d5c 
> 
> Diff: https://reviews.apache.org/r/48400/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-17 Thread Jay Guo 

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

(Updated June 18, 2016, 2:33 a.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

Add a helper function _tasks() to collect tasks. This is
to be used by both tasks() and getTasks() in operator API.


Diffs (updated)
-

  src/master/http.cpp dca814b070e513420175a2df6db7844fd425c3fa 
  src/master/master.hpp 2bddfe1c6638281e3ed34146490bc33f66d87d5c 

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


Testing
---

make check on ubuntu 14.04


Thanks,

Jay Guo

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-09 Thread Jay Guo 

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

(Updated June 9, 2016, 11:13 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Fix indentation.


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


Repository: mesos


Description
---

Add a helper function _tasks() to collect tasks. This is
to be used by both tasks() and getTasks() in operator API.


Diffs (updated)
-

  src/master/http.cpp 6e1bf9557a854a89fa9173223295816a9e114e7c 
  src/master/master.hpp 2c45dab291a153b42809ab12e4252bf58559feeb 

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


Testing
---

make check on ubuntu 14.04


Thanks,

Jay Guo

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-09 Thread Vinod Kone 

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


Ship it!




Ship It!

- Vinod Kone


On June 9, 2016, midnight, Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48400/
> ---
> 
> (Updated June 9, 2016, midnight)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a helper function _tasks() to collect tasks. This is
> to be used by both tasks() and getTasks() in operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
> 
> Diff: https://reviews.apache.org/r/48400/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-08 Thread Jay Guo 

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

(Updated June 9, 2016, midnight)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Add a helper function _tasks() to collect tasks. This is
to be used by both tasks() and getTasks() in operator API.


Diffs (updated)
-

  src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
  src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 

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


Testing
---

make check on ubuntu 14.04


Thanks,

Jay Guo

Re: Review Request 48400: Extract task collection logic to helper function.

2016-06-08 Thread Vinod Kone 

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




src/master/master.hpp (lines 1245 - 1246)


no need to take refs for these primitive types. just take them by value.

...
size_t limit,
size_t offset,


also see my comments in the last review in this chain. i think this 
function's responsiblity can be little different than what you have here.


- Vinod Kone


On June 8, 2016, 7:17 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48400/
> ---
> 
> (Updated June 8, 2016, 7:17 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5493
> https://issues.apache.org/jira/browse/MESOS-5493
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a helper function _tasks() to collect tasks. This is
> to be used by both tasks() and getTasks() in operator API.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 4b2d1386e1ecb447b597a432f9df9adaa5c3aa37 
>   src/master/master.hpp 790da3ce686401c378ad9c62d497d60893c4ce41 
> 
> Diff: https://reviews.apache.org/r/48400/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>