Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-25 Thread Alexander Rukletsov 

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

(Updated Nov. 25, 2016, 11:49 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

To facilitate code reuse, HealthChecker::reschedule() is generalized.
This will become even more valuable when we add pause/resume functions.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-24 Thread Gastón Kleiman 

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


Ship it!




Ship It!

- Gastón Kleiman


On Nov. 23, 2016, 12:30 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Nov. 23, 2016, 12:30 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To facilitate code reuse, HealthChecker::reschedule() is generalized.
> This will become even more valuable when we add pause/resume functions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 837d1358e418d21536da488e4a23cbfa41db6060 
>   src/health-check/health_checker.cpp 
> af5500be249c74a4a5e64bf38dea607173e2f998 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-22 Thread Alexander Rukletsov 


> On Oct. 21, 2016, 7:35 p.m., Benjamin Mahler wrote:
> > src/health-check/health_checker.cpp, line 188
> > 
> >
> > Isn't this going to lead to some slightly confusing logging where we 
> > say "Rescheduling" for the first health check?
> 
> Alexander Rukletsov wrote:
> ```
> I 15:07:43.066488 1601536 health_checker.cpp:193] Health check 
> starting in 0ns, grace period 0ns
> I 15:07:43.066573 1601536 health_checker.cpp:618] Rescheduling health 
> check in 0ns
> W 15:07:43.158612 3747840 health_checker.cpp:215] Health check failed 
> 1 times consecutively: COMMAND health check failed: Command returned exited 
> with status 1
> I 15:07:43.158704 3747840 health_checker.cpp:618] Rescheduling health 
> check in 0ns
> ```
> I don't think it is very confusing, but if you have a strong opinion, 
> I'll change that.
> 
> Benjamin Mahler wrote:
> Could we ensure (separately from this change) that we log the entire 
> health check config when using vlog level 1?
> 
> Seems like it would be an easy improvement to go from 
> s/Rescheduling/Scheduling/ for every instance of this log line?

Posted https://reviews.apache.org/r/54007/


- Alexander


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


On Nov. 23, 2016, 12:30 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Nov. 23, 2016, 12:30 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To facilitate code reuse, HealthChecker::reschedule() is generalized.
> This will become even more valuable when we add pause/resume functions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 837d1358e418d21536da488e4a23cbfa41db6060 
>   src/health-check/health_checker.cpp 
> af5500be249c74a4a5e64bf38dea607173e2f998 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-22 Thread Alexander Rukletsov 

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

(Updated Nov. 23, 2016, 12:30 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

To facilitate code reuse, HealthChecker::reschedule() is generalized.
This will become even more valuable when we add pause/resume functions.


Diffs (updated)
-

  src/health-check/health_checker.hpp 837d1358e418d21536da488e4a23cbfa41db6060 
  src/health-check/health_checker.cpp af5500be249c74a4a5e64bf38dea607173e2f998 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-17 Thread Benjamin Mahler 

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


Ship it!





src/health-check/health_checker.hpp (line 122)


maybe call this `schedule()`?


- Benjamin Mahler


On Nov. 14, 2016, 10:35 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Nov. 14, 2016, 10:35 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To facilitate code reuse, HealthChecker::reschedule() is generalized.
> This will become even more valuable when we add pause/resume functions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> a1dc72493ff31b87068d5691f4d5b794392caf76 
>   src/health-check/health_checker.cpp 
> e2b32e2d57515202f547d12ba492ad8eb633641b 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-17 Thread Benjamin Mahler 


> On Oct. 21, 2016, 7:35 p.m., Benjamin Mahler wrote:
> > src/health-check/health_checker.cpp, line 188
> > 
> >
> > Isn't this going to lead to some slightly confusing logging where we 
> > say "Rescheduling" for the first health check?
> 
> Alexander Rukletsov wrote:
> ```
> I 15:07:43.066488 1601536 health_checker.cpp:193] Health check 
> starting in 0ns, grace period 0ns
> I 15:07:43.066573 1601536 health_checker.cpp:618] Rescheduling health 
> check in 0ns
> W 15:07:43.158612 3747840 health_checker.cpp:215] Health check failed 
> 1 times consecutively: COMMAND health check failed: Command returned exited 
> with status 1
> I 15:07:43.158704 3747840 health_checker.cpp:618] Rescheduling health 
> check in 0ns
> ```
> I don't think it is very confusing, but if you have a strong opinion, 
> I'll change that.

Could we ensure (separately from this change) that we log the entire health 
check config when using vlog level 1?

Seems like it would be an easy improvement to go from 
s/Rescheduling/Scheduling/ for every instance of this log line?


- Benjamin


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


On Nov. 14, 2016, 10:35 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Nov. 14, 2016, 10:35 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To facilitate code reuse, HealthChecker::reschedule() is generalized.
> This will become even more valuable when we add pause/resume functions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> a1dc72493ff31b87068d5691f4d5b794392caf76 
>   src/health-check/health_checker.cpp 
> e2b32e2d57515202f547d12ba492ad8eb633641b 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-15 Thread haosdent huang 

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


Ship it!




Ship It!

- haosdent huang


On Nov. 14, 2016, 10:35 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Nov. 14, 2016, 10:35 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To facilitate code reuse, HealthChecker::reschedule() is generalized.
> This will become even more valuable when we add pause/resume functions.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> a1dc72493ff31b87068d5691f4d5b794392caf76 
>   src/health-check/health_checker.cpp 
> e2b32e2d57515202f547d12ba492ad8eb633641b 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-14 Thread Alexander Rukletsov 

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

(Updated Nov. 14, 2016, 10:35 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description (updated)
---

To facilitate code reuse, HealthChecker::reschedule() is generalized.
This will become even more valuable when we add pause/resume functions.


Diffs
-

  src/health-check/health_checker.hpp a1dc72493ff31b87068d5691f4d5b794392caf76 
  src/health-check/health_checker.cpp e2b32e2d57515202f547d12ba492ad8eb633641b 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-14 Thread Alexander Rukletsov 

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

(Updated Nov. 14, 2016, 10:28 a.m.)


Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/health-check/health_checker.hpp a1dc72493ff31b87068d5691f4d5b794392caf76 
  src/health-check/health_checker.cpp e2b32e2d57515202f547d12ba492ad8eb633641b 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-11-11 Thread Alexander Rukletsov 


> On Oct. 21, 2016, 7:35 p.m., Benjamin Mahler wrote:
> > src/health-check/health_checker.cpp, line 188
> > 
> >
> > Isn't this going to lead to some slightly confusing logging where we 
> > say "Rescheduling" for the first health check?

```
I 15:07:43.066488 1601536 health_checker.cpp:193] Health check starting in 
0ns, grace period 0ns
I 15:07:43.066573 1601536 health_checker.cpp:618] Rescheduling health check 
in 0ns
W 15:07:43.158612 3747840 health_checker.cpp:215] Health check failed 1 
times consecutively: COMMAND health check failed: Command returned exited with 
status 1
I 15:07:43.158704 3747840 health_checker.cpp:618] Rescheduling health check 
in 0ns
```
I don't think it is very confusing, but if you have a strong opinion, I'll 
change that.


- Alexander


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


On Oct. 14, 2016, 12:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Oct. 14, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-10-21 Thread Benjamin Mahler 

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




src/health-check/health_checker.cpp (line 187)


Isn't this going to lead to some slightly confusing logging where we say 
"Rescheduling" for the first health check?


- Benjamin Mahler


On Oct. 14, 2016, 12:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Oct. 14, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-10-17 Thread haosdent huang 


> On Oct. 17, 2016, 7:03 a.m., haosdent huang wrote:
> > Is there any reason we try to avoid `check.interval_seconds()` in 
> > `reschedule`. Would be greate if could update the commit message about the 
> > motivation.
> 
> haosdent huang wrote:
> I feel it would be better to use https://reviews.apache.org/r/52867/ as 
> the reference for this patch.

Sorry for typo, should be https://reviews.apache.org/r/52868/.


- haosdent


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


On Oct. 14, 2016, 12:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Oct. 14, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-10-17 Thread haosdent huang 


> On Oct. 17, 2016, 7:03 a.m., haosdent huang wrote:
> > Is there any reason we try to avoid `check.interval_seconds()` in 
> > `reschedule`. Would be greate if could update the commit message about the 
> > motivation.

I feel it would be better to use https://reviews.apache.org/r/52867/ as the 
reference for this patch.


- haosdent


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


On Oct. 14, 2016, 12:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Oct. 14, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-10-17 Thread haosdent huang 

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



Is there any reason we try to avoid `check.interval_seconds()` in `reschedule`. 
Would be greate if could update the commit message about the motivation.

- haosdent huang


On Oct. 14, 2016, 12:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Oct. 14, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-10-14 Thread Gastón Kleiman 

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 14, 2016, 12:38 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52866/
> ---
> 
> (Updated Oct. 14, 2016, 12:38 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and haosdent huang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/health-check/health_checker.hpp 
> 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
> 
> Diff: https://reviews.apache.org/r/52866/diff/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/52873/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 52866: Refactored HealthChecker::reschedule to take duration as an argument.

2016-10-14 Thread Alexander Rukletsov 

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

Review request for mesos, Gastón Kleiman and haosdent huang.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/health-check/health_checker.hpp 392b4d5bd1e5831994b9366c1eb5a2911e19860f 
  src/health-check/health_checker.cpp 96ae1a733ff3d211b84d0893b4603873af1c89f0 

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


Testing
---

See https://reviews.apache.org/r/52873/.


Thanks,

Alexander Rukletsov