Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

2015-06-15 Thread Michael Park

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

(Updated June 15, 2015, 12:39 p.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Addressed BenH's comment. The slave now sends `TASK_LOST` status updates since 
the fact that the checkpointed resources are available on the slave is not a 
global invariant we try to guarantee.


Repository: mesos


Description
---

No bug was observed (yet), but realized I forgot about this in the dynamic 
reservations patches.


Diffs (updated)
-

  src/slave/slave.cpp 67732a40ef683cb0188425f0bba8fe8ab83e461c 

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


Testing
---

`make check`


Thanks,

Michael Park



Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

2015-06-15 Thread Michael Park

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



src/slave/slave.cpp
https://reviews.apache.org/r/35433/#comment140336

Introduce a new `REASON_` enum for this.



src/slave/slave.cpp
https://reviews.apache.org/r/35433/#comment140337

Same here


- Michael Park


On June 15, 2015, 12:39 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35433/
 ---
 
 (Updated June 15, 2015, 12:39 p.m.)
 
 
 Review request for mesos, Benjamin Hindman and Jie Yu.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 No bug was observed (yet), but realized I forgot about this in the dynamic 
 reservations patches.
 
 
 Diffs
 -
 
   src/slave/slave.cpp 67732a40ef683cb0188425f0bba8fe8ab83e461c 
 
 Diff: https://reviews.apache.org/r/35433/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

2015-06-15 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [35433]

All tests passed.

- Mesos ReviewBot


On June 15, 2015, 12:39 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35433/
 ---
 
 (Updated June 15, 2015, 12:39 p.m.)
 
 
 Review request for mesos, Benjamin Hindman and Jie Yu.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 No bug was observed (yet), but realized I forgot about this in the dynamic 
 reservations patches.
 
 
 Diffs
 -
 
   src/slave/slave.cpp 67732a40ef683cb0188425f0bba8fe8ab83e461c 
 
 Diff: https://reviews.apache.org/r/35433/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

2015-06-14 Thread Benjamin Hindman

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


Just so I understand, does this mean if we happen to get in the unfortunate 
situation where a slave has neglected to get the dynamic reservation because it 
was just starting up and then it gets the task launch it will shutdown the 
slave because the CHECK will fail? I would expect the slave to simply send a 
TASK_LOST. Said another way, this is not an assertion our code guarantees. If 
instead we were waiting for some kind of an ack from the slave that it received 
the dynamic reservation before it send the task launch then a CHECK would make 
sense.

- Benjamin Hindman


On June 13, 2015, 9:43 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35433/
 ---
 
 (Updated June 13, 2015, 9:43 p.m.)
 
 
 Review request for mesos, Benjamin Hindman and Jie Yu.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 No bug was observed (yet), but realized I forgot about this in the dynamic 
 reservations patches.
 
 
 Diffs
 -
 
   src/slave/slave.cpp 9af69d8f0b28c9441c684886c52320378f9b2869 
 
 Diff: https://reviews.apache.org/r/35433/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Re: Review Request 35433: CHECK that checkpointed resources exist on the slave.

2015-06-13 Thread Mesos ReviewBot

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


Patch looks great!

Reviews applied: [35433]

All tests passed.

- Mesos ReviewBot


On June 13, 2015, 9:43 p.m., Michael Park wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/35433/
 ---
 
 (Updated June 13, 2015, 9:43 p.m.)
 
 
 Review request for mesos, Benjamin Hindman and Jie Yu.
 
 
 Repository: mesos
 
 
 Description
 ---
 
 No bug was observed (yet), but realized I forgot about this in the dynamic 
 reservations patches.
 
 
 Diffs
 -
 
   src/slave/slave.cpp 9af69d8f0b28c9441c684886c52320378f9b2869 
 
 Diff: https://reviews.apache.org/r/35433/diff/
 
 
 Testing
 ---
 
 `make check`
 
 
 Thanks,
 
 Michael Park
 




Review Request 35433: CHECK that checkpointed resources exist on the slave.

2015-06-13 Thread Michael Park

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

Review request for mesos, Benjamin Hindman and Jie Yu.


Repository: mesos


Description
---

No bug was observed (yet), but realized I forgot about this in the dynamic 
reservations patches.


Diffs
-

  src/slave/slave.cpp 9af69d8f0b28c9441c684886c52320378f9b2869 

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


Testing
---

`make check`


Thanks,

Michael Park