Re: Review Request 62609: Protect against Future callbacks deleting instance.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62609 was successfully built and tested.

Reviews applied: `['62609']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62609

- Mesos Reviewbot Windows


On Sept. 27, 2017, 3:16 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62609/
> ---
> 
> (Updated Sept. 27, 2017, 3:16 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8010
> https://issues.apache.org/jira/browse/MESOS-8010
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-8010.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> a11a588941b02d776da2f563dd246a92dfbbc360 
> 
> 
> Diff: https://reviews.apache.org/r/62609/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 62609: Protect against Future callbacks deleting instance.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Sept. 27, 2017, 3:16 a.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62609/
> ---
> 
> (Updated Sept. 27, 2017, 3:16 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8010
> https://issues.apache.org/jira/browse/MESOS-8010
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-8010.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/future.hpp 
> a11a588941b02d776da2f563dd246a92dfbbc360 
> 
> 
> Diff: https://reviews.apache.org/r/62609/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Review Request 62609: Protect against Future callbacks deleting instance.

[Benjamin Hindman]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

See MESOS-8010.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
a11a588941b02d776da2f563dd246a92dfbbc360 


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


Testing
---

make check


Thanks,

Benjamin Hindman

Re: Review Request 62595: Added test module wrapping the basic HTTP authenticatee.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62595 was successfully built and tested.

Reviews applied: `['62587', '62591', '62592', '62594', '62595']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62595

- Mesos Reviewbot Windows


On Sept. 26, 2017, 10:54 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62595/
> ---
> 
> (Updated Sept. 26, 2017, 10:54 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Armand Grillet, Benjamin Bannier, 
> Greg Mann, Kapil Arya, and Vinod Kone.
> 
> 
> Bugs: MESOS-8017
> https://issues.apache.org/jira/browse/MESOS-8017
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds test module wrapping the basic HTTP authenticatee. Additionally
> renames the former testhttpauthenticator library to accomodate for
> the added test HTTP authenticatee module.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/examples/CMakeLists.txt d4f1af4f072efdc68fa0b722f42b1d8aa1779b6e 
>   src/examples/test_http_authenticator_module.cpp 
> a2fdc282e741f36a429923a7d6082750a89b7b13 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/module.cpp 5b83fd6358ddea4c9d849b8992e1a6040ef74505 
> 
> 
> Diff: https://reviews.apache.org/r/62595/diff/1/
> 
> 
> Testing
> ---
> 
> Integration test in external project. Further tests upcoming.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 62230: Avoid GC pruning events from blocking other processes.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On Sept. 26, 2017, 3:04 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62230/
> ---
> 
> (Updated Sept. 26, 2017, 3:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7964
> https://issues.apache.org/jira/browse/MESOS-7964
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch dispatches `rmdirs` to a single executor instead of multiple
> `AsyncExecutor`s such that heavy-duty pruning events won't occupy all
> worker threads and block other actors.
> 
> 
> Diffs
> -
> 
>   src/slave/gc.cpp 83e4e2f3aba5c0d9900cf0beeea6e92320f889e7 
>   src/slave/gc_process.hpp c383ce28411622692e42401c80e9443e7b1f5099 
> 
> 
> Diff: https://reviews.apache.org/r/62230/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 62592: Added basic HTTP authenticatee implementation.

[Till Toenshoff]

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

Review request for mesos, Armand Grillet, Benjamin Bannier, Greg Mann, and 
Kapil Arya.


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


Repository: mesos


Description
---

Added basic HTTP authenticatee implementation.


Diffs
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/authentication/http/basic_authenticatee.hpp PRE-CREATION 
  src/authentication/http/basic_authenticatee.cpp PRE-CREATION 
  src/common/http.hpp 0e6b1c59860e75c04e2e3be2520ed2b39c84ac90 


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


Testing
---

Integration test in external project. Further tests upcoming.


Thanks,

Till Toenshoff

Review Request 62595: Added test module wrapping the basic HTTP authenticatee.

[Till Toenshoff]

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

Review request for mesos, Anand Mazumdar, Armand Grillet, Benjamin Bannier, 
Greg Mann, Kapil Arya, and Vinod Kone.


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


Repository: mesos


Description
---

Adds test module wrapping the basic HTTP authenticatee. Additionally
renames the former testhttpauthenticator library to accomodate for
the added test HTTP authenticatee module.


Diffs
-

  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/examples/CMakeLists.txt d4f1af4f072efdc68fa0b722f42b1d8aa1779b6e 
  src/examples/test_http_authenticator_module.cpp 
a2fdc282e741f36a429923a7d6082750a89b7b13 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/module.cpp 5b83fd6358ddea4c9d849b8992e1a6040ef74505 


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


Testing
---

Integration test in external project. Further tests upcoming.


Thanks,

Till Toenshoff

Review Request 62587: Added HTTP authenticatee interface definition.

[Till Toenshoff]

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

Review request for mesos, Armand Grillet, Benjamin Bannier, Greg Mann, and 
Kapil Arya.


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


Repository: mesos


Description
---

see summary.


Diffs
-

  include/mesos/authentication/http/authenticatee.hpp PRE-CREATION 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 


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


Testing
---

Integration test in external project. Further tests upcoming.


Thanks,

Till Toenshoff

Review Request 62591: Modularized HTTP authenticatee.

[Till Toenshoff]

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

Review request for mesos, Armand Grillet, Benjamin Bannier, Greg Mann, and 
Kapil Arya.


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


Repository: mesos


Description
---

see summary.


Diffs
-

  include/mesos/module/http_authenticatee.hpp PRE-CREATION 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/module/manager.cpp ec5614a6d2e9555fd8c8e35309ef05ad5c312638 


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


Testing
---

Integration test in external project. Further tests upcoming.


Thanks,

Till Toenshoff

Review Request 62594: Updated scheduler library for modularized HTTP authenticatee use.

[Till Toenshoff]

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

Review request for mesos, Anand Mazumdar, Armand Grillet, Benjamin Bannier, 
Greg Mann, Kapil Arya, and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/scheduler/flags.hpp 1e8efc06b40b17d6fa9d9516a83893485804fc72 
  src/scheduler/scheduler.cpp 78f53707364ab988bcc53ec2c95490df04cd9a6c 


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


Testing
---

Integration test in external project. Further tests upcoming.


Thanks,

Till Toenshoff

Re: Review Request 60511: Added MockMetadataManager and test concurrent prune and pull.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 60511 was successfully built and tested.

Reviews applied: `['55334', '55335', '59687', '56721', '60471', '56722', 
'60511']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/60511

- Mesos Reviewbot Windows


On Aug. 7, 2017, 9:49 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60511/
> ---
> 
> (Updated Aug. 7, 2017, 9:49 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added MockMetadataManager and test concurrent prune and pull.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1cf68665d33bd40a7605d26c96fb7b618407fdd0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> f357710cb19aec3654b0604f7909d068eaf20095 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 920be77b16178a4458d72145020c015130799ec4 
> 
> 
> Diff: https://reviews.apache.org/r/60511/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62475', '62476', '62477', '62581', '62478', '62479', 
'62480', '62481', '62507', '62531', '62554']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose 
--gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554/logs/mesos-tests-stdout.log):

```
[   OK ] FilesTest.DetachTest (5 ms)
[ RUN  ] FilesTest.ReadTest
[   OK ] FilesTest.ReadTest (292 ms)
[ RUN  ] FilesTest.DownloadTest
[   OK ] FilesTest.DownloadTest (126 ms)
[ RUN  ] FilesTest.DebugTest
[   OK ] FilesTest.DebugTest (123 ms)
[ RUN  ] FilesTest.AuthenticationTest
[   OK ] FilesTest.AuthenticationTest (99 ms)
[--] 6 tests from FilesTest (807 ms total)

[--] 3 tests from GarbageCollectorTest
[ RUN  ] GarbageCollectorTest.Schedule
[   OK ] GarbageCollectorTest.Schedule (123 ms)
[ RUN  ] GarbageCollectorTest.Unschedule
[   OK ] GarbageCollectorTest.Unschedule (36 ms)
[ RUN  ] GarbageCollectorTest.Prune
[   OK ] GarbageCollectorTest.Prune (37 ms)
[--] 3 tests from GarbageCollectorTest (299 ms total)

[--] 5 tests from GarbageCollectorIntegrationTest
[ RUN  ] GarbageCollectorIntegrationTest.Restart

C:\mesos\mesos\src\tests\gc_tests.cpp(332): ERROR: this mock object (used in 
test GarbageCollectorIntegrationTest.Restart) should be deleted but never is. 
Its address is @00752FB59910.
C:\mesos\mesos\src\tests\containerizer.cpp(414): ERROR: this mock object (used 
in test GarbageCollectorIntegrationTest.Restart) should be deleted but never 
is. Its address is @00752FB59DE0.
C:\mesos\mesos\src\tests\gc_tests.cpp(317): ERROR: this mock object (used in 
test GarbageCollectorIntegrationTest.Restart) should be deleted but never is. 
Its address is @00752FB5B370.
C:\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object (used 
in test GarbageCollectorIntegrationTest.Restart) should be deleted but never 
is. Its address is @01E405FCC520.
C:\mesos\mesos\3rdparty\libprocess\include\process/gmock.hpp(235): ERROR: this 
mock object (used in test GarbageCollectorIntegrationTest.Restart) should be 
deleted but never is. Its address is @01E406498898.
ERROR: 5 leaked mock objects found at program exit.
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554/logs/mesos-tests-stderr.log):

```
  scalar {
value: 1024
  }
}
resources {
  name: "disk"
  type: SCALAR
  scalar {
value: 1024
  }
}
resources {
  name: "ports"
  type: RANGES
  ranges {
range {
  begin: 31000
  end: 32000
}
  }
}
checkpoint: true
port: 56595


To remedy this do as follows:
Step 1: rm -f C:\Users\mesos\AppData\Local\Temp\2\ppnykI\meta\slaves\latest
This ensures agent doesn't recover old live executors.
Step 2: Restart the agent.
```

- Mesos Reviewbot Windows


On Sept. 26, 2017, 8:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62554/
> ---
> 
> (Updated Sept. 26, 2017, 8:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7448
> https://issues.apache.org/jira/browse/MESOS-7448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces two master options `registry_max_gone_agent_age`
> (age based) and `registry_max_gone_agent_count` (count based) for
> GC'ing the list of gone agents in the registry.
> 
> Review: https://reviews.apache.org/r/62554
> 
> 
> Diffs
> -
> 
>   src/master/flags.cpp fa6d274cb52448269a8108ffdc5f07bad8b222f2 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/tests/master_tests.cpp 98908c00c2d5bfd08122fd317a2ce95332d9583d 
>   src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
>   src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 
> 
> 
> Diff: https://reviews.apache.org/r/62554/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62230: Avoid GC pruning events from blocking other processes.

[Chun-Hung Hsiao]

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

(Updated Sept. 26, 2017, 10:04 p.m.)


Review request for mesos, Benjamin Mahler, Jie Yu, and Jiang Yan Xu.


Changes
---

Addressed xujyan's comments.


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


Repository: mesos


Description
---

This patch dispatches `rmdirs` to a single executor instead of multiple
`AsyncExecutor`s such that heavy-duty pruning events won't occupy all
worker threads and block other actors.


Diffs (updated)
-

  src/slave/gc.cpp 83e4e2f3aba5c0d9900cf0beeea6e92320f889e7 
  src/slave/gc_process.hpp c383ce28411622692e42401c80e9443e7b1f5099 


Diff: https://reviews.apache.org/r/62230/diff/4/

Changes: https://reviews.apache.org/r/62230/diff/3-4/


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 62481: Fixed tests impacted by no longer removing the agent symlink.

[Anand Mazumdar]

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

(Updated Sept. 26, 2017, 8:45 p.m.)


Review request for .


Changes
---

Review comments.


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


Repository: mesos


Description
---

This change fixes tests that relied on the agent work directory
symlink being removed when the agent receives the shutdown message
from the master.

Review: https://reviews.apache.org/r/62481


Diffs (updated)
-

  src/tests/gc_tests.cpp 5581861e8ff834b4e79a51bef5f28aac7e7d259d 
  src/tests/slave_recovery_tests.cpp 0cd2b5d37e35ccc2fa2c14db750d1314238bc312 
  src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 62481: Fixed tests impacted by no longer removing the agent symlink.

[Anand Mazumdar]

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

(Updated Sept. 26, 2017, 8:45 p.m.)


Review request for .


Changes
---

Review comments.


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


Repository: mesos


Description
---

This change fixes tests that relied on the agent work directory
symlink being removed when the agent receives the shutdown message
from the master.

Review: https://reviews.apache.org/r/62481


Diffs (updated)
-

  src/tests/gc_tests.cpp 5581861e8ff834b4e79a51bef5f28aac7e7d259d 
  src/tests/slave_recovery_tests.cpp 0cd2b5d37e35ccc2fa2c14db750d1314238bc312 
  src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 26, 2017, 8:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62554/
> ---
> 
> (Updated Sept. 26, 2017, 8:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7448
> https://issues.apache.org/jira/browse/MESOS-7448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces two master options `registry_max_gone_agent_age`
> (age based) and `registry_max_gone_agent_count` (count based) for
> GC'ing the list of gone agents in the registry.
> 
> Review: https://reviews.apache.org/r/62554
> 
> 
> Diffs
> -
> 
>   src/master/flags.cpp fa6d274cb52448269a8108ffdc5f07bad8b222f2 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/tests/master_tests.cpp 98908c00c2d5bfd08122fd317a2ce95332d9583d 
>   src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
>   src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 
> 
> 
> Diff: https://reviews.apache.org/r/62554/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Anand Mazumdar]

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

(Updated Sept. 26, 2017, 8:30 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fix a flags.cpp comment.


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


Repository: mesos


Description (updated)
---

This change introduces two master options `registry_max_gone_agent_age`
(age based) and `registry_max_gone_agent_count` (count based) for
GC'ing the list of gone agents in the registry.

Review: https://reviews.apache.org/r/62554


Diffs (updated)
-

  src/master/flags.cpp fa6d274cb52448269a8108ffdc5f07bad8b222f2 
  src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
  src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
  src/tests/master_tests.cpp 98908c00c2d5bfd08122fd317a2ce95332d9583d 
  src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
  src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 


Diff: https://reviews.apache.org/r/62554/diff/5/

Changes: https://reviews.apache.org/r/62554/diff/4-5/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 62477: Added the gone agents to the master registry.

[Anand Mazumdar]

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

(Updated Sept. 26, 2017, 8:25 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments.


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


Repository: mesos


Description (updated)
---

This change adds the list of gone agents to the registry and also
introduces the `MarkSlaveGone` operation on the Mesos master. This
would be used by the Master Operator API handler to insert an agent
into the list of gone agent.


Diffs (updated)
-

  src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 


Diff: https://reviews.apache.org/r/62477/diff/4/

Changes: https://reviews.apache.org/r/62477/diff/3-4/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Anand Mazumdar]

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

(Updated Sept. 26, 2017, 8:23 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments.


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


Repository: mesos


Description (updated)
---

This change introduces two master options `registry_max_gone_agent_age`
(age based) and `registry_max_gone_agent_count` (count based) for
GC'ing the list of gone agents in the registry.

Review: https://reviews.apache.org/r/62554


Diffs (updated)
-

  src/master/flags.cpp fa6d274cb52448269a8108ffdc5f07bad8b222f2 
  src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
  src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
  src/tests/master_tests.cpp 98908c00c2d5bfd08122fd317a2ce95332d9583d 
  src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
  src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 


Diff: https://reviews.apache.org/r/62554/diff/4/

Changes: https://reviews.apache.org/r/62554/diff/3-4/


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 55335: Backfilled required fields in TaskInfo in MesosContainerizer* tests.

[Zhitao Li]

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

(Updated Sept. 26, 2017, 8:14 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Fix scope of the common function.


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


Repository: mesos


Description
---

These tests were using a `TaskInfo` field which is missing required
protobuf fields. After the previous patch begins to checkpoint
`ContainerConfig`, the incomplete `TaskInfo` cannot be checkpointed,
thus caused these tests to fail.

This patch backfills these fields to make these tests pass.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
e61a85df6ec5308ccd2832e66df803b0ad7b53ee 


Diff: https://reviews.apache.org/r/55335/diff/6/

Changes: https://reviews.apache.org/r/55335/diff/5-6/


Testing
---

GTEST_FILTER="MesosContainerizer*" make check


Thanks,

Zhitao Li

Re: Review Request 62579: [WIP] Added a test `BlkioIsolatorTest.ROOT_BlkioUsage`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62579 was successfully built and tested.

Reviews applied: `['62579']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62579

- Mesos Reviewbot Windows


On Sept. 26, 2017, 4:33 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62579/
> ---
> 
> (Updated Sept. 26, 2017, 4:33 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8013
> https://issues.apache.org/jira/browse/MESOS-8013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `BlkioIsolatorTest.ROOT_BlkioUsage`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
>   src/tests/containerizer/blkio_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62579/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

(Updated Sept. 26, 2017, 7:09 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Added `CHECK_SOME`


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
cc23b4d91be16fc95a131c09d07378b801e34d6f 
  src/slave/containerizer/mesos/containerizer.cpp 
4d5dc13f363f5d8886983d7dd06a5cecc177c345 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/9/

Changes: https://reviews.apache.org/r/55334/diff/8-9/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Vinod Kone]

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




src/master/flags.cpp
Line 603 (original), 603 (patched)


can you just say "partitioned/removed/gone" as you did above?



src/tests/registrar_tests.cpp
Line 455 (original), 456 (patched)


why did this change?


- Vinod Kone


On Sept. 25, 2017, 11:39 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62554/
> ---
> 
> (Updated Sept. 25, 2017, 11:39 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7448
> https://issues.apache.org/jira/browse/MESOS-7448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces two master options `registry_max_gone_agent_age`
> (age based) and `registry_max_gone_agent_count` (count based) for
> GC'ing the list of gone agents in the registry.
> 
> 
> Diffs
> -
> 
>   src/master/flags.cpp fa6d274cb52448269a8108ffdc5f07bad8b222f2 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/tests/master_tests.cpp 98908c00c2d5bfd08122fd317a2ce95332d9583d 
>   src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
>   src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 
> 
> 
> Diff: https://reviews.apache.org/r/62554/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62479: Removed the logic for removing the latest symlink on the agent.

[Vinod Kone]

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



Can you also add a blurb to CHANGELOG about the new semantics? I think it's 
worth calling out.

- Vinod Kone


On Sept. 21, 2017, 8:28 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62479/
> ---
> 
> (Updated Sept. 21, 2017, 8:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change removes the logic of removing the latest symlink on
> receiving the shutdown message from the Mesos master. This ensures
> that agents come back with the same agent ID upon a successful
> shutdown similar to the behavior when they come back post a reboot.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 01020a624bdd67ee6371f607d897ad71a2cdcc82 
> 
> 
> Diff: https://reviews.apache.org/r/62479/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62481: Fixed tests impacted by no longer removing the agent symlink.

[Vinod Kone]

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




src/tests/gc_tests.cpp
Line 381 (original)


I think we should wait until `__recover` gets executed to be sure that no 
gc has been scheduled.


- Vinod Kone


On Sept. 26, 2017, 4:59 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62481/
> ---
> 
> (Updated Sept. 26, 2017, 4:59 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change fixes tests that relied on the agent work directory
> symlink being removed when the agent receives the shutdown message
> from the master.
> 
> Review: https://reviews.apache.org/r/62481
> 
> 
> Diffs
> -
> 
>   src/tests/gc_tests.cpp 5581861e8ff834b4e79a51bef5f28aac7e7d259d 
>   src/tests/slave_recovery_tests.cpp 0cd2b5d37e35ccc2fa2c14db750d1314238bc312 
>   src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 
> 
> 
> Diff: https://reviews.apache.org/r/62481/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62479: Removed the logic for removing the latest symlink on the agent.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 21, 2017, 8:28 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62479/
> ---
> 
> (Updated Sept. 21, 2017, 8:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change removes the logic of removing the latest symlink on
> receiving the shutdown message from the Mesos master. This ensures
> that agents come back with the same agent ID upon a successful
> shutdown similar to the behavior when they come back post a reboot.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 01020a624bdd67ee6371f607d897ad71a2cdcc82 
> 
> 
> Diff: https://reviews.apache.org/r/62479/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62478: Added the mark agent gone handler on the master.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 21, 2017, 8:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62478/
> ---
> 
> (Updated Sept. 21, 2017, 8:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the neccessary logic for handling the mark agent
> gone call on the master. Once an agent is marked as gone, it's
> not allowed to re-register with the Mesos master. GC'ing the
> list of gone agents (it can grow unbounded) would be added in
> a separate change.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 28d0393fb5962df4d731521265efd81a54e1e655 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/master/validation.cpp a6e6a90af7d8d1242e28e93af551a2096db62939 
> 
> 
> Diff: https://reviews.apache.org/r/62478/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62581: Added `__removeSlave` function and made `_markUnreachable` use it.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 26, 2017, 6:05 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62581/
> ---
> 
> (Updated Sept. 26, 2017, 6:05 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This would also be used by the `markGone()` function in the next
> review to remove agents.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
> 
> 
> Diff: https://reviews.apache.org/r/62581/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62477: Added the gone agents to the master registry.

[Vinod Kone]

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




src/tests/registrar_tests.cpp
Lines 384-385 (patched)


this should succeed right?


- Vinod Kone


On Sept. 21, 2017, 8:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62477/
> ---
> 
> (Updated Sept. 21, 2017, 8:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7444
> https://issues.apache.org/jira/browse/MESOS-7444
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the list of gone agents to the registry and also
> introduces the `MarkSlaveGone` operation on the Mesos master. This
> would be used by the Master Operator API handler to insert an agent
> into the list of gone agent.
> 
> Review: https://reviews.apache.org/r/62477
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 
> 
> 
> Diff: https://reviews.apache.org/r/62477/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62477: Added the gone agents to the master registry.

[Vinod Kone]

> On Sept. 26, 2017, 12:02 a.m., Vinod Kone wrote:
> > src/master/registry.proto
> > Lines 60 (patched)
> > 
> >
> > s/master/operator/ ?
> 
> Anand Mazumdar wrote:
> hmm, not sure about this. The operator inititates the gone operation but 
> the master uses its own 'clock' to set the gone time.

I guess you mention "operator" in the comment above `gone` so that's ok. I was 
mainly concerned because this sentence almost makes it look like it's the 
master that is marking it gone even though it only relates to time.


- Vinod


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


On Sept. 21, 2017, 8:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62477/
> ---
> 
> (Updated Sept. 21, 2017, 8:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7444
> https://issues.apache.org/jira/browse/MESOS-7444
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the list of gone agents to the registry and also
> introduces the `MarkSlaveGone` operation on the Mesos master. This
> would be used by the Master Operator API handler to insert an agent
> into the list of gone agent.
> 
> Review: https://reviews.apache.org/r/62477
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 
> 
> 
> Diff: https://reviews.apache.org/r/62477/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62475: Added `MARK_AGENT_GONE` call to the v1 Master API.

[Vinod Kone]

> On Sept. 25, 2017, 11:41 p.m., Vinod Kone wrote:
> > include/mesos/master/master.proto
> > Lines 205-206 (patched)
> > 
> >
> > The last sentence is not strictly true right? A new agent with a new id 
> > but old volumes/reservations can still come back right? If so, we should 
> > mention that possibility.
> 
> Anand Mazumdar wrote:
> hmm, thats possible. The reason I did not mention it was that it's not 
> possible without the operator manually tinkering and removing the `latest` 
> symlink. What do you think?

I see. ok.


- Vinod


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


On Sept. 24, 2017, 5:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62475/
> ---
> 
> (Updated Sept. 24, 2017, 5:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7443
> https://issues.apache.org/jira/browse/MESOS-7443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the `MARK_AGENT_GONE` call that can be
> used by operators to assert that a given agent has failed. It
> is specially useful for stateful frameworks to ascertain whether
> its safe to move the workload to a new agent.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto b94e90287982e620749c10bec77cf0af10318415 
>   include/mesos/v1/master/master.proto 
> 7499fa4f62ab18dd3cd4827461717bc9c688dc49 
> 
> 
> Diff: https://reviews.apache.org/r/62475/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62554 was successfully built and tested.

Reviews applied: `['62475', '62476', '62477', '62478', '62479', '62480', 
'62481', '62507', '62531', '62554']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554

- Mesos Reviewbot Windows


On Sept. 25, 2017, 11:39 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62554/
> ---
> 
> (Updated Sept. 25, 2017, 11:39 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7448
> https://issues.apache.org/jira/browse/MESOS-7448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces two master options `registry_max_gone_agent_age`
> (age based) and `registry_max_gone_agent_count` (count based) for
> GC'ing the list of gone agents in the registry.
> 
> 
> Diffs
> -
> 
>   src/master/flags.cpp fa6d274cb52448269a8108ffdc5f07bad8b222f2 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/tests/master_tests.cpp 98908c00c2d5bfd08122fd317a2ce95332d9583d 
>   src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
>   src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 
> 
> 
> Diff: https://reviews.apache.org/r/62554/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

[Zhitao Li]

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

(Updated Sept. 26, 2017, 6:15 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.


Diffs (updated)
-

  src/slave/containerizer/composing.hpp 
06d68eef5de7745e32f0e808f11016bcc285dd8f 
  src/slave/containerizer/composing.cpp 
587f009384f0c7ef87482686578dc822d3d5b208 
  src/slave/containerizer/containerizer.hpp 
449bb5d0902936faae7bf9bae9c703b219aed842 
  src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
  src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
  src/slave/containerizer/mesos/containerizer.hpp 
cc23b4d91be16fc95a131c09d07378b801e34d6f 
  src/slave/containerizer/mesos/containerizer.cpp 
4d5dc13f363f5d8886983d7dd06a5cecc177c345 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
1cf68665d33bd40a7605d26c96fb7b618407fdd0 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
f357710cb19aec3654b0604f7909d068eaf20095 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 
  src/slave/containerizer/mesos/provisioner/store.hpp 
01ab83dca79e51b8c96d18ee65705912b0ac8324 
  src/slave/containerizer/mesos/provisioner/store.cpp 
cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
  src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac 
  src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a 
  src/tests/containerizer/mock_containerizer.hpp 
0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 


Diff: https://reviews.apache.org/r/56721/diff/8/

Changes: https://reviews.apache.org/r/56721/diff/7-8/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 59687: Added tests for recovering ContainerConfig.

[Zhitao Li]

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

(Updated Sept. 26, 2017, 6:14 p.m.)


Review request for mesos, Gilbert Song and Jason Lai.


Changes
---

Rebase and remove whitespace at the end of the line.


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


Repository: mesos


Description
---

Added tests for recovering ContainerConfig.


Diffs (updated)
-

  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
f8b4423de8a468501336acc5ee0c67f181dc65f5 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 55335: Backfilled required fields in TaskInfo in MesosContainerizer* tests.

[Zhitao Li]

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

(Updated Sept. 26, 2017, 6:13 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase and create helper function to address duplicate.


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


Repository: mesos


Description
---

These tests were using a `TaskInfo` field which is missing required
protobuf fields. After the previous patch begins to checkpoint
`ContainerConfig`, the incomplete `TaskInfo` cannot be checkpointed,
thus caused these tests to fail.

This patch backfills these fields to make these tests pass.


Diffs (updated)
-

  src/tests/containerizer/mesos_containerizer_tests.cpp 
e61a85df6ec5308ccd2832e66df803b0ad7b53ee 


Diff: https://reviews.apache.org/r/55335/diff/5/

Changes: https://reviews.apache.org/r/55335/diff/4-5/


Testing
---

GTEST_FILTER="MesosContainerizer*" make check


Thanks,

Zhitao Li

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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




src/slave/containerizer/mesos/containerizer.cpp
Line 1050 (original), 1088 (patched)


Added a check for `config.isSome()` and a `warning` log line.


- Zhitao Li


On Sept. 26, 2017, 6:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Sept. 26, 2017, 6:09 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch includes the following change:
> - Checkpointed `ContainerConfig` used to launch a container;
> - Added helper function to read checkpointed `ContainerConfig`;
> - Recovered `ContainerConfig`.
> - Make `ContainerConfig` Option in `Container` struct to indicate whether 
> recovered.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> cc23b4d91be16fc95a131c09d07378b801e34d6f 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 4d5dc13f363f5d8886983d7dd06a5cecc177c345 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/8/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62230: Avoid GC pruning events from blocking other processes.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/slave/gc.cpp
Lines 84 (patched)


We already have

```
  ~Executor()
  {
terminate(process);
wait(process);
  }
```

Does the `stop()` here make a difference?



src/slave/gc.cpp
Line 229 (original), 230 (patched)


There's a `#include ` added for `async` that we can 
remove now.



src/slave/gc_process.hpp
Lines 110 (patched)


Can we add a bit more expanation since the name `executor` is pretty 
generic?

e.g., 

```
// For executing path removals in a separate thread.
```


- Jiang Yan Xu


On Sept. 12, 2017, 1:52 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62230/
> ---
> 
> (Updated Sept. 12, 2017, 1:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7964
> https://issues.apache.org/jira/browse/MESOS-7964
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch dispatches `rmdirs` to a single executor instead of multiple
> `AsyncExecutor`s such that heavy-duty pruning events won't occupy all
> worker threads and block other actors.
> 
> 
> Diffs
> -
> 
>   src/slave/gc.cpp 83e4e2f3aba5c0d9900cf0beeea6e92320f889e7 
>   src/slave/gc_process.hpp c383ce28411622692e42401c80e9443e7b1f5099 
> 
> 
> Diff: https://reviews.apache.org/r/62230/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Zhitao Li]

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

(Updated Sept. 26, 2017, 6:09 p.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


Changes
---

Rebase with Gilbert's comments addressed.


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


Repository: mesos


Description
---

This patch includes the following change:
- Checkpointed `ContainerConfig` used to launch a container;
- Added helper function to read checkpointed `ContainerConfig`;
- Recovered `ContainerConfig`.
- Make `ContainerConfig` Option in `Container` struct to indicate whether 
recovered.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.hpp 
cc23b4d91be16fc95a131c09d07378b801e34d6f 
  src/slave/containerizer/mesos/containerizer.cpp 
4d5dc13f363f5d8886983d7dd06a5cecc177c345 
  src/slave/containerizer/mesos/paths.hpp 
a03f15e01e8eb8a1326baad8d7db96a79d785482 
  src/slave/containerizer/mesos/paths.cpp 
0c61c20c345a327ec469b382558aaeed0280e754 


Diff: https://reviews.apache.org/r/55334/diff/8/

Changes: https://reviews.apache.org/r/55334/diff/7-8/


Testing
---


Thanks,

Zhitao Li

Review Request 62581: Added `__removeSlave` function and made `_markUnreachable` use it.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

This would also be used by the `markGone()` function in the next
review to remove agents.


Diffs
-

  src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
  src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 


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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 62475: Added `MARK_AGENT_GONE` call to the v1 Master API.

[Anand Mazumdar]

> On Sept. 25, 2017, 11:41 p.m., Vinod Kone wrote:
> > include/mesos/master/master.proto
> > Lines 205-206 (patched)
> > 
> >
> > The last sentence is not strictly true right? A new agent with a new id 
> > but old volumes/reservations can still come back right? If so, we should 
> > mention that possibility.

hmm, thats possible. The reason I did not mention it was that it's not possible 
without the operator manually tinkering and removing the `latest` symlink. What 
do you think?


- Anand


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


On Sept. 24, 2017, 5:06 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62475/
> ---
> 
> (Updated Sept. 24, 2017, 5:06 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7443
> https://issues.apache.org/jira/browse/MESOS-7443
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces the `MARK_AGENT_GONE` call that can be
> used by operators to assert that a given agent has failed. It
> is specially useful for stateful frameworks to ascertain whether
> its safe to move the workload to a new agent.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto b94e90287982e620749c10bec77cf0af10318415 
>   include/mesos/v1/master/master.proto 
> 7499fa4f62ab18dd3cd4827461717bc9c688dc49 
> 
> 
> Diff: https://reviews.apache.org/r/62475/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62548: Reorganized and updated the contribution guidelines.

[Tom Runyon]

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



I like the idea of adding PR submissions for "trivial" improvements.  I would 
suggest adding some guidelines on what that would mean.  Our internal team used 
to have full code reviews for non-trivial changes vs PRs for trivial changes 
and eventually we've creeped to having only PRs since its that much easier.  
Some line in the sand would help people understand the divide, and prevent that 
line from shifting.

- Tom Runyon


On Sept. 25, 2017, 6:02 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62548/
> ---
> 
> (Updated Sept. 25, 2017, 6:02 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Mahler, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-564
> https://issues.apache.org/jira/browse/MESOS-564
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reorganized and updated the contribution guidelines.
> 
> 
> Diffs
> -
> 
>   docs/home.md f9b35e3e8f9af024a58760c345931d73a83654ff 
>   docs/newbie-guide.md e9f2aaac1bb986120f34b1d006e3a2f5eb2779ff 
>   docs/reopening-reviews.md fe5046830bbdf28fcc2377ffa5792549920afbc8 
>   docs/reporting-a-bug.md a7e372c1f0d3a34a06244aecb1dfeef7356b8928 
>   docs/submitting-a-patch.md ffc6e561b8721b8849ef6025c15936ea712d3bfa 
>   site/source/community.html.md f56131fedb935ff695206948e16252c62ae0f36a 
> 
> 
> Diff: https://reviews.apache.org/r/62548/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 62479: Removed the logic for removing the latest symlink on the agent.

[Anand Mazumdar]

> On Sept. 26, 2017, 12:47 a.m., Vinod Kone wrote:
> > src/slave/slave.cpp
> > Line 889 (original)
> > 
> >
> > So sending a shutdown message now is only for shutting down the 
> > tasks/executors? Can we add/update a comment on the ShutdownSlaveMessage 
> > proto?

Looks like the `ShutdownMessage` already has this comment i.e., it only 
shutsdown tasks/executor before committing suicide.


- Anand


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


On Sept. 21, 2017, 8:28 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62479/
> ---
> 
> (Updated Sept. 21, 2017, 8:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change removes the logic of removing the latest symlink on
> receiving the shutdown message from the Mesos master. This ensures
> that agents come back with the same agent ID upon a successful
> shutdown similar to the behavior when they come back post a reboot.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 01020a624bdd67ee6371f607d897ad71a2cdcc82 
> 
> 
> Diff: https://reviews.apache.org/r/62479/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62478: Added the mark agent gone handler on the master.

[Anand Mazumdar]

> On Sept. 26, 2017, 12:41 a.m., Vinod Kone wrote:
> > src/master/http.cpp
> > Lines 5312-5338 (patched)
> > 
> >
> > These warnings and return messages seem inconsistent? Can we make them 
> > consistent?

Looks like I updated it already in the second diff; so should be fine. Dropping


> On Sept. 26, 2017, 12:41 a.m., Vinod Kone wrote:
> > src/master/master.cpp
> > Line 7245 (original), 7237 (patched)
> > 
> >
> > Would've been better if this refactor was done in a dependent review.

Yep, my bad. Creating a separate patch


- Anand


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


On Sept. 21, 2017, 8:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62478/
> ---
> 
> (Updated Sept. 21, 2017, 8:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the neccessary logic for handling the mark agent
> gone call on the master. Once an agent is marked as gone, it's
> not allowed to re-register with the Mesos master. GC'ing the
> list of gone agents (it can grow unbounded) would be added in
> a separate change.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 28d0393fb5962df4d731521265efd81a54e1e655 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/master/validation.cpp a6e6a90af7d8d1242e28e93af551a2096db62939 
> 
> 
> Diff: https://reviews.apache.org/r/62478/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62481: Fixed tests impacted by no longer removing the agent symlink.

[Anand Mazumdar]

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

(Updated Sept. 26, 2017, 4:59 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description (updated)
---

This change fixes tests that relied on the agent work directory
symlink being removed when the agent receives the shutdown message
from the master.

Review: https://reviews.apache.org/r/62481


Diffs (updated)
-

  src/tests/gc_tests.cpp 5581861e8ff834b4e79a51bef5f28aac7e7d259d 
  src/tests/slave_recovery_tests.cpp 0cd2b5d37e35ccc2fa2c14db750d1314238bc312 
  src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 


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

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 62477: Added the gone agents to the master registry.

[Anand Mazumdar]

> On Sept. 25, 2017, 11:56 p.m., Vinod Kone wrote:
> > src/master/master.hpp
> > Lines 2306 (patched)
> > 
> >
> > Looks like this is not possible according to the code in the next 
> > review. If so, can you return an `Error` with a comment similar to #2127.

My bad; Fix incoming!


- Anand


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


On Sept. 21, 2017, 8:27 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62477/
> ---
> 
> (Updated Sept. 21, 2017, 8:27 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7444
> https://issues.apache.org/jira/browse/MESOS-7444
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the list of gone agents to the registry and also
> introduces the `MarkSlaveGone` operation on the Mesos master. This
> would be used by the Master Operator API handler to insert an agent
> into the list of gone agent.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/tests/registrar_tests.cpp b5fc45887e25345d49feb4975f6256cfd7b0fe55 
> 
> 
> Diff: https://reviews.apache.org/r/62477/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62553: Fixed a flaky test.

[Benjamin Hindman]

> On Sept. 26, 2017, 1:08 a.m., Gilbert Song wrote:
> > src/tests/containerizer/cgroups_isolator_tests.cpp
> > Line 445 (original), 444 (patched)
> > 
> >
> > The flaky behavior is still reproducable:
> > ```
> > [ RUN  ] CgroupsIsolatorTest.ROOT_CGROUPS_CFS_EnableCfs
> > I0926 01:07:11.409624 20616 exec.cpp:162] Version: 1.5.0
> > I0926 01:07:11.10 20637 exec.cpp:237] Executor registered on agent 
> > e36d5653-eee2-4f69-92ee-3d8aec9a3cdc-S0
> > I0926 01:07:11.563771 20636 executor.cpp:171] Received SUBSCRIBED event
> > I0926 01:07:11.565313 20636 executor.cpp:175] Subscribed executor on 
> > vagrant-ubuntu-wily-64
> > I0926 01:07:11.566074 20636 executor.cpp:171] Received LAUNCH event
> > I0926 01:07:11.567462 20636 executor.cpp:633] Starting task 
> > 7995dc33-b190-4295-b643-b274f278ad50
> > I0926 01:07:11.583531 20636 executor.cpp:477] Running 
> > '/vagrant/mesos/build/src/mesos-containerizer launch 
> > '
> > I0926 01:07:11.594823 20636 executor.cpp:646] Forked command at 20639
> > ../../src/tests/containerizer/cgroups_isolator_tests.cpp:444: Failure
> > Expected: (0.35) >= (cpuTime), actual: 0.35 vs 0.39
> > *** Aborted at 1506388031 (unix time) try "date -d @1506388031" if you 
> > are using GNU date ***
> > PC: @  0x263f886 testing::UnitTest::AddTestPartResult()
> > *** SIGSEGV (@0x0) received by PID 17734 (TID 0x7f106c83a800) from PID 
> > 0; stack trace: ***
> > @ 0x7f1062d24d10 (unknown)
> > @  0x263f886 testing::UnitTest::AddTestPartResult()
> > @  0x263f409 testing::internal::AssertHelper::operator=()
> > @  0x246ebd7 
> > mesos::internal::tests::CgroupsIsolatorTest_ROOT_CGROUPS_CFS_EnableCfs_Test::TestBody()
> > @  0x267d903 
> > testing::internal::HandleSehExceptionsInMethodIfSupported<>()
> > @  0x266a467 
> > testing::internal::HandleExceptionsInMethodIfSupported<>()
> > @  0x26495d5 testing::Test::Run()
> > @  0x264a301 testing::TestInfo::Run()
> > @  0x264aa17 testing::TestCase::Run()
> > @  0x2652418 testing::internal::UnitTestImpl::RunAllTests()
> > @  0x267ae33 
> > testing::internal::HandleSehExceptionsInMethodIfSupported<>()
> > @  0x266c457 
> > testing::internal::HandleExceptionsInMethodIfSupported<>()
> > @  0x26520d5 testing::UnitTest::Run()
> > @  0x1619be1 RUN_ALL_TESTS()
> > @  0x161861d main
> > @ 0x7f106296aa40 (unknown)
> > @   0xaec8a9 _start
> > I0926 01:07:11.910209 20630 exec.cpp:517] Agent exited ... shutting down
> > I0926 01:07:11.910732 20630 executor.cpp:171] Received SHUTDOWN event
> > I0926 01:07:11.911000 20630 executor.cpp:743] Shutting down
> > I0926 01:07:11.911231 20630 executor.cpp:850] Sending SIGTERM to 
> > process tree at pid 20639
> > ```
> > 
> > Should we give a 200ms buffer instead?

Yes, that's fine with me. But I don't love this test. The real crux here is 
that this test is really dependent on the environment, i.e., what is running on 
that machine. Is there a better way to do this? Is there something else we can 
"test" to ensure that CFS has been turned on, rather than trying to actually 
exercise the CFS algorithm?


- Benjamin


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


On Sept. 25, 2017, 10:47 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62553/
> ---
> 
> (Updated Sept. 25, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a flaky test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> 3fc93417f2d3febf2feca3ec1c8476c9edcfbf4d 
> 
> 
> Diff: https://reviews.apache.org/r/62553/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Review Request 62579: [WIP] Added a test `BlkioIsolatorTest.ROOT_BlkioUsage`.

[Qian Zhang]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Added a test `BlkioIsolatorTest.ROOT_BlkioUsage`.


Diffs
-

  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/tests/CMakeLists.txt def233d13f627f58362fdc61ac3e9c104d65fe7e 
  src/tests/containerizer/blkio_isolator_tests.cpp PRE-CREATION 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 62067: Added 'mesos container list' command to CLI.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 61212.

Failed command: `python.exe .\support\apply-reviews.py -n -r 61212`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62067

Relevant logs:

- 
[apply-review-61212-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62067/logs/apply-review-61212-stdout.log):

```
error: patch failed: src/python/cli_new/lib/cli/util.py:22
error: src/python/cli_new/lib/cli/util.py: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 4, 2017, 7:54 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62067/
> ---
> 
> (Updated Sept. 4, 2017, 7:54 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7285
> https://issues.apache.org/jira/browse/MESOS-7285
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the containers running
> on all the agents or a specific one.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
>   src/python/cli_new/lib/cli/plugins/container/__init__.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/container/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/__init__.py 
> 0daf28869e107263c51653ace39e3b1826871048 
>   src/python/cli_new/lib/cli/tests/container.py PRE-CREATION 
>   src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 
> 
> 
> Diff: https://reviews.apache.org/r/62067/diff/2/
> 
> 
> Testing
> ---
> 
> To test with one master:
> 
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> 
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61213: Added 'mesos task list' command to CLI.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 61213 was successfully built and tested.

Reviews applied: `['61212', '61213']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/61213

- Mesos Reviewbot Windows


On Sept. 26, 2017, 2:57 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61213/
> ---
> 
> (Updated Sept. 26, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7840
> https://issues.apache.org/jira/browse/MESOS-7840
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the active tasks in a cluster
> by reaching the tasks endpoint of a master.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
>   src/python/cli_new/lib/cli/plugins/task/__init__.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/task/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/__init__.py 
> 0daf28869e107263c51653ace39e3b1826871048 
>   src/python/cli_new/lib/cli/tests/task.py PRE-CREATION 
>   src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 
> 
> 
> Diff: https://reviews.apache.org/r/61213/diff/5/
> 
> 
> Testing
> ---
> 
> To test with one master:
> ```
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> ```
> 
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61213: Added 'mesos task list' command to CLI.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Sept. 26, 2017, 2:57 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61213/
> ---
> 
> (Updated Sept. 26, 2017, 2:57 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7840
> https://issues.apache.org/jira/browse/MESOS-7840
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the active tasks in a cluster
> by reaching the tasks endpoint of a master.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
>   src/python/cli_new/lib/cli/plugins/task/__init__.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/task/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/__init__.py 
> 0daf28869e107263c51653ace39e3b1826871048 
>   src/python/cli_new/lib/cli/tests/task.py PRE-CREATION 
>   src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 
> 
> 
> Diff: https://reviews.apache.org/r/61213/diff/5/
> 
> 
> Testing
> ---
> 
> To test with one master:
> ```
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> ```
> 
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61213: Added 'mesos task list' command to CLI.

[Armand Grillet]

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

(Updated Sept. 26, 2017, 2:57 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Resolved issue.


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


Repository: mesos


Description
---

This command displays the active tasks in a cluster
by reaching the tasks endpoint of a master.


Diffs (updated)
-

  src/python/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
  src/python/cli_new/lib/cli/plugins/task/__init__.py PRE-CREATION 
  src/python/cli_new/lib/cli/plugins/task/main.py PRE-CREATION 
  src/python/cli_new/lib/cli/tests/__init__.py 
0daf28869e107263c51653ace39e3b1826871048 
  src/python/cli_new/lib/cli/tests/task.py PRE-CREATION 
  src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 


Diff: https://reviews.apache.org/r/61213/diff/5/

Changes: https://reviews.apache.org/r/61213/diff/4-5/


Testing
---

To test with one master:
```
$ ./bootstrap
$ source activate
$ mesos-cli-tests
```

I also checked that the Python linter was still working.


Thanks,

Armand Grillet

Re: Review Request 61213: Added 'mesos task list' command to CLI.

[Kevin Klues]

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




src/python/cli_new/lib/cli/plugins/task/main.py
Lines 57 (patched)


Save the master address into a temporary variable in its own try block.


- Kevin Klues


On Sept. 4, 2017, 2:49 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61213/
> ---
> 
> (Updated Sept. 4, 2017, 2:49 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7840
> https://issues.apache.org/jira/browse/MESOS-7840
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the active tasks in a cluster
> by reaching the tasks endpoint of a master.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
>   src/python/cli_new/lib/cli/plugins/task/__init__.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/task/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/__init__.py 
> 0daf28869e107263c51653ace39e3b1826871048 
>   src/python/cli_new/lib/cli/tests/task.py PRE-CREATION 
>   src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 
> 
> 
> Diff: https://reviews.apache.org/r/61213/diff/4/
> 
> 
> Testing
> ---
> 
> To test with one master:
> ```
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> ```
> 
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62065: Added 'mesos agent list' command to CLI.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62065`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62065

Relevant logs:

- 
[apply-review-62065-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62065/logs/apply-review-62065-stdout.log):

```
error: patch failed: src/python/cli_new/bin/settings.py:38
error: src/python/cli_new/bin/settings.py: patch does not apply
error: patch failed: src/python/cli_new/lib/cli/tests/__init__.py:20
error: src/python/cli_new/lib/cli/tests/__init__.py: patch does not apply
error: patch failed: src/python/cli_new/tests/main.py:27
error: src/python/cli_new/tests/main.py: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 4, 2017, 2:51 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62065/
> ---
> 
> (Updated Sept. 4, 2017, 2:51 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the agents in a cluster by
> reaching the slaves endpoint of a master.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
>   src/python/cli_new/lib/cli/plugins/agent/__init__.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/agent/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/__init__.py 
> 0daf28869e107263c51653ace39e3b1826871048 
>   src/python/cli_new/lib/cli/tests/agent.py PRE-CREATION 
>   src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 
> 
> 
> Diff: https://reviews.apache.org/r/62065/diff/2/
> 
> 
> Testing
> ---
> 
> To test with one master:
> 
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> 
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61212: Added CLI utility function to verify addresses.

[Armand Grillet]

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

(Updated Sept. 26, 2017, 2:15 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Resolved issues.


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


Repository: mesos


Description
---

This will be used by future plugins.


Diffs (updated)
-

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


Diff: https://reviews.apache.org/r/61212/diff/7/

Changes: https://reviews.apache.org/r/61212/diff/6-7/


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Re: Review Request 62067: CLI: Added 'mesos container list' command.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 62573.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62573`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62067

Relevant logs:

- 
[apply-review-62573-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62067/logs/apply-review-62573-stdout.log):

```
error: patch failed: src/python/cli_new/lib/cli/util.py:185
error: src/python/cli_new/lib/cli/util.py: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 4, 2017, 2:54 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62067/
> ---
> 
> (Updated Sept. 4, 2017, 2:54 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7285
> https://issues.apache.org/jira/browse/MESOS-7285
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This command displays the containers running
> on all the agents or a specific one.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bin/settings.py d42df04b0ff42bb6f466842e59223cd90a74d5c0 
>   src/python/cli_new/lib/cli/plugins/container/__init__.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/container/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/__init__.py 
> 0daf28869e107263c51653ace39e3b1826871048 
>   src/python/cli_new/lib/cli/tests/container.py PRE-CREATION 
>   src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 
> 
> 
> Diff: https://reviews.apache.org/r/62067/diff/2/
> 
> 
> Testing
> ---
> 
> To test with one master:
> 
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> 
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 61212: Added CLI utility function to verify addresses.

[Kevin Klues]

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




src/python/cli_new/lib/cli/util.py
Lines 162 (patched)


Can we change this comment to make the usage of 'basestring' here more 
clear (how it is the base class for both unicode and string and depending on 
how this field is passed in it could be either of these).



src/python/cli_new/lib/cli/util.py
Lines 169 (patched)


s/respect/match/



src/python/cli_new/lib/cli/util.py
Lines 188 (patched)


Let's break this out into its own commit.


- Kevin Klues


On Sept. 26, 2017, 1:53 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61212/
> ---
> 
> (Updated Sept. 26, 2017, 1:53 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7840
> https://issues.apache.org/jira/browse/MESOS-7840
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used by future plugins.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/61212/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 62573: Added CLI utility function to get agent address.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

This will be used by future plugins.


Diffs
-

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


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


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Re: Review Request 61212: Added CLI utility function to verify addresses.

[Armand Grillet]

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

(Updated Sept. 26, 2017, 1:53 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Cut patch.


Summary (updated)
-

Added CLI utility function to verify addresses.


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


Repository: mesos


Description
---

This will be used by future plugins.


Diffs (updated)
-

  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


Diff: https://reviews.apache.org/r/61212/diff/6/

Changes: https://reviews.apache.org/r/61212/diff/5-6/


Testing
---

Tested manually, PEP8 and Pylint used to make sure that the code style is 
correct.


Thanks,

Armand Grillet

Re: Review Request 60088: CLI: Added 'master' key as an acceptable key in config.toml.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> ---
> 
> (Updated Sept. 25, 2017, 2:03 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
>   src/python/cli_new/lib/cli/config.py 
> 36a32f94bb12a033705b20f3c91d8bba972ba6c5 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/60088/diff/7/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60088: CLI: Added 'master' key as an acceptable key in config.toml.

[Kevin Klues]

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




src/python/cli_new/lib/cli/config.py
Lines 57 (patched)


I'll remove this comment before committing since we now pull i nthe 
constants from elsewhere.


- Kevin Klues


On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> ---
> 
> (Updated Sept. 25, 2017, 2:03 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
>   src/python/cli_new/lib/cli/config.py 
> 36a32f94bb12a033705b20f3c91d8bba972ba6c5 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/60088/diff/7/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62572: Added constants for the Mesos CLI.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 62544.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62544`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62572

Relevant logs:

- 
[apply-review-62544-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62572/logs/apply-review-62544-stdout.log):

```
error: patch failed: src/python/pylint.config:17
error: src/python/pylint.config: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 26, 2017, 1:26 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62572/
> ---
> 
> (Updated Sept. 26, 2017, 1:26 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will be used by future commands and functions of the CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/constants.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/62572/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 62572: Added constants for the Mesos CLI.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

This will be used by future commands and functions of the CLI.


Diffs
-

  src/python/cli_new/lib/cli/constants.py PRE-CREATION 


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


Testing
---


Thanks,

Armand Grillet

Re: Review Request 60088: CLI: Added 'master' key as an acceptable key in config.toml.

[Kevin Klues]

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




src/python/cli_new/README.md
Lines 82 (patched)


I would add a "For example:" here.



src/python/cli_new/lib/cli/config.py
Lines 54 (patched)


We shouldn't need this anymore.



src/python/cli_new/lib/cli/config.py
Lines 58 (patched)


We should probably pull this in from the constants file instead of hard 
coding it as a string here.



src/python/cli_new/lib/cli/config.py
Lines 65 (patched)


The indentation here seems wrong.



src/python/cli_new/lib/cli/config.py
Lines 67 (patched)


One too many spaces here



src/python/cli_new/lib/cli/config.py
Lines 70 (patched)


the indentation here seems wrong



src/python/cli_new/lib/cli/config.py
Lines 80 (patched)


The 'master' address



src/python/cli_new/lib/cli/config.py
Lines 88 (patched)


We need to first check if 'addresses' in 'zk_field' before we can index it.



src/python/cli_new/lib/cli/config.py
Lines 91 (patched)


We need to first check if 'path' in 'zk_field' before we can index it.



src/python/cli_new/lib/cli/config.py
Lines 95 (patched)


I think it's OK to start with a `/` it just can't be *only* `/'



src/python/cli_new/lib/cli/config.py
Lines 102 (patched)


The 'zookeeper' address.



src/python/cli_new/lib/cli/config.py
Lines 111 (patched)


s/leader/leading/


- Kevin Klues


On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> ---
> 
> (Updated Sept. 25, 2017, 2:03 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
>   src/python/cli_new/lib/cli/config.py 
> 36a32f94bb12a033705b20f3c91d8bba972ba6c5 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/60088/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 60088: CLI: Added 'master' key as an acceptable key in config.toml.

[Kevin Klues]

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




src/python/cli_new/README.md
Lines 82 (patched)


At the end of this comment I'd add "For example:"



src/python/cli_new/lib/cli/config.py
Lines 72 (patched)


Let's verify the address format here and throw an error if it's wrong.


- Kevin Klues


On Sept. 25, 2017, 2:03 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60088/
> ---
> 
> (Updated Sept. 25, 2017, 2:03 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7284
> https://issues.apache.org/jira/browse/MESOS-7284
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This key is a field that has to be composed of
> an `address` or `zookeeper` field, but not both.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md c5475c7f4632fde5303ed1c901918d9088eac660 
>   src/python/cli_new/lib/cli/config.py 
> 36a32f94bb12a033705b20f3c91d8bba972ba6c5 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/60088/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually, PEP8 and Pylint used to make sure that the code style is 
> correct.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>