Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

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

(Updated Aug. 15, 2017, 6:17 p.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 


Diff: https://reviews.apache.org/r/61434/diff/3/

Changes: https://reviews.apache.org/r/61434/diff/2-3/


Testing
---

https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded

Additionally rendered in MacDown.


Thanks,

Alexander Rukletsov

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Lines 204 (patched)
> > 
> >
> > I think we also need to mention that the expectation here is that task 
> > is listening on the `loopback` interface along with any other routeable 
> > interface to which it might be listening.

Good idea.


> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Line 265 (original), 494 (patched)
> > 
> >
> > Why do we need to share the `mnt` namespace? This is already done by 
> > the executor for `MesosContainerizer` so why does the `checker` need to do 
> > this if it is running as part of the executor?

Nope, not for the command executor case when `rootfs` is specified.


- Alexander


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


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 9, 2017, 12:47 a.m., Gastón Kleiman wrote:
> > docs/health-checks.md
> > Lines 266 (patched)
> > 
> >
> > Don't the executors send an empty check status in this case? See 
> > `CommandExecutorCheckTest.CommandCheckTimeout`.
> 
> Alexander Rukletsov wrote:
> Correct. This is an artefact from health-checks-only times and should be 
> updated.

Actually no : ). I meant "no result" as "absence of the result", but I see how 
it is read differently.


- Alexander


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


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 9, 2017, 12:47 a.m., Gastón Kleiman wrote:
> > docs/health-checks.md
> > Lines 266 (patched)
> > 
> >
> > Don't the executors send an empty check status in this case? See 
> > `CommandExecutorCheckTest.CommandCheckTimeout`.

Correct. This is an artefact from health-checks-only times and should be 
updated.


- Alexander


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


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 11, 2017, 6:05 p.m., Greg Mann wrote:
> > docs/health-checks.md
> > Lines 272-278 (patched)
> > 
> >
> > Should we also call out here that setting interval_seconds to zero is a 
> > really bad idea?

Yes, good suggestion.


- Alexander


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


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 11, 2017, 6:05 p.m., Greg Mann wrote:
> > docs/health-checks.md
> > Lines 102 (patched)
> > 
> >
> > s/it/them/

:blush:


- Alexander


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


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Greg Mann]

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




docs/health-checks.md
Line 29 (original), 29 (patched)


s/defines/which defines a/



docs/health-checks.md
Lines 36 (patched)


s/task's/tasks'/



docs/health-checks.md
Lines 74 (patched)


s/certain condition/a certain condition/



docs/health-checks.md
Lines 75 (patched)


s/of specific/of a specific/



docs/health-checks.md
Lines 80-83 (patched)


I can't figure out precisely what is being said here; could you try to 
re-word?



docs/health-checks.md
Lines 85-86 (patched)


What do you mean by "only the executor knows how to interpret `TaskInfo`"?

I might replace with something like:

"It is the responsibility of the executor to interpret `CheckInfo` and 
`HealthCheckInfo` and perform checks appropriately."



docs/health-checks.md
Lines 86 (patched)


s/only executor/only the executor/



docs/health-checks.md
Lines 97 (patched)


s/checking all/checking that all/



docs/health-checks.md
Lines 102 (patched)


s/it/them/



docs/health-checks.md
Lines 109-112 (patched)


Could you make the punctuation at the ends of these lines consistent?



docs/health-checks.md
Lines 110-112 (patched)


Forgot to increment numbers: 2, 3



docs/health-checks.md
Lines 118 (patched)


s/internal task's/the task's internal/



docs/health-checks.md
Lines 135 (patched)


s/given HTTP/given that HTTP/



docs/health-checks.md
Lines 143 (patched)


s/in the future/in the future the/



docs/health-checks.md
Lines 157 (patched)


s/As/As the/



docs/health-checks.md
Lines 159 (patched)


s/as few as possible updates/as few updates as possible/



docs/health-checks.md
Lines 272-278 (patched)


