Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

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

Ship it!


Ship It!

- Vinod Kone


On Oct. 16, 2015, 2:36 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Oct. 16, 2015, 2:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ba12a83 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Oct. 16, 2015, 2:36 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Oct. 16, 2015, 2:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ba12a83 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

> On 十月 15, 2015, 6:38 p.m., Vinod Kone wrote:
> > src/tests/status_update_manager_tests.cpp, line 844
> > 
> >
> > new line.

Do you mean to add an another new line? I found two new lines are added before 
all functions in this test file, so I add an another new line before this 
function.


- Yong Qiao


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


On 十月 16, 2015, 2:36 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated 十月 16, 2015, 2:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ba12a83 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated 十月 16, 2015, 2:36 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs (updated)
-

  src/master/master.cpp ba12a83 
  src/tests/status_update_manager_tests.cpp 9970d71 

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


Testing
---

UT:
1. Write a test for this change.
2. make successfully!
3. make check successfully!
4. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated 十月 16, 2015, 2:03 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs (updated)
-

  src/master/master.cpp 6bee4f3 
  src/tests/status_update_manager_tests.cpp 9970d71 

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


Testing
---

UT:
1. Write a test for this change.
2. make successfully!
3. make check successfully!
4. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

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

Ship it!



src/master/master.cpp (line 6022)


s/transited/transitioned/ ?



src/tests/status_update_manager_tests.cpp (line 844)


new line.



src/tests/status_update_manager_tests.cpp (lines 845 - 849)


This comment is a bit confusing. The fix in this review is for master and 
not slave/status update manager. Why is this comment talking about semantics of 
slave/status update manager?

How about?

// This test verifies that if master receives a status update for an 
already terminated
// task it forwards it without changing the state of the task.



src/tests/status_update_manager_tests.cpp (line 943)


No need for settle() here.


- Vinod Kone


On Sept. 24, 2015, 9:45 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 24, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6bee4f3 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Sept. 24, 2015, 9:45 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 24, 2015, 9:45 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6bee4f3 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 1. Write a test for this change.
> 2. make successfully!
> 3. make check successfully!
> 4. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated Sept. 24, 2015, 9:45 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs
-

  src/master/master.cpp 6bee4f3 
  src/tests/status_update_manager_tests.cpp 9970d71 

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


Testing (updated)
---

UT:
1. Write a test for this change.
2. make successfully!
3. make check successfully!
4. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

> On Sept. 17, 2015, 10:25 p.m., Vinod Kone wrote:
> > Can you write a test for this?
> 
> Yong Qiao Wang wrote:
> I find the code changes in this patch does not be tested with an 
> end-to-end case except to check the error log messages of master, so my test 
> strategy are:
> 
> 1. Change the python test executor to send the TASK_FINASHED with two 
> times;
> 2. Check the error log message of maser;
> 
> Test passwd!
> 
> Vinod, I am not familiar with current test framework of mesos, cloud you 
> provide some similar test code, and I can refer to write my test case for 
> this. Thanks!
> 
> Vinod Kone wrote:
> Instaed of updating python executor, write a self-contained unit test. 
> Take a look at "UnacknowledgedTerminalTask" and 
> "ReleaseResourcesForTerminalTaskWithPendingUpdates" in 
> src/tests/master_tests.cpp for inspiration on how to write such a test.
> 
> Also, instead of sending TASK_FINISHED multiple times, have the executor 
> send TASK_FINISHED followed by a TASK_KILLED. That way you can check that the 
> status of the task hasn't changed in the master.
> 
> Yong Qiao Wang wrote:
> Thanks Vinod. In Master::statusUpdate function, I find if the status 
> update is invalid(such as out-of-order updates), we still forward the updates 
> to related framework and update the metrics 
> (metrics->valid_status_updates++), is it reasonable?
> 
> Vinod Kone wrote:
> yes.

Vinod, I have appended the test code, my test strantegy are:
1. Launch a task and finish it, then the task status will be changed to 
TASK_FINISHED.
2. Send a TASK_KILLED update for the same task again.
3. Use the HTTP endpoint /tasks to check out latest status of this task, it 
should be TASK_FINISHED rather than TASK_KILLED.


- Yong Qiao


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


On Sept. 24, 2015, 9:39 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 24, 2015, 9:39 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6bee4f3 
>   src/tests/status_update_manager_tests.cpp 9970d71 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated Sept. 24, 2015, 9:39 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs (updated)
-

  src/master/master.cpp 6bee4f3 
  src/tests/status_update_manager_tests.cpp 9970d71 

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


Testing
---

UT:

1. make successfully!
2. make check successfully!
3. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

> On Sept. 17, 2015, 10:25 p.m., Vinod Kone wrote:
> > Can you write a test for this?
> 
> Yong Qiao Wang wrote:
> I find the code changes in this patch does not be tested with an 
> end-to-end case except to check the error log messages of master, so my test 
> strategy are:
> 
> 1. Change the python test executor to send the TASK_FINASHED with two 
> times;
> 2. Check the error log message of maser;
> 
> Test passwd!
> 
> Vinod, I am not familiar with current test framework of mesos, cloud you 
> provide some similar test code, and I can refer to write my test case for 
> this. Thanks!
> 
> Vinod Kone wrote:
> Instaed of updating python executor, write a self-contained unit test. 
> Take a look at "UnacknowledgedTerminalTask" and 
> "ReleaseResourcesForTerminalTaskWithPendingUpdates" in 
> src/tests/master_tests.cpp for inspiration on how to write such a test.
> 
> Also, instead of sending TASK_FINISHED multiple times, have the executor 
> send TASK_FINISHED followed by a TASK_KILLED. That way you can check that the 
> status of the task hasn't changed in the master.
> 
> Yong Qiao Wang wrote:
> Thanks Vinod. In Master::statusUpdate function, I find if the status 
> update is invalid(such as out-of-order updates), we still forward the updates 
> to related framework and update the metrics 
> (metrics->valid_status_updates++), is it reasonable?

yes.


- Vinod


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


On Sept. 18, 2015, 7:15 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 18, 2015, 7:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

> On Sept. 17, 2015, 10:25 p.m., Vinod Kone wrote:
> > Can you write a test for this?
> 
> Yong Qiao Wang wrote:
> I find the code changes in this patch does not be tested with an 
> end-to-end case except to check the error log messages of master, so my test 
> strategy are:
> 
> 1. Change the python test executor to send the TASK_FINASHED with two 
> times;
> 2. Check the error log message of maser;
> 
> Test passwd!
> 
> Vinod, I am not familiar with current test framework of mesos, cloud you 
> provide some similar test code, and I can refer to write my test case for 
> this. Thanks!
> 
> Vinod Kone wrote:
> Instaed of updating python executor, write a self-contained unit test. 
> Take a look at "UnacknowledgedTerminalTask" and 
> "ReleaseResourcesForTerminalTaskWithPendingUpdates" in 
> src/tests/master_tests.cpp for inspiration on how to write such a test.
> 
> Also, instead of sending TASK_FINISHED multiple times, have the executor 
> send TASK_FINISHED followed by a TASK_KILLED. That way you can check that the 
> status of the task hasn't changed in the master.

Thanks Vinod. In Master::statusUpdate function, I find if the status update is 
invalid(such as out-of-order updates), we still forward the updates to related 
framework and update the metrics (metrics->valid_status_updates++), is it 
reasonable?


- Yong Qiao


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


On Sept. 18, 2015, 7:15 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 18, 2015, 7:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

> On Sept. 17, 2015, 10:25 p.m., Vinod Kone wrote:
> > Can you write a test for this?
> 
> Yong Qiao Wang wrote:
> I find the code changes in this patch does not be tested with an 
> end-to-end case except to check the error log messages of master, so my test 
> strategy are:
> 
> 1. Change the python test executor to send the TASK_FINASHED with two 
> times;
> 2. Check the error log message of maser;
> 
> Test passwd!
> 
> Vinod, I am not familiar with current test framework of mesos, cloud you 
> provide some similar test code, and I can refer to write my test case for 
> this. Thanks!

Instaed of updating python executor, write a self-contained unit test. Take a 
look at "UnacknowledgedTerminalTask" and 
"ReleaseResourcesForTerminalTaskWithPendingUpdates" in 
src/tests/master_tests.cpp for inspiration on how to write such a test.

Also, instead of sending TASK_FINISHED multiple times, have the executor send 
TASK_FINISHED followed by a TASK_KILLED. That way you can check that the status 
of the task hasn't changed in the master.


- Vinod


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


On Sept. 18, 2015, 7:15 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 18, 2015, 7:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Sept. 18, 2015, 7:15 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 18, 2015, 7:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

> On Sept. 17, 2015, 10:25 p.m., Vinod Kone wrote:
> > Can you write a test for this?

I find the code changes in this patch does not be tested with an end-to-end 
case except to check the error log messages of master, so my test strategy are:

1. Change the python test executor to send the TASK_FINASHED with two times;
2. Check the error log message of maser;

Test passwd!

Vinod, I am not familiar with current test framework of mesos, cloud you 
provide some similar test code, and I can refer to write my test case for this. 
Thanks!


- Yong Qiao


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


On Sept. 18, 2015, 7:15 a.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 18, 2015, 7:15 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 1c4e7af 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated Sept. 18, 2015, 7:15 a.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs (updated)
-

  src/master/master.cpp 1c4e7af 

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


Testing
---

UT:

1. make successfully!
2. make check successfully!
3. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

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


Can you write a test for this?


src/master/master.cpp (lines 5552 - 5554)


This comment is a bit confusing. The master receiving a second terminal 
update doesn't mean the slave sent it by mistake. It could happen if the 
executor sends it too. I would change this as follows

// Once a task's state has been transitioned to terminal state, no further
// terminal updates should result in a state change. These are the same
// semantics that are enforced by the slave.


- Vinod Kone


On Sept. 5, 2015, 11:41 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 5, 2015, 11:41 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5589eca 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Sept. 5, 2015, 1:38 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 5, 2015, 1:38 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 5589eca 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> UT:
> 
> 1. make successfully!
> 2. make check successfully!
> 3. Run test framework successfully!
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

(Updated 九月 5, 2015, 1:38 p.m.)


Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs (updated)
-

  src/master/master.cpp 5589eca 

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


Testing (updated)
---

UT:

1. make successfully!
2. make check successfully!
3. Run test framework successfully!


Thanks,

Yong Qiao Wang

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

> On 九月 4, 2015, 7:42 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 4118
> > 
> >
> > Instead of not calling updateTask() the logic should be inside 
> > updateTask() to do the right thing. Note that updateTask() already has 
> > logic to deal with duplicate terminal updates.
> 
> Vinod Kone wrote:
> Also, needs a test!

Thanks a billion for your many times review.  I have updated the changes by 
your usful comments.


- Yong Qiao


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


On 九月 2, 2015, 1:13 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated 九月 2, 2015, 1:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

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



src/master/master.cpp (line 4118)


Instead of not calling updateTask() the logic should be inside updateTask() 
to do the right thing. Note that updateTask() already has logic to deal with 
duplicate terminal updates.


- Vinod Kone


On Sept. 2, 2015, 1:13 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 2, 2015, 1:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Vinod Kone]

> On Sept. 4, 2015, 7:42 p.m., Vinod Kone wrote:
> > src/master/master.cpp, line 4118
> > 
> >
> > Instead of not calling updateTask() the logic should be inside 
> > updateTask() to do the right thing. Note that updateTask() already has 
> > logic to deal with duplicate terminal updates.

Also, needs a test!


- Vinod


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


On Sept. 2, 2015, 1:13 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 2, 2015, 1:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Re: Review Request 38051: Only update the task status when its old status is not terminal.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38051]

All tests passed.

- Mesos ReviewBot


On Sept. 2, 2015, 1:13 p.m., Yong Qiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38051/
> ---
> 
> (Updated Sept. 2, 2015, 1:13 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-2864
> https://issues.apache.org/jira/browse/MESOS-2864
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Only update the task status when its old status is not terminal.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 
> 
> Diff: https://reviews.apache.org/r/38051/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Yong Qiao Wang
> 
>

Review Request 38051: Only update the task status when its old status is not terminal.

[Yong Qiao Wang]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Only update the task status when its old status is not terminal.


Diffs
-

  src/master/master.cpp 56bcbcc08fa0f98416c5048080adb25efc588019 

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


Testing
---


Thanks,

Yong Qiao Wang