Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-07-01 Thread Vinod Kone 

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

(Updated July 1, 2015, 10:16 p.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and Marco 
Massenzio.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 34ce744f84465ecc9aeecd5fdc3d06047a4b7d92 
  src/scheduler/scheduler.cpp f360e4d062488986b14e3d48d140996e8ed9e7d6 
  src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 

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


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-07-01 Thread Vinod Kone 


> On June 28, 2015, 11:56 p.m., Benjamin Hindman wrote:
> > src/master/master.cpp, line 2247
> > 
> >
> > How about a comment that captures the backwards compatible semantics 
> > you're accounting for here?

Not sure what you mean but I added comments.


- Vinod


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


On July 1, 2015, 5:24 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35855/
> ---
> 
> (Updated July 1, 2015, 5:24 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and 
> Marco Massenzio.
> 
> 
> Bugs: MESOS-2551
> https://issues.apache.org/jira/browse/MESOS-2551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 34ce744f84465ecc9aeecd5fdc3d06047a4b7d92 
>   src/scheduler/scheduler.cpp f360e4d062488986b14e3d48d140996e8ed9e7d6 
>   src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 
> 
> Diff: https://reviews.apache.org/r/35855/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-07-01 Thread Vinod Kone 


> On June 25, 2015, 8:25 a.m., Marco Massenzio wrote:
> > src/master/master.cpp, line 2831
> > 
> >
> > not sure this is allowed in our code style, but this could be a good 
> > place for `nullptr`
> > 
> > Also: shouldn't we LOG(ERROR) the fact that a given `offerId` results 
> > in a NULL ptr (in an `else` clause)?

since it is race, i did a LOG(WARNING).


> On June 25, 2015, 8:25 a.m., Marco Massenzio wrote:
> > src/tests/scheduler_tests.cpp, line 704
> > 
> >
> > could this be more specific ("exactly" one)?

yup. also made it an assert because it doesnt make sense to move forward 
otherwise.


- Vinod


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


On July 1, 2015, 5:24 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35855/
> ---
> 
> (Updated July 1, 2015, 5:24 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and 
> Marco Massenzio.
> 
> 
> Bugs: MESOS-2551
> https://issues.apache.org/jira/browse/MESOS-2551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 34ce744f84465ecc9aeecd5fdc3d06047a4b7d92 
>   src/scheduler/scheduler.cpp f360e4d062488986b14e3d48d140996e8ed9e7d6 
>   src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 
> 
> Diff: https://reviews.apache.org/r/35855/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-06-30 Thread Kapil Arya 

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



src/master/master.cpp (line 1672)


Nice catch. Seems like you've caught as potential segfault (here and below) 
:-).


- Kapil Arya


On July 1, 2015, 1:24 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35855/
> ---
> 
> (Updated July 1, 2015, 1:24 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and 
> Marco Massenzio.
> 
> 
> Bugs: MESOS-2551
> https://issues.apache.org/jira/browse/MESOS-2551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 34ce744f84465ecc9aeecd5fdc3d06047a4b7d92 
>   src/scheduler/scheduler.cpp f360e4d062488986b14e3d48d140996e8ed9e7d6 
>   src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 
> 
> Diff: https://reviews.apache.org/r/35855/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-06-30 Thread Vinod Kone 

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

(Updated July 1, 2015, 5:24 a.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and Marco 
Massenzio.


Changes
---

rebased on top of Call protobuf changes. haven't addressed comments yet.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 34ce744f84465ecc9aeecd5fdc3d06047a4b7d92 
  src/scheduler/scheduler.cpp f360e4d062488986b14e3d48d140996e8ed9e7d6 
  src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 

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


Testing
---

make check


Thanks,

Vinod Kone

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-06-28 Thread Benjamin Hindman 


> On June 25, 2015, 3:57 a.m., Isabel Jimenez wrote:
> > src/master/master.cpp, line 1654
> > 
> >
> > could we have here consistent braces syntax with scheduler.cpp switch 
> > case?

Perhaps for consistency, and safety, we just add braces for each of these 
switch cases.


- Benjamin


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


On June 25, 2015, 1:11 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35855/
> ---
> 
> (Updated June 25, 2015, 1:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and 
> Marco Massenzio.
> 
> 
> Bugs: MESOS-2551
> https://issues.apache.org/jira/browse/MESOS-2551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
>   src/scheduler/scheduler.cpp 1efc6fb351e49deaa8f626823592bc9155f5137b 
>   src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 
> 
> Diff: https://reviews.apache.org/r/35855/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-06-28 Thread Benjamin Hindman 

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

Ship it!



src/master/master.cpp (line 2242)


How about a comment that captures the backwards compatible semantics you're 
accounting for here?


- Benjamin Hindman


On June 25, 2015, 1:11 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35855/
> ---
> 
> (Updated June 25, 2015, 1:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and 
> Marco Massenzio.
> 
> 
> Bugs: MESOS-2551
> https://issues.apache.org/jira/browse/MESOS-2551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
>   src/scheduler/scheduler.cpp 1efc6fb351e49deaa8f626823592bc9155f5137b 
>   src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 
> 
> Diff: https://reviews.apache.org/r/35855/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-06-25 Thread Marco Massenzio 

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

Ship it!



src/master/master.cpp (line 2826)


not sure this is allowed in our code style, but this could be a good place 
for `nullptr`

Also: shouldn't we LOG(ERROR) the fact that a given `offerId` results in a 
NULL ptr (in an `else` clause)?



src/tests/scheduler_tests.cpp (line 704)


could this be more specific ("exactly" one)?


- Marco Massenzio


On June 25, 2015, 1:11 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35855/
> ---
> 
> (Updated June 25, 2015, 1:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and 
> Marco Massenzio.
> 
> 
> Bugs: MESOS-2551
> https://issues.apache.org/jira/browse/MESOS-2551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
>   src/scheduler/scheduler.cpp 1efc6fb351e49deaa8f626823592bc9155f5137b 
>   src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 
> 
> Diff: https://reviews.apache.org/r/35855/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-06-24 Thread Isabel Jimenez 

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



src/master/master.cpp (line 1653)


could we have here consistent braces syntax with scheduler.cpp switch case?


- Isabel Jimenez


On June 25, 2015, 1:11 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35855/
> ---
> 
> (Updated June 25, 2015, 1:11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and 
> Marco Massenzio.
> 
> 
> Bugs: MESOS-2551
> https://issues.apache.org/jira/browse/MESOS-2551
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
>   src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
>   src/scheduler/scheduler.cpp 1efc6fb351e49deaa8f626823592bc9155f5137b 
>   src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 
> 
> Diff: https://reviews.apache.org/r/35855/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 35855: Added Decline call support to the master and C++ scheduler library.

2015-06-24 Thread Vinod Kone 

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

(Updated June 25, 2015, 1:11 a.m.)


Review request for mesos, Anand Mazumdar, Ben Mahler, Isabel Jimenez, and Marco 
Massenzio.


Changes
---

fixed missed return statements.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.hpp af83d3e82d2c161b3cc4583e78a8cbbd2f9a4064 
  src/master/master.cpp 0782b543b451921d2240958c7ef612a9e30972df 
  src/scheduler/scheduler.cpp 1efc6fb351e49deaa8f626823592bc9155f5137b 
  src/tests/scheduler_tests.cpp cbe6c91a1b4f864ceb11cf062da0ada6c9666f9f 

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


Testing
---

make check


Thanks,

Vinod Kone