Should we also call out here that setting interval_seconds to zero is a 
really bad idea?



docs/health-checks.md
Line 219 (original), 459 (patched)


s/during first/during the first/

s/response time/response times/



docs/health-checks.md
Lines 280-281 (original), 522-523 (patched)


I might recommend:

"Regardless of executor, all checks and health checks consume resources 
from the task's resource allocation."



docs/health-checks.md
Line 281 (original), 523 (patched)


s/towards task's/towards the task's/



docs/health-checks.md
Lines 539-541 (patched)


I might recommend:

"Due to its short-polling nature, a check whose state oscillates repeatedly 
may lead to scalability issues due to a high volume of task status updates."


- Greg Mann


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61434]

Logs available here: http://104.210.40.105/logs/master/61434

- Mesos Reviewbot Windows


On Aug. 11, 2017, 12:12 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 11, 2017, 12:12 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/2/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

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

(Updated Aug. 11, 2017, 12:12 a.m.)


Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.


Changes
---

Addressed some comments.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 


Diff: https://reviews.apache.org/r/61434/diff/2/

Changes: https://reviews.apache.org/r/61434/diff/1-2/


Testing
---

https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded

Additionally rendered in MacDown.


Thanks,

Alexander Rukletsov

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Line 29 (original), 29 (patched)
> > 
> >
> > s/defines/which defines?

It was intended as a simple enumeration.


> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Lines 208 (patched)
> > 
> >
> > how is the `host` resolved? It's not necessary that it will resolve to 
> > `127.0.0.1`?

Yes, we do not resolve anything. I'll reword to "set".


> On Aug. 9, 2017, 5:30 p.m., Avinash sridharan wrote:
> > docs/health-checks.md
> > Line 267 (original), 496 (patched)
> > 
> >
> > A bit confused here? I thought only `Health checks` are supported for 
> > `docker executor` and not `checks`?

Right, but health checks are using checks underneath.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 9, 2017, 12:47 a.m., Gastón Kleiman wrote:
> > docs/health-checks.md
> > Lines 136 (patched)
> > 
> >
> > Actually... only one status update is sent after a success, but 
> > failures are NOT deduplicated, see:
> > 
> > 
> > https://github.com/apache/mesos/blob/86e635b9f11e441bce3901a941c2d52b20518dbb/src/tests/health_check_tests.cpp#L990-L1076

Good catch!


> On Aug. 9, 2017, 12:47 a.m., Gastón Kleiman wrote:
> > docs/health-checks.md
> > Line 273 (original), 501 (patched)
> > 
> >
> > s/docker/Docker/

We agreed not to capitalize docker for docker containerizer and docker executor.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 4, 2017, 7:18 p.m., Vinod Kone wrote:
> > docs/health-checks.md
> > Lines 209 (patched)
> > 
> >
> > Is it 127.0.0.1 even in the CNI network case? cc @avinash

Yes, however, there is no resolution in this case happening.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Avinash sridharan]

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




docs/health-checks.md
Line 29 (original), 29 (patched)


s/defines/which defines?



docs/health-checks.md
Lines 162 (patched)


s/the following/to the following?



docs/health-checks.md
Lines 204 (patched)


I think we also need to mention that the expectation here is that task is 
listening on the `loopback` interface along with any other routeable interface 
to which it might be listening.



docs/health-checks.md
Lines 208 (patched)


how is the `host` resolved? It's not necessary that it will resolve to 
`127.0.0.1`?



docs/health-checks.md
Lines 232 (patched)


Ditto to the comments on the `HTTP Checks` section.



docs/health-checks.md
Line 265 (original), 494 (patched)


Why do we need to share the `mnt` namespace? This is already done by the 
executor for `MesosContainerizer` so why does the `checker` need to do this if 
it is running as part of the executor?



docs/health-checks.md
Line 267 (original), 496 (patched)


A bit confused here? I thought only `Health checks` are supported for 
`docker executor` and not `checks`?


