Re: Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Vinod Kone]

> On Feb. 6, 2016, 2:26 p.m., Klaus Ma wrote:
> > src/slave/http.cpp, line 402
> > 
> >
> > For GUI developer, it's better to export more info when it's 
> > recovering, e.g. build_time, build_date. So I'd like to export agent state 
> > in `/state`.

I would rather we tease out static information into separate endpoints. For 
example, version info (including build_time, build_date, git_sha etc) is 
already exposed in /version endpoint. Having partial information in an endpoint 
is more confusing.


- Vinod


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


On Feb. 5, 2016, 10:46 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43267/
> ---
> 
> (Updated Feb. 5, 2016, 10:46 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4066
> https://issues.apache.org/jira/browse/MESOS-4066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will help operators hitting the /state endpoint by not returning 
> incomplete
> data when slave is still recoverying.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
> 
> Diff: https://reviews.apache.org/r/43267/diff/
> 
> 
> Testing
> ---
> 
> make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"
> 
> ./bin/mesos-tests.sh 
> --gtest_filter="*StateEndpointUnavailableDuringRecovery*" --gtest_repeat=1000 
> --gtest_break_on_failure
> 
> ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Vinod Kone]

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

(Updated Feb. 9, 2016, 12:40 a.m.)


Review request for mesos, Anand Mazumdar and Ben Mahler.


Changes
---

addressed comments.


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


Repository: mesos


Description
---

This will help operators hitting the /state endpoint by not returning incomplete
data when slave is still recoverying.


Diffs (updated)
-

  src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
  src/tests/slave_tests.cpp 2a34d541eb8e2fc4802cacf38195460fe6094abc 

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


Testing
---

make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"

./bin/mesos-tests.sh --gtest_filter="*StateEndpointUnavailableDuringRecovery*" 
--gtest_repeat=1000 --gtest_break_on_failure

./bin/mesos-tests.sh


Thanks,

Vinod Kone

Re: Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Anand Mazumdar]

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


Ship it!




LGTM

- Anand Mazumdar


On Feb. 9, 2016, 12:40 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43267/
> ---
> 
> (Updated Feb. 9, 2016, 12:40 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4066
> https://issues.apache.org/jira/browse/MESOS-4066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will help operators hitting the /state endpoint by not returning 
> incomplete
> data when slave is still recoverying.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
>   src/tests/slave_tests.cpp 2a34d541eb8e2fc4802cacf38195460fe6094abc 
> 
> Diff: https://reviews.apache.org/r/43267/diff/
> 
> 
> Testing
> ---
> 
> make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"
> 
> ./bin/mesos-tests.sh 
> --gtest_filter="*StateEndpointUnavailableDuringRecovery*" --gtest_repeat=1000 
> --gtest_break_on_failure
> 
> ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Anand Mazumdar]

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



Looks good. Just a couple of minor issues.


src/tests/slave_tests.cpp (line 1339)


Can we just do:

`TestContainerizer containerizer1;` similar to what we do a few lines later 
in this test for `containerizer2` to be consistent?

Another option can be to modify both the occurences and wrap them in 
`process::Owned`. I would prefer the first option since we do so for all the 
other tests in this file too.



src/tests/slave_tests.cpp (line 1357)


Nit: I thought we decided to not do `Times(1)` explicitly in our test code?



src/tests/slave_tests.cpp (line 1397)


This expectation should be done before we invoke `StartSlave` to avoid a 
potential race.


- Anand Mazumdar


On Feb. 5, 2016, 10:46 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43267/
> ---
> 
> (Updated Feb. 5, 2016, 10:46 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4066
> https://issues.apache.org/jira/browse/MESOS-4066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will help operators hitting the /state endpoint by not returning 
> incomplete
> data when slave is still recoverying.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
> 
> Diff: https://reviews.apache.org/r/43267/diff/
> 
> 
> Testing
> ---
> 
> make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"
> 
> ./bin/mesos-tests.sh 
> --gtest_filter="*StateEndpointUnavailableDuringRecovery*" --gtest_repeat=1000 
> --gtest_break_on_failure
> 
> ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43267]

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

- Mesos ReviewBot


On Feb. 5, 2016, 10:46 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43267/
> ---
> 
> (Updated Feb. 5, 2016, 10:46 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4066
> https://issues.apache.org/jira/browse/MESOS-4066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will help operators hitting the /state endpoint by not returning 
> incomplete
> data when slave is still recoverying.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
> 
> Diff: https://reviews.apache.org/r/43267/diff/
> 
> 
> Testing
> ---
> 
> make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"
> 
> ./bin/mesos-tests.sh 
> --gtest_filter="*StateEndpointUnavailableDuringRecovery*" --gtest_repeat=1000 
> --gtest_break_on_failure
> 
> ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Klaus Ma]

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




src/slave/http.cpp (line 402)


For GUI developer, it's better to export more info when it's recovering, 
e.g. build_time, build_date. So I'd like to export agent state in `/state`.



src/tests/slave_tests.cpp (line 1405)


```
driver.stop()
driver.join()
```


- Klaus Ma


On Feb. 6, 2016, 6:46 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43267/
> ---
> 
> (Updated Feb. 6, 2016, 6:46 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4066
> https://issues.apache.org/jira/browse/MESOS-4066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will help operators hitting the /state endpoint by not returning 
> incomplete
> data when slave is still recoverying.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
> 
> Diff: https://reviews.apache.org/r/43267/diff/
> 
> 
> Testing
> ---
> 
> make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"
> 
> ./bin/mesos-tests.sh 
> --gtest_filter="*StateEndpointUnavailableDuringRecovery*" --gtest_repeat=1000 
> --gtest_break_on_failure
> 
> ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Vinod Kone]

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

Review request for mesos, Anand Mazumdar and Ben Mahler.


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


Repository: mesos


Description
---

This will help operators hitting the /state endpoint by not returning incomplete
data when slave is still recoverying.


Diffs
-

  src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
  src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 

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


Testing
---

make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"

./bin/mesos-tests.sh --gtest_filter="*StateEndpointUnavailableDuringRecovery*" 
--gtest_repeat=1000 --gtest_break_on_failure

./bin/mesos-tests.sh


Thanks,

Vinod Kone