Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195350 --- Ship it! Will commit with a few touches like the following.

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195339 --- Ship it! LGTM. I still see open issues though, can you please

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/ --- (Updated Jan. 12, 2018, 7:24 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 12, 2018, 2:24 a.m., Vinod Kone wrote: > > AFAICT, in 1.4.x we show unreachable terminal tasks on a removed agent as > > completed tasks. But now, we show them as unreachable tasks. If that's true > > it's an API change that needs to be called out. Is that really backwards > >

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195241 --- LGTM modulo Vinod's comments. Sorry we have gone back and forth

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 11, 2018, 6:24 p.m., Vinod Kone wrote: > > AFAICT, in 1.4.x we show unreachable terminal tasks on a removed agent as > > completed tasks. But now, we show them as unreachable tasks. If that's true > > it's an API change that needs to be called out. Is that really backwards > >

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195282 --- AFAICT, in 1.4.x we show unreachable terminal tasks on a removed

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review195245 --- FAIL: Mesos tests failed to build. Reviews applied: `['64940']`

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 4, 2018, 5:25 p.m., Vinod Kone wrote: > > src/master/master.cpp > > Lines 10037-10056 (original), 10039-10062 (patched) > > > > > > I think we shouldn't create a TASK_UNREACHABLE status update and call

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review194998 --- src/master/master.cpp Line 10043 (original), 10038-10044

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review194999 --- FAIL: Some Mesos tests failed. Reviews applied: `['64940']`

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 5, 2018, 1:25 a.m., Vinod Kone wrote: > > src/master/master.cpp > > Lines 10037-10056 (original), 10039-10062 (patched) > > > > > > I think we shouldn't create a TASK_UNREACHABLE status update and call

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 4, 2018, 2:09 p.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 10061 (patched) > > > > > > Perhaps move all the logic around determining the unreachable task down > > here. > > > >

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 5, 2018, 1:25 a.m., Vinod Kone wrote: > > src/master/master.cpp > > Lines 10037-10056 (original), 10039-10062 (patched) > > > > > > I think we shouldn't create a TASK_UNREACHABLE status update and call

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 5, 2018, 1:25 a.m., Vinod Kone wrote: > > src/tests/master_tests.cpp > > Lines 7630 (patched) > > > > > > `the task`, which task? are both the tasks using the same executor? We need one task to finish

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/ --- (Updated Jan. 5, 2018, 7:06 p.m.) Review request for mesos, Benjamin Mahler,

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 4, 2018, 5:25 p.m., Vinod Kone wrote: > > src/master/master.cpp > > Lines 10037-10056 (original), 10039-10062 (patched) > > > > > > I think we shouldn't create a TASK_UNREACHABLE status update and call

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 5, 2018, 1:25 a.m., Vinod Kone wrote: > > src/master/master.cpp > > Lines 10037-10056 (original), 10039-10062 (patched) > > > > > > I think we shouldn't create a TASK_UNREACHABLE status update and call

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review194794 --- src/master/master.cpp Lines 10037-10056 (original), 10039-10062

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 4, 2018, 10:09 p.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 10061 (patched) > > > > > > Perhaps move all the logic around determining the unreachable task down > > here. > > > >

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 4, 2018, 10:09 p.m., Jiang Yan Xu wrote: > > src/tests/master_tests.cpp > > Lines 7637 (patched) > > > > > > You could just wait for the the status update (being droppped) instead > > of capaturing

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review194796 --- src/master/master.cpp Lines 10061 (patched)

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 4, 2018, 10:09 p.m., Jiang Yan Xu wrote: > > src/master/master.cpp > > Lines 10058-10060 (patched) > > > > > > Perhaps we just need to comment on the logic we use to determine if the > > task is

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

> On Jan. 4, 2018, 7:06 p.m., Ilya Pronin wrote: > > src/master/master.cpp > > Lines 10061 (patched) > > > > > > Nit: The expression can be simplified to `!terminal && unreachable`. I think it's a matter of taste

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review194777 --- src/master/master.cpp Lines 10058-10060 (patched)

Re: Review Request 64940: Prevented a crash when an agent with terminal tasks is lost.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64940/#review194772 --- Fix it, then Ship it! Looks good to me!