- Avinash sridharan


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [61434]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Gastón Kleiman]

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




docs/health-checks.md
Lines 54-55 (original), 59-60 (patched)


I'd revert part of the changes: 

`s/if those are failing health checks/if the health checks fail/`



docs/health-checks.md
Lines 87 (patched)


s/except docker executor/except the Docker executor/



docs/health-checks.md
Lines 112 (patched)


s/unhealthy task/an unhealthy task/



docs/health-checks.md
Lines 114 (patched)


s/1+2+3: run the check/1+2+3: they run the check/



docs/health-checks.md
Lines 136 (patched)


Actually... only one status update is sent after a success, but failures 
are NOT deduplicated, see:


https://github.com/apache/mesos/blob/86e635b9f11e441bce3901a941c2d52b20518dbb/src/tests/health_check_tests.cpp#L990-L1076



docs/health-checks.md
Lines 160 (patched)


s/3rdparty/third party/



docs/health-checks.md
Lines 168-169 (patched)


s/check has not run yet, check has timed out/a has not run yet, or a check 
has timed out/



docs/health-checks.md
Lines 180 (patched)


s/`CheckInfo.Command` message/the `CheckInfo.Command` message/



docs/health-checks.md
Lines 194 (patched)


I'd add here (and in the following snippets):

```
TaskInfo task = [...];

```



docs/health-checks.md
Lines 266 (patched)


Don't the executors send an empty check status in this case? See 
`CommandExecutorCheckTest.CommandCheckTimeout`.



docs/health-checks.md
Line 273 (original), 501 (patched)


s/docker/Docker/


- Gastón Kleiman


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [61434]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

> On Aug. 4, 2017, 7:18 p.m., Vinod Kone wrote:
> > docs/health-checks.md
> > Lines 110 (patched)
> > 
> >
> > s/1./2./ ? or is this markdown style?

Yeah, actual numbers have no effect on the rendered HTML: 
https://daringfireball.net/projects/markdown/syntax#list
This approach is less error-prone to future additions, but I don't have a 
strong opinion.


- Alexander


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


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Vinod Kone]

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


Fix it, then Ship it!




This is great!


docs/health-checks.md
Lines 110 (patched)


s/1./2./ ? or is this markdown style?



docs/health-checks.md
Lines 118 (patched)


/transitions./transitions and make global decisions/



docs/health-checks.md
Lines 136 (patched)


I know you mentioned about check de-duplication above, but worthwhile 
mentioning it here as well to contrast.



docs/health-checks.md
Lines 138 (patched)


Docker executor currently supports health checks but not checks.



docs/health-checks.md
Lines 141 (patched)


s/reasons,/reasons;/



docs/health-checks.md
Lines 162 (patched)


s/adheres/adheres to/



docs/health-checks.md
Lines 209 (patched)


Is it 127.0.0.1 even in the CNI network case? cc @avinash


- Vinod Kone


On Aug. 4, 2017, 6:14 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61434/
> ---
> 
> (Updated Aug. 4, 2017, 6:14 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-7349
> https://issues.apache.org/jira/browse/MESOS-7349
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 
> 
> 
> Diff: https://reviews.apache.org/r/61434/diff/1/
> 
> 
> Testing
> ---
> 
> https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded
> 
> Additionally rendered in MacDown.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

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

(Updated Aug. 4, 2017, 6:09 p.m.)


Review request for mesos, Gastón Kleiman and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 


Diff: https://reviews.apache.org/r/61434/diff/1/


Testing
---

https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded

Additionally rendered in MacDown.


Thanks,

Alexander Rukletsov

Review Request 61434: Updated health-checks.md to include description of generalized checks.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  docs/health-checks.md d0749347d1f9398004e56b03503d655b5fba75f4 


Diff: https://reviews.apache.org/r/61434/diff/1/


Testing
---

https://gist.github.com/rukletsov/517a136cb6a0e8b909db30b36b13eded

Additionally rendered in MacDown.


Thanks,

Alexander Rukletsov