Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review151050 --- Ship it! Master (59b4d31) is green with this patch. ./build-s

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review151045 --- Ship it! Ship It! - Joshua Cohen On Sept. 30, 2016, 5:17 p.m

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 30, 2016, 5:17 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 30, 2016, 4:11 p.m., Joshua Cohen wrote: > > Can you add a test case for the scenario where a status provider throws > > during initialization? > > Kai Huang wrote: > Oh, that's a good point. Will do. Thanks. e2e's passed for me locally w/ this patch, so once the tests are added

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 30, 2016, 4:11 p.m., Joshua Cohen wrote: > > Can you add a test case for the scenario where a status provider throws > > during initialization? Oh, that's a good point. Will do. - Kai --- This is an automatically generated e

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review151029 --- Can you add a test case for the scenario where a status provider t

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150972 --- Ship it! Master (655105d) is green with this patch. ./build-s

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 28, 2016, 9:52 p.m., Joshua Cohen wrote: > > I tried to commit this, but e2e tests hung for me. > > > > Kai, can you investigate? > > Kai Huang wrote: > There is a bug of thermos kill for task running in docker > container(https://issues.apache.org/jira/browse/AURORA-1426 ). It

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 29, 2016, 11:11 p.m.) Review request for Aurora, Joshua Cohen an

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 28, 2016, 9:52 p.m., Joshua Cohen wrote: > > I tried to commit this, but e2e tests hung for me. > > > > Kai, can you investigate? There is a bug of thermos kill for task running in docker container(https://issues.apache.org/jira/browse/AURORA-1426 ). It is likely due to my code cha

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150769 --- I tried to commit this, but e2e tests hung for me. Kai, can you i

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150764 --- Ship it! Master (1c1c0a5) is green with this patch. ./build-s

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 28, 2016, 9:07 p.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150644 --- Ship it! Master (69cba78) is green with this patch. ./build-s

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 27, 2016, 10:25 p.m.) Review request for Aurora, Joshua Cohen, M

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 27, 2016, 9:27 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/executor/common/status_checker.py, line 104 > > > > > > This is slightly beyond the pull request, but for the matter of > > genera

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150612 --- Ship it! LGTM. A couple of minor remarks below. I am late to

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150628 --- Ship it! lgtm overall, thanks for iterating! Modulo the below

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150428 --- src/main/python/apache/aurora/executor/common/health_checker.py (

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 23, 2016, 3:18 p.m., Zameer Manji wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, line 120 > > > > > > This check is brittle to determine if health checking is enabled. > > Please co

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150427 --- Ship it! Ship It! - Zameer Manji On Sept. 23, 2016, 11:58 a.

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 23, 2016, 10:18 p.m., Zameer Manji wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, line 120 > > > > > > This check is brittle to determine if health checking is enabled. > > Please c

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150246 --- I like the additional testing done. src/main/python/apache/auror

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150226 --- Ship it! Master (4ead189) is green with this patch. ./build-s

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 23, 2016, 6:58 p.m.) Review request for Aurora, Joshua Cohen, Ma

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 23, 2016, 4:53 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, line 81 > > > > > > I don't think we should expose this simply for the sake of testing, > > besides b

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150200 --- src/main/python/apache/aurora/executor/aurora_executor.py (line 8

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150117 --- Ship it! Master (4ead189) is green with this patch. ./build-s

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review150115 --- src/main/python/apache/aurora/executor/aurora_executor.py (line 1

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 23, 2016, 12:57 a.m.) Review request for Aurora, Joshua Cohen, M

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 5:34 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, lines > > 279-283 > > > > > > This logic needs to be refactored to reduce duplication and

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 5:34 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, line 149 > > > > > > leftover? will remove it. - Kai

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 5:34 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, lines 184-185 > > > > > > Are there any guarantees the status_manager will always deal with > >

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 5:34 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/aurora_executor.py, lines 184-185 > > > > > > Are there any guarantees the status_manager will always deal with > >

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 5:34 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, lines > > 162-164 > > > > > > This should be moved into the `_maybe_update_failure_count(

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 6:25 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/common/health_checker.py, line 135 > > > > > > Should this be a call to the `initial_in_progress` function? If so, > > t

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review149859 --- src/main/python/apache/aurora/executor/aurora_executor.py (lines

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 5:34 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/executor/status_manager.py, line 68 > > > > > > This is exactly the problem I alluded to above. What if there is no > > entry

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 21, 2016, 5:34 p.m., Maxim Khutornenko wrote: > > api/src/main/thrift/org/apache/aurora/gen/api.thrift, lines 1218-1220 > > > > > > What's the motivation behind moving these constants into the thrift > >

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review149864 --- src/main/python/apache/aurora/executor/common/health_checker.py (

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

> On Sept. 20, 2016, 12:28 a.m., Kai Huang wrote: > > src/main/python/apache/aurora/executor/common/status_checker.py, line 104 > > > > > > Use TaskState.Value('TASK_RUNNING') here instead of > > mesos_pb2.TASK_RUNN

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review149846 --- api/src/main/thrift/org/apache/aurora/gen/api.thrift (lines 1218

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review149588 --- Ship it! Master (4c4040f) is green with this patch. ./build-s

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review149581 --- src/main/python/apache/aurora/executor/common/status_checker.py (

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 20, 2016, 12:25 a.m.) Review request for Aurora, Joshua Cohen, M

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/#review148834 --- Master (5069f93) is red with this patch. ./build-support/jenkins

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 14, 2016, 12:47 a.m.) Review request for Aurora, Joshua Cohen, M

Re: Review Request 51876: Modify executor state transition logic to rely on health checks (if enabled)

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51876/ --- (Updated Sept. 14, 2016, 12:40 a.m.) Review request for Aurora, Joshua Cohen, M