Re: Review Request 62553: Fixed a flaky test.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [62203, 62553]

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

- Mesos Reviewbot


On 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
> 
>

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/#review186194
---



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/constants.hpp 725680b1e5f1fb9ca87ec5130d0eff72309432ae 
>   src/master/flags.hpp edda71af41a8d199bb24b84487300e9c85b12fec 
>   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/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

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

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

- Mesos Reviewbot


On 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/constants.hpp 725680b1e5f1fb9ca87ec5130d0eff72309432ae 
>   src/master/flags.hpp edda71af41a8d199bb24b84487300e9c85b12fec 
>   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/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Gilbert Song]

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


Fix it, then Ship it!




This patch LGTM.


src/tests/containerizer/nested_mesos_containerizer_tests.cpp
Lines 1648 (patched)


extra space?


- Gilbert Song


On May 31, 2017, 11 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59687/
> ---
> 
> (Updated May 31, 2017, 11 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jason Lai.
> 
> 
> Bugs: MESOS-6894
> https://issues.apache.org/jira/browse/MESOS-6894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for recovering ContainerConfig.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> e46643434dc85d766bd549a037f36a89a6738678 
> 
> 
> Diff: https://reviews.apache.org/r/59687/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Gilbert Song]

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




src/tests/containerizer/mesos_containerizer_tests.cpp
Line 714 (original), 714-720 (patched)


will it look cleaner if use `createTask()` helper?


- Gilbert Song


On Aug. 1, 2017, 10:10 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55335/
> ---
> 
> (Updated Aug. 1, 2017, 10:10 a.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
> ---
> 
> 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
> -
> 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 1fc56c4a8ca95bb47af968ad72dfef69455e7d46 
> 
> 
> Diff: https://reviews.apache.org/r/55335/diff/4/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="MesosContainerizer*" make check
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 62553: Fixed a flaky test.

[Gilbert Song]

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


Fix it, then Ship it!





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?


- Gilbert Song


On Sept. 25, 2017, 3: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, 3: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
> 
>

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/#review186189
---




src/master/flags.cpp
Lines 610-629 (patched)


Can we just reuse the `registry_max_agent_count` and 
`registry_max_agent_age` flags/values? They seem generic enough and used for 
partitioned and removed agents already.


- 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/constants.hpp 725680b1e5f1fb9ca87ec5130d0eff72309432ae 
>   src/master/flags.hpp edda71af41a8d199bb24b84487300e9c85b12fec 
>   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/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62531: Added authorization for 'MARK_AGENT_GONE' call.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 25, 2017, 5:22 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62531/
> ---
> 
> (Updated Sept. 25, 2017, 5:22 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7446
> https://issues.apache.org/jira/browse/MESOS-7446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the relevant ACL's for doing AuthZ (any or none
> access).
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9109283d81a3d25332fd74234099b5e6c9264c06 
>   include/mesos/authorizer/authorizer.proto 
> 38f0e0b48e2a243a594b5d81bddb2ce6fe4cf6bd 
>   src/authorizer/local/authorizer.cpp 
> 82ae846fd39de64704f0e8bd0fe2972f3750d2e6 
>   src/master/http.cpp 28d0393fb5962df4d731521265efd81a54e1e655 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/tests/api_tests.cpp d260a1c9560e8ff6b46eea7f2f4ddb11e18653e3 
> 
> 
> Diff: https://reviews.apache.org/r/62531/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62507: Added documentation about 'MARK_AGENT_GONE' call.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 22, 2017, 6:15 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62507/
> ---
> 
> (Updated Sept. 22, 2017, 6:15 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-8007
> https://issues.apache.org/jira/browse/MESOS-8007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/62529
> 
> 
> Diffs
> -
> 
>   docs/operator-http-api.md f6cfcf1b8bbdc79fac2f361fb770546baf01d992 
> 
> 
> Diff: https://reviews.apache.org/r/62507/diff/2/
> 
> 
> Testing
> ---
> 
> Verified that the doc renders fine.
> 
> 
> 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/#review186185
---




src/tests/slave_recovery_tests.cpp
Line 3137 (original), 3137 (patched)


s/different/same/


- 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/62481/
> ---
> 
> (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 fixes tests that relied on the agent work directory
> symlink being removed when the agent receives the shutdown message
> from the master.
> 
> 
> 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/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/#review186184
---




src/tests/gc_tests.cpp
Line 384 (original), 374 (patched)


What's the point of advancing the clock if the slave process is not up?


- 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/62481/
> ---
> 
> (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 fixes tests that relied on the agent work directory
> symlink being removed when the agent receives the shutdown message
> from the master.
> 
> 
> 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/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62480: Added tests for the agent gone operation.

[Vinod Kone]

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


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/62480/
> ---
> 
> (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 adds tests for marking an active agent as gone and also
> another test for ensuring that running task on an agent marked as gone
> are correctly transitioned to 'TASK_GONE'.
> 
> Review: https://reviews.apache.org/r/62480
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp d260a1c9560e8ff6b46eea7f2f4ddb11e18653e3 
> 
> 
> Diff: https://reviews.apache.org/r/62480/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62553: Fixed a flaky test.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62553 was successfully built and tested.

Reviews applied: `['62203', '62553']`

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

- Mesos Reviewbot Windows


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
> 
>

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/#review186180
---




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?


- 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 55334: Checkpoint and recover `ContainerConfig` in Mesos containerizer.

[Gilbert Song]

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



could you rebase this chain? sorry for the delay, but thank you, Zhitao!


src/slave/containerizer/mesos/containerizer.cpp
Lines 689 (patched)


log the containerId?



src/slave/containerizer/mesos/containerizer.cpp
Lines 695 (patched)


should we use `WARNING` here? since everytime the containerizer recovers, 
it might log a bunch of warning if there are many legacy containers running.



src/slave/containerizer/mesos/containerizer.cpp
Lines 696 (patched)


do we still plan to back fill?



src/slave/containerizer/mesos/containerizer.cpp
Lines 755 (patched)


ditto.



src/slave/containerizer/mesos/containerizer.cpp
Lines 792-794 (patched)


identify this log for both executor container and nested container?



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


this may cause segfault, right?

we should consider address this TODO in a separate patch in favor of 
checkpointing the `ContainerConfig`.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1114-1117 (patched)


How about:

Checkpoint the `ContainerConfig` which includes all information to launch a 
container. Critical information (e.g., `ContainerInfo`) can be used for 
tracking container image usage.



src/slave/containerizer/mesos/containerizer.cpp
Lines 1130 (patched)


quote the path?



src/slave/containerizer/mesos/containerizer.cpp
Line 1191 (original), 1247 (patched)


add a `CHECK_SOME()` in the very beginning of `prepare()` funtion?



src/slave/containerizer/mesos/containerizer.cpp
Line 1486 (original), 1545 (patched)


fix the indentation (extra space)?



src/slave/containerizer/mesos/paths.cpp
Lines 438 (patched)


Move below `getContainerTermination()`?



src/slave/containerizer/mesos/paths.cpp
Lines 449 (patched)


quote the `path`?

and add `containerId`?



src/slave/containerizer/mesos/paths.cpp
Lines 458 (patched)


permutate the space to the line above?

mind fix getContainerTermination() as well?


- Gilbert Song


On Aug. 1, 2017, 10:10 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55334/
> ---
> 
> (Updated Aug. 1, 2017, 10:10 a.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 
> fd586306f7a6d3d2cdb58481f0ef4906de8d0f88 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 6f100b516f2750732acaed693f7023b1e44b39d9 
>   src/slave/containerizer/mesos/paths.hpp 
> 12ae7c7329f9e8d334cb32c30cc38465bddc046c 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
> 
> 
> Diff: https://reviews.apache.org/r/55334/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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/#review186177
---




src/master/http.cpp
Lines 5312-5338 (patched)


These warnings and return messages seem inconsistent? Can we make them 
consistent?



src/master/http.cpp
Lines 5318 (patched)


s/is being/is already being/ to be more explicit?



src/master/http.cpp
Lines 5361 (patched)


is this indentation right?



src/master/master.cpp
Line 7245 (original), 7237 (patched)


Would've been better if this refactor was done in a dependent review.


- 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/2/
> 
> 
> 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/#review186178
---




src/master/registry.proto
Lines 60 (patched)


s/master/operator/ ?


- 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.
> 
> 
> 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 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/#review186176
---




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.


- 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.
> 
> 
> 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 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/#review186174
---


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/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 62476: Added the `REASON_AGENT_REMOVED_BY_OPERATOR` to the mesos protos.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 21, 2017, 9:38 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62476/
> ---
> 
> (Updated Sept. 21, 2017, 9:38 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7426
> https://issues.apache.org/jira/browse/MESOS-7426
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Review: https://reviews.apache.org/r/62476
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto fa475dcc3aa0b15ee4b2b740a24ab3c0b9a3274c 
>   include/mesos/v1/mesos.proto dfba4181203e23f7eedf67c19379d031e0993fd5 
> 
> 
> Diff: https://reviews.apache.org/r/62476/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

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

[Vinod Kone]

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


Fix it, then Ship it!





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.


- Vinod Kone


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
> 
>

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/
---

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/constants.hpp 725680b1e5f1fb9ca87ec5130d0eff72309432ae 
  src/master/flags.hpp edda71af41a8d199bb24b84487300e9c85b12fec 
  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/1/


Testing
---

make check


Thanks,

Anand Mazumdar

Review Request 62553: Fixed a flaky test.

[Benjamin Hindman]

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

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

Re: Review Request 61109: Used the default value when parsing an optional enum field from JSON.

[Benjamin Mahler]

> On Sept. 8, 2017, 11:59 p.m., James Peach wrote:
> > This looks pretty reasonable to me. It's unfortunate that this will convert 
> > all invalid enum names into the default value, but AFAICT that is 
> > unavoidable.
> 
> Benjamin Mahler wrote:
> Since we're talking about optional enums, it's not obvious to me whether 
> it's better to leave it unset or to set it to the default. With a required 
> enum, we can't leave it unset so it seems like the default value makes the 
> most sense. However, shouldn't the caller specify the behavior they want? 
> Much like `JsonParseOptions.ignore_unknown_fields` is an explicit option? 
> This would be something like `use_default_for_unknown_enum_values`?
> 
> Qian Zhang wrote:
> @Ben, the problem is when `Content-Type` is `application/x-protobuf`, our 
> current implementation is an inexistent enum value will be parsed to the 
> default enum value (i.e., `UNKNOWN`), that is what we have done in 
> MESOS-4997, but when `Content-Type` is `application/json`, the current 
> behavior is different: when parsing an inexistent enum value, we will get an 
> error like `Failed to find enum for 'xxx'` rather than parsing it to the 
> default enum value. So in this patch, I just want to make the two protocols 
> (`application/x-protobuf` and `application/json`) have consistent behavior.
> 
> Benjamin Mahler wrote:
> I see, so this is aiming to make it consistent:
> 
> (1) protobuf: unknown enum value -> set to default
> (2) json before this change: unknown enum value -> error
> (3) json after this change: unknown enum value -> set to default
> 
> When you say "our implementation" for (1), are you referring to what the 
> protobuf parsing functions are doing? Or something that we implemented? If 
> it's the former, then this change sounds good to me, since we're just 
> mimicking the protobuf library parsing behavior in JSON.
> 
> Qian Zhang wrote:
> > When you say "our implementation" for (1), are you referring to what 
> the protobuf parsing functions are doing? Or something that we implemented?
> 
> I am referring to the work that we have done in MESOS-4997, i.e., always 
> use optional enum field and include an UNKNOWN value as the first entry in 
> the enum list, that way any inexistent enum value will be parsed to the 
> default enum value (i.e., UNKNOWN). However, I did not figure out an easy way 
> to verify it, I just infer this behavior based on the description in 
> MESOS-4997.

>From MESOS-4997:

> https://developers.google.com/protocol-buffers/docs/proto#updating

> enum is compatible with int32, uint32, int64, and uint64 in terms of wire 
> format (note that values will be truncated if they don't fit), but be aware 
> that client code may treat them differently when the message is deserialized. 
> Notably, unrecognized enum values are discarded when the message is 
> deserialized, which makes the field's has.. accessor return false and its 
> getter return the first value listed in the enum definition. However, an 
> integer field will always preserve its value. Because of this, you need to be 
> very careful when upgrading an integer to an enum in terms of receiving out 
> of bounds enum values on the wire.

This seems to state that unknown enum values will be stripped, i.e. not set to 
the default. It will be unset and accessors will return the default.


- Benjamin


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


On Sept. 21, 2017, 1:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61109/
> ---
> 
> (Updated Sept. 21, 2017, 1:58 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-7828
> https://issues.apache.org/jira/browse/MESOS-7828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously in MESOS-4997, we have made any inexistent enum value will
> be parsed to the default enum value when parsing protobuf message from
> a serialized string. Now in this patch, I made parsing protobuf message
> from a JSON have the same behavior.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 
> 
> 
> Diff: https://reviews.apache.org/r/61109/diff/2/
> 
> 
> Testing
> ---
> 
> With this patch, when accessing master endpoint with an inexistent enum `xxx` 
> in a JSON:
> ```
> curl -X POST -H "Content-Type: application/json" -d '{"type": "xxx"}' 
> 127.0.0.1:5050/api/v1
> ```
> The master log will be:
> ```
> I0725 23:09:53.097790   665 http.cpp:1133] HTTP POST for /master/api/v1 from 
> 127.0.0.1:49566 with User-Agent='curl/7.47.0'
>

Re: Review Request 62353: Added a master-registry backed resource provider manager registry.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 62282.

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

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

Relevant logs:

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

```
error: patch failed: src/common/resources.cpp:174
error: src/common/resources.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 25, 2017, 7:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62353/
> ---
> 
> (Updated Sept. 25, 2017, 7:42 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an implementation of the resource provider registrar
> backed by the master's registrar. With that it becomes possible to
> persist resource provider manager state in a single master registrar,
> but use the narrowly defined resource provider registry.
> 
> 
> Diffs
> -
> 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/62353/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [62548]

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

- Mesos Reviewbot


On 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 59019: Updated the documentation for '--docker_registry' option.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['59019']`

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/59019

Relevant logs:

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

```
[ RUN  ] ContentType/AgentAPITest.NestedContainerKillNotFound/1
[   OK ] ContentType/AgentAPITest.NestedContainerKillNotFound/1 (246 ms)
[ RUN  ] ContentType/AgentAPITest.AttachContainerOutputFailure/0
[   OK ] ContentType/AgentAPITest.AttachContainerOutputFailure/0 (837 ms)
[ RUN  ] ContentType/AgentAPITest.AttachContainerOutputFailure/1
[   OK ] ContentType/AgentAPITest.AttachContainerOutputFailure/1 (903 ms)
[ RUN  ] ContentType/AgentAPITest.DefaultAccept/0
[   OK ] ContentType/AgentAPITest.DefaultAccept/0 (327 ms)
[ RUN  ] ContentType/AgentAPITest.DefaultAccept/1
[   OK ] ContentType/AgentAPITest.DefaultAccept/1 (293 ms)
[--] 30 tests from ContentType/AgentAPITest (12894 ms total)

[--] 4 tests from HTTPCommandExecutor/CommandExecutorTest
[ RUN  ] HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/0
[   OK ] HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/0 
(1927 ms)
[ RUN  ] HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/1
[   OK ] HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/1 
(2083 ms)
[ RUN  ] HTTPCommandExecutor/CommandExecutorTest.TaskKillingCapability/0
[   OK ] HTTPCommandExecutor/CommandExecutorTest.TaskKillingCapability/0 
(2022 ms)
[ RUN  ] HTTPCommandExecutor/CommandExecutorTest.TaskKillingCapability/1
[   OK ] HTTPCommandExecutor/CommandExecutorTest.TaskKillingCapability/1 
(1966 ms)
[--] 4 tests from HTTPCommandExecutor/CommandExecutorTest (8265 ms 
total)

[--] 9 tests from MesosContainerizer/DefaultExecutorTest
[ RUN  ] MesosContainerizer/DefaultExecutorTest.TaskRunning/0
[   OK ] MesosContainerizer/DefaultExecutorTest.TaskRunning/0 (3384 ms)
[ RUN  ] MesosContainerizer/DefaultExecutorTest.KillTask/0
[   OK ] MesosContainerizer/DefaultExecutorTest.KillTask/0 (4661 ms)
[ RUN  ] MesosContainerizer/DefaultExecutorTest.KillTaskGroupOnTaskFailure/0
```

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

```
I0925 21:15:37.030777 29412 master.cpp:3267] Deactivating framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (default)
I0925 21:15:37.038780 30596 hierarchical.cpp:412] Deactivated framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0-
W0925 21:15:37.039780 29412 master.hpp:2439] Unable to send event to framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (default): connection closed
I0925 21:15:37.040781 29412 master.cpp:9225] Removing offer 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0-O1
I0925 21:15:37.040781 29412 master.cpp:3244] Disconnecting framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (default)
I0925 21:15:37.041776 29412 master.cpp:1450] Giving framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (default) 0ns to failover
I0925 21:15:37.053776 28796 master.cpp:7557] Framework failover timeout, 
removing framework c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (default)
I0925 21:15:37.054775 28796 master.cpp:8418] Removing framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (default)
I0925 21:15:37.054775 30088 slave.cpp:3248] Shutting down framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0-
I0925 21:15:37.054775 28796 master.cpp:8993] Updating the state of task 
3cf57446-7f3a-4f14-aac9-5cb32daa36be of framework 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (latest state: TASK_FAILED, status 
update state: TASK_KILLED)
I0925 21:15:37.055774 30088 slave.cpp:5755] Shutting down executor 'default' of 
framework c6fb8ea8-c65c-44d6-aebd-361ab70365f0- (via HTTP)
I0925 21:15:37.055774 28796 master.cpp:9087] Removing task 
3cf57446-7f3a-4f14-aac9-5cb32daa36be with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":0.1},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":32.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":32.0},"type":"SCALAR"}]
 of framework c6fb8ea8-c65c-44d6-aebd-361ab70365f0- on agent 
c6fb8ea8-c65c-44d6-aebd-361ab70365f0-S0 at slave(206)@10.3.1.5:53141 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0925 21:15:37.069774 28796 master.cpp:8993] Updating the state of task 
c8bc3d37-1225-443f-86cb-ebf9383e46cd of framework 
c6fb8ea8-c

Re: Review Request 62252: Added `process::Executor::execute()`.

[Chun-Hung Hsiao]

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

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


Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Michael Park.


Changes
---

Addressed benh's comments and rebased.


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


Repository: mesos


Description
---

This patch adds a convenient interface to `process::Executor` to
asynchronously execute arbitrary functions.


Diffs (updated)
-

  3rdparty/libprocess/include/process/executor.hpp 
cd2f2f03cba8a435f142b31def9f89a6cd61af73 
  3rdparty/libprocess/src/tests/process_tests.cpp 
82efb2f8449e4cb13620cae1a49321fc42e2db60 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 59019: Updated the documentation for '--docker_registry' option.

[Gilbert Song]

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




docs/configuration.md
Lines 1516 (patched)


s/containerizers/containerizer/g



src/slave/flags.cpp
Lines 173 (patched)


ditto.


- Gilbert Song


On Sept. 25, 2017, 12:22 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59019/
> ---
> 
> (Updated Sept. 25, 2017, 12:22 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the documentation for '--docker_registry' option.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 1605e3d56a5735124ac76e22e3ac3119a93c509b 
>   src/slave/flags.cpp 490f78e997dc4013d42674b2448ce280d82c163a 
> 
> 
> Diff: https://reviews.apache.org/r/59019/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62252: Added `process::Executor::execute()`.

[Benjamin Hindman]

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


Fix it, then Ship it!




Just a few minor cleanups and then I'll commit it.


3rdparty/libprocess/src/tests/process_tests.cpp
Line 1212 (original), 1215 (patched)


Let's use lambdas! I've seen you do this in previous patches and it's just 
easier to read (than to have to specify the member function):

```
.WillOnce(InvokeWithoutArgs([&]() {
  event1Called.decrement();
}));
```



3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1214-1215 (original), 1217-1218 (patched)


As long as you're moving stuff around, let's move this and the second 
`CountDownLatch` down to after the first `AWAIT_READY` too.



3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1252 (patched)


`AWAIT_READY(executor.execute(f1));`



3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1264 (patched)


`AWAIT_READY(executor.execute(std::bind(f2, 42)));`



3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1274-1276 (patched)


We can simplify these next ones: `AWAIT_EXPECT_EQ(f3Result, 
executor.execute(f3));`



3rdparty/libprocess/src/tests/process_tests.cpp
Lines 1286-1288 (patched)


`AWAIT_EXPECT_EQ(f4Result, executor.execute(std::bind(f4, 42)));`


- Benjamin Hindman


On Sept. 21, 2017, 8:53 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62252/
> ---
> 
> (Updated Sept. 21, 2017, 8:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benjamin Mahler, and Michael Park.
> 
> 
> Bugs: MESOS-7970
> https://issues.apache.org/jira/browse/MESOS-7970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a convenient interface to `process::Executor` to
> asynchronously execute arbitrary functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/executor.hpp 
> cd2f2f03cba8a435f142b31def9f89a6cd61af73 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 82efb2f8449e4cb13620cae1a49321fc42e2db60 
> 
> 
> Diff: https://reviews.apache.org/r/62252/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

[Benjamin Bannier]

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

(Updated Sept. 25, 2017, 9:42 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed comments from Jan.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


Diff: https://reviews.apache.org/r/61528/diff/12/

Changes: https://reviews.apache.org/r/61528/diff/11-12/


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Benjamin Bannier]

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

(Updated Sept. 25, 2017, 9:42 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed comments from Jan, improved comments.


Repository: mesos


Description
---

The agent's resource provider manager sends a
'ResourceProviderMessage' when its managed resources change. This
commit adds handling in the agent so that an 'UpdateSlaveMessage' is
sent to the master to update the total resource available on the
agent.

In order to provide push-like handling of the resource provider
manager's message queue, we chain recursive calls to the handler for
continuous processing. Initially, processing is kicked off from
'Slave::initialize'. In this simple implementation we e.g., provide no
direct way to stop processing of messages, yet, but it can be achieved
by e.g., replacing the manager with a new instance (this would also
require updating routes).

Since the agent can only send an 'UpdateSlaveMessage' when it is
registered with a master, a simple back-off of 5 s is implemented which
will defer processing of a ready message should the agent not yet have
registered.

To facilitate logging we add a stringification function for
'ResourceProviderMessage's.

This patch also adjusts a number of tests to no expect two
'UpdateSlaveMessage's.


Diffs (updated)
-

  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 
  src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca 
  src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 


Diff: https://reviews.apache.org/r/61183/diff/13/

Changes: https://reviews.apache.org/r/61183/diff/12-13/


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 62353: Added a master-registry backed resource provider manager registry.

[Benjamin Bannier]

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

(Updated Sept. 25, 2017, 9:42 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

This patch adds an implementation of the resource provider registrar
backed by the master's registrar. With that it becomes possible to
persist resource provider manager state in a single master registrar,
but use the narrowly defined resource provider registry.


Diffs (updated)
-

  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62548 was successfully built and tested.

Reviews applied: `['62548']`

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

- Mesos Reviewbot Windows


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 59019: Updated the documentation for '--docker_registry' option.

[Chun-Hung Hsiao]

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

(Updated Sept. 25, 2017, 7:22 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Rebased and addressed gilbert's comments.


Repository: mesos


Description
---

Updated the documentation for '--docker_registry' option.


Diffs (updated)
-

  docs/configuration.md 1605e3d56a5735124ac76e22e3ac3119a93c509b 
  src/slave/flags.cpp 490f78e997dc4013d42674b2448ce280d82c163a 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 62531: Added authorization for 'MARK_AGENT_GONE' call.

[Anand Mazumdar]

> On Sept. 25, 2017, 7:19 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some Mesos tests failed.
> > 
> > Reviews applied: `['62475', '62476', '62477', '62478', '62479', '62480', 
> > '62481', '62507', '62531']`
> > 
> > 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/62531
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62531/logs/mesos-tests-stdout.log):
> > 
> > ```
> > [ RUN  ] ContentType/MasterAPITest.CreateAndDestroyVolumes/0
> > [   OK ] ContentType/MasterAPITest.CreateAndDestroyVolumes/0 (963 ms)
> > [ RUN  ] ContentType/MasterAPITest.CreateAndDestroyVolumes/1
> > [   OK ] ContentType/MasterAPITest.CreateAndDestroyVolumes/1 (1110 ms)
> > [ RUN  ] ContentType/MasterAPITest.GetWeights/0
> > [   OK ] ContentType/MasterAPITest.GetWeights/0 (249 ms)
> > [ RUN  ] ContentType/MasterAPITest.GetWeights/1
> > [   OK ] ContentType/MasterAPITest.GetWeights/1 (234 ms)
> > [ RUN  ] ContentType/MasterAPITest.UpdateWeights/0
> > [   OK ] ContentType/MasterAPITest.UpdateWeights/0 (289 ms)
> > [ RUN  ] ContentType/MasterAPITest.UpdateWeights/1
> > [   OK ] ContentType/MasterAPITest.UpdateWeights/1 (301 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFile/0
> > [   OK ] ContentType/MasterAPITest.ReadFile/0 (327 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFile/1
> > [   OK ] ContentType/MasterAPITest.ReadFile/1 (337 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFileInvalidPath/0
> > [   OK ] ContentType/MasterAPITest.ReadFileInvalidPath/0 (211 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFileInvalidPath/1
> > [   OK ] ContentType/MasterAPITest.ReadFileInvalidPath/1 (216 ms)
> > [ RUN  ] ContentType/MasterAPITest.Teardown/0
> > [   OK ] ContentType/MasterAPITest.Teardown/0 (939 ms)
> > [ RUN  ] ContentType/MasterAPITest.Teardown/1
> > [   OK ] ContentType/MasterAPITest.Teardown/1 (1091 ms)
> > [ RUN  ] ContentType/MasterAPITest.MarkRegisteredAgentGone/0
> > 
> > C:\mesos\mesos\3rdparty\libprocess\include\process/gmock.hpp(235): ERROR: 
> > this mock object (used in test 
> > ContentType/MasterAPITest.MarkRegisteredAgentGone/0) should be deleted but 
> > never is. Its address is @026D5BF22CF8.
> > C:\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object 
> > (used in test ContentType/MasterAPITest.MarkRegisteredAgentGone/0) should 
> > be deleted but never is. Its address is @026D5D295500.
> > ERROR: 2 leaked mock objects found at program exit.
> > ```
> > 
> > - 
> > [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62531/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: 54003
> > 
> > 
> > To remedy this do as follows:
> > Step 1: rm -f C:\Users\mesos\AppData\Local\Temp\2\FiWVVF\meta\slaves\latest
> > This ensures agent doesn't recover old live executors.
> > Step 2: Restart the agent.
> > ```
> 
> Andrew Schwartzmeyer wrote:
> This appears to be failing due to 
> https://issues.apache.org/jira/browse/MESOS-7604. If you want to use 
> `TEST_F_TEMP_DISABLED_ON_WINDOWS` that'd be fine, and add a `// 
> TODO(andschwa): Enable this when MESOS-7604 is fixed.` that'd be great :)

Thanks for the tip Andrew; updating the review shortly.


- Anand


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


On Sept. 25, 2017, 5:22 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62531/
> ---
> 
> (Updated Sept. 25, 2017, 5:22 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7446
> https://issues.apache.org/jira/browse/MESOS-7446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the relevant ACL's for doing AuthZ (any or none
> access).
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9109283d81a3d25332fd74234099b5e6c9264c06 
>   include/mesos/authorizer/authorizer.proto 
> 38f0e0b48e2a243a594b5d81bddb2ce6fe4cf6bd 
>   src/authorizer/local/authorizer.cpp 
> 82a

Re: Review Request 62531: Added authorization for 'MARK_AGENT_GONE' call.

[Andrew Schwartzmeyer]

> On Sept. 25, 2017, 12:19 a.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some Mesos tests failed.
> > 
> > Reviews applied: `['62475', '62476', '62477', '62478', '62479', '62480', 
> > '62481', '62507', '62531']`
> > 
> > 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/62531
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62531/logs/mesos-tests-stdout.log):
> > 
> > ```
> > [ RUN  ] ContentType/MasterAPITest.CreateAndDestroyVolumes/0
> > [   OK ] ContentType/MasterAPITest.CreateAndDestroyVolumes/0 (963 ms)
> > [ RUN  ] ContentType/MasterAPITest.CreateAndDestroyVolumes/1
> > [   OK ] ContentType/MasterAPITest.CreateAndDestroyVolumes/1 (1110 ms)
> > [ RUN  ] ContentType/MasterAPITest.GetWeights/0
> > [   OK ] ContentType/MasterAPITest.GetWeights/0 (249 ms)
> > [ RUN  ] ContentType/MasterAPITest.GetWeights/1
> > [   OK ] ContentType/MasterAPITest.GetWeights/1 (234 ms)
> > [ RUN  ] ContentType/MasterAPITest.UpdateWeights/0
> > [   OK ] ContentType/MasterAPITest.UpdateWeights/0 (289 ms)
> > [ RUN  ] ContentType/MasterAPITest.UpdateWeights/1
> > [   OK ] ContentType/MasterAPITest.UpdateWeights/1 (301 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFile/0
> > [   OK ] ContentType/MasterAPITest.ReadFile/0 (327 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFile/1
> > [   OK ] ContentType/MasterAPITest.ReadFile/1 (337 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFileInvalidPath/0
> > [   OK ] ContentType/MasterAPITest.ReadFileInvalidPath/0 (211 ms)
> > [ RUN  ] ContentType/MasterAPITest.ReadFileInvalidPath/1
> > [   OK ] ContentType/MasterAPITest.ReadFileInvalidPath/1 (216 ms)
> > [ RUN  ] ContentType/MasterAPITest.Teardown/0
> > [   OK ] ContentType/MasterAPITest.Teardown/0 (939 ms)
> > [ RUN  ] ContentType/MasterAPITest.Teardown/1
> > [   OK ] ContentType/MasterAPITest.Teardown/1 (1091 ms)
> > [ RUN  ] ContentType/MasterAPITest.MarkRegisteredAgentGone/0
> > 
> > C:\mesos\mesos\3rdparty\libprocess\include\process/gmock.hpp(235): ERROR: 
> > this mock object (used in test 
> > ContentType/MasterAPITest.MarkRegisteredAgentGone/0) should be deleted but 
> > never is. Its address is @026D5BF22CF8.
> > C:\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object 
> > (used in test ContentType/MasterAPITest.MarkRegisteredAgentGone/0) should 
> > be deleted but never is. Its address is @026D5D295500.
> > ERROR: 2 leaked mock objects found at program exit.
> > ```
> > 
> > - 
> > [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62531/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: 54003
> > 
> > 
> > To remedy this do as follows:
> > Step 1: rm -f C:\Users\mesos\AppData\Local\Temp\2\FiWVVF\meta\slaves\latest
> > This ensures agent doesn't recover old live executors.
> > Step 2: Restart the agent.
> > ```

This appears to be failing due to 
https://issues.apache.org/jira/browse/MESOS-7604. If you want to use 
`TEST_F_TEMP_DISABLED_ON_WINDOWS` that'd be fine, and add a `// TODO(andschwa): 
Enable this when MESOS-7604 is fixed.` that'd be great :)


- Andrew


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


On Sept. 24, 2017, 10:22 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62531/
> ---
> 
> (Updated Sept. 24, 2017, 10:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7446
> https://issues.apache.org/jira/browse/MESOS-7446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the relevant ACL's for doing AuthZ (any or none
> access).
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9109283d81a3d25332fd74234099b5e6c9264c06 
>   include/mesos/authorizer/authorizer.proto 
> 38f0e0b48e2a243a594b5d81bddb2ce6fe4cf6bd 
>   src/authorizer/local/authorizer.cpp 
> 82ae846fd39de64704f0e8bd0fe2972f3750d2e6 
>   src/master/http.cpp 28d0393fb5962df4d731521265efd81a54

Re: Review Request 59019: Updated the documentation for '--docker_registry' option.

[Gilbert Song]

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


Fix it, then Ship it!




could you also fix the doc at slave/flag.cpp?


docs/configuration.md
Line 1360 (original), 1360 (patched)


align the length of this line?


- Gilbert Song


On May 4, 2017, 4:26 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59019/
> ---
> 
> (Updated May 4, 2017, 4:26 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the documentation for '--docker_registry' option.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 79cada3c9403881bf257d653f721d32e55607a7f 
> 
> 
> Diff: https://reviews.apache.org/r/59019/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62381: Removed `docker exec` when performing health checks in docker executor.

[Andrei Budnik]

> On Sept. 19, 2017, 11:13 p.m., Gastón Kleiman wrote:
> > src/tests/health_check_tests.cpp
> > Lines 1121 (patched)
> > 
> >
> > I think that we have to use `TEST_F_TEMP_DISABLED_ON_WINDOWS` here.

This test passes on Windows. Is it really necessary to use 
`TEST_F_TEMP_DISABLED_ON_WINDOWS`?


- Andrei


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


On Sept. 18, 2017, 5:21 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62381/
> ---
> 
> (Updated Sept. 18, 2017, 5:21 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gastón 
> Kleiman, haosdent huang, and Lukas Loesche.
> 
> 
> Bugs: MESOS-4812
> https://issues.apache.org/jira/browse/MESOS-4812
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos can enter namespaces via `setns` for launched subprocesses, hence
> we got rid of `docker exec` for command health checks. This change
> leads to more stable command health checks by making them independent
> from docker daemon that might hang. Also, this commit fixes the issue
> with incorrect escaping of quote characters in command health checks.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/tests/health_check_tests.cpp f4b50b1cb505084f64bf2dd279d9189ca65c8cdc 
> 
> 
> Diff: https://reviews.apache.org/r/62381/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

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

[Greg Mann]

---
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 (updated)
-

  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/

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


Testing
---


Thanks,

Greg Mann

Review Request 62548: Reorganized and updated the contribution guidelines.

[Greg Mann]

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

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/1/


Testing
---


Thanks,

Greg Mann

Re: Review Request 62518: Fixed default executor handling of nested container status.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62518 was successfully built and tested.

Reviews applied: `['62518']`

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

- Mesos Reviewbot Windows


On Sept. 25, 2017, 3:21 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62518/
> ---
> 
> (Updated Sept. 25, 2017, 3:21 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default executor was not handling a missing nested container
> exit status correctly. It was assuming the protobuf accessor was
> returning an Option rather than explicitly checking whether the
> `exit_status` field was present in the message.
> 
> Added the explicit check for the `exit_status` field, and always
> propagated an appropriate message into the status update, even
> when the `exit_status` is absent. Added some documentation of
> the `exit_status` field to the protobuf definition files.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
>   include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 
>   src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
> 
> 
> Diff: https://reviews.apache.org/r/62518/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 62214: Added JavaScript linter.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62214 was successfully built and tested.

Reviews applied: `['62333', '62214']`

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

- Mesos Reviewbot Windows


On Sept. 25, 2017, 2:56 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Sept. 25, 2017, 2:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This linter runs when changes on a JavaScript file are being committed.
> The linter used is ESLint with a configuration based on our current JS
> code base. The linter and its dependencies (i.e. Node.js) are installed
> in a environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/3/
> 
> 
> Testing
> ---
> 
> Following this commit, I have tried to commit a change on a JavaScript file 
> and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62518: Fixed default executor handling of nested container status.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [62518]

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

- Mesos Reviewbot


On Sept. 25, 2017, 8:51 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62518/
> ---
> 
> (Updated Sept. 25, 2017, 8:51 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default executor was not handling a missing nested container
> exit status correctly. It was assuming the protobuf accessor was
> returning an Option rather than explicitly checking whether the
> `exit_status` field was present in the message.
> 
> Added the explicit check for the `exit_status` field, and always
> propagated an appropriate message into the status update, even
> when the `exit_status` is absent. Added some documentation of
> the `exit_status` field to the protobuf definition files.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
>   include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 
>   src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
> 
> 
> Diff: https://reviews.apache.org/r/62518/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 62518: Fixed default executor handling of nested container status.

[James Peach]

> On Sept. 25, 2017, 12:42 a.m., Qian Zhang wrote:
> > Why do we need to change those .py files? I do not see you mentioned it in 
> > the commit message.

AFAICT `post-reviews` gets confused if you `git fetch` without doing a `git 
pull` and you end up including spurious changes. Removed them.


- James


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


On Sept. 22, 2017, 11:22 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62518/
> ---
> 
> (Updated Sept. 22, 2017, 11:22 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default executor was not handling a missing nested container
> exit status correctly. It was assuming the protobuf accessor was
> returning an Option rather than explicitly checking whether the
> `exit_status` field was present in the message.
> 
> Added the explicit check for the `exit_status` field, and always
> propagated an appropriate message into the status update, even
> when the `exit_status` is absent. Added some documentation of
> the `exit_status` field to the protobuf definition files.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
>   include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 
>   src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
> 
> 
> Diff: https://reviews.apache.org/r/62518/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 62518: Fixed default executor handling of nested container status.

[James Peach]

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

(Updated Sept. 25, 2017, 3:21 p.m.)


Review request for mesos, Andrei Budnik and Anand Mazumdar.


Repository: mesos


Description
---

The default executor was not handling a missing nested container
exit status correctly. It was assuming the protobuf accessor was
returning an Option rather than explicitly checking whether the
`exit_status` field was present in the message.

Added the explicit check for the `exit_status` field, and always
propagated an appropriate message into the status update, even
when the `exit_status` is absent. Added some documentation of
the `exit_status` field to the protobuf definition files.


Diffs (updated)
-

  include/mesos/agent/agent.proto 7c8c8a7d8298e91e4e002327b3b27d4c74b5cbae 
  include/mesos/v1/agent/agent.proto 3e199124b23fa027232790d99370fe2f33660096 
  src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 


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

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


Testing
---

make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 62214: Added JavaScript linter.

[Armand Grillet]

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

(Updated Sept. 25, 2017, 2:56 p.m.)


Review request for mesos, Benjamin Mahler and Kevin Klues.


Changes
---

Improved naming.


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


Repository: mesos


Description
---

This linter runs when changes on a JavaScript file are being committed.
The linter used is ESLint with a configuration based on our current JS
code base. The linter and its dependencies (i.e. Node.js) are installed
in a environment using Virtualenv and then Nodeenv.


Diffs (updated)
-

  src/webui/.eslintrc.js PRE-CREATION 
  src/webui/.gitignore PRE-CREATION 
  src/webui/bootstrap PRE-CREATION 
  src/webui/pip-requirements.txt PRE-CREATION 
  support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 


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

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


Testing
---

Following this commit, I have tried to commit a change on a JavaScript file and 
checked that ESLinter was correctly running.


Thanks,

Armand Grillet

Re: Review Request 62333: Added class for linters using a virtual environment.

[Armand Grillet]

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

(Updated Sept. 25, 2017, 2:55 p.m.)


Review request for mesos, Benjamin Mahler and Kevin Klues.


Changes
---

Resolved issues.


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


Repository: mesos


Description (updated)
---

This is used by the Python linter and
will be used by the Javascript linter.


Diffs (updated)
-

  support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 


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

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


Testing
---

Manual updates of `.py` and `.js` files then test commits to see if the linters 
before and afer removing their `.virtualenv` were still working as expected.


Thanks,

Armand Grillet

Re: Review Request 62333: Added class for linters using a virtual environment.

[Armand Grillet]

> On Sept. 19, 2017, 10:35 p.m., Benjamin Mahler wrote:
> > support/mesos-style.py
> > Lines 253-254 (patched)
> > 
> >
> > Is this clear to you? I don't understand why there is this distinction. 
> > My intuition would tell me that we don't need to rebuild the virtualenv if 
> > it's already built, but of course this code seems to suggest we always 
> > rebuild for a full lint?

After checking the history of the file, the logic behind `file_list` can be 
found in this commit: 
https://github.com/apache/mesos/commit/2e55afb35a912d0b6f8968bfb34832e645cb7903.
 Commit message:
```
The python linter needs to be run in a virtualenv due to how
it requires multiple modules that may or may not be installed
(or the correct version) in the host environment.  We originally
re-made this virtualenv whenever some specific files were
changed (such as the pip-requirements.txt, which describes
the virtualenv's dependencies).

However, this logic was missing the case where we ran
support/mesos-style.py with no arguments.  In this case, the
entire codebase is linted.  Therefore, the virtualenv should
also be rebuilt in this case.
```


- Armand


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


On Sept. 25, 2017, 2:55 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62333/
> ---
> 
> (Updated Sept. 25, 2017, 2:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used by the Python linter and
> will be used by the Javascript linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62333/diff/2/
> 
> 
> Testing
> ---
> 
> Manual updates of `.py` and `.js` files then test commits to see if the 
> linters before and afer removing their `.virtualenv` were still working as 
> expected.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

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

[Armand Grillet]

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

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


Review request for mesos and Kevin Klues.


Changes
---

Resolved issues.


Summary (updated)
-

Added CLI utility functions 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/4/

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


Testing
---

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


Thanks,

Armand Grillet

Re: Review Request 61211: Added default configuration file for CLI tests.

[Armand Grillet]

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

(Updated Sept. 25, 2017, 2:03 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Updated `default_config.toml`.


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


Repository: mesos


Description
---

This will be used by future tests, it overwrites
the configuration file defined by the user.


Diffs (updated)
-

  src/python/cli_new/tests/default_config.toml PRE-CREATION 
  src/python/cli_new/tests/main.py 3e4d2e449a6485206700b4a490d325a393d31f90 


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

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


Testing
---

```
$ ./bootstrap
$ source activate
$ mesos-cli-tests
```

I also checked that the Python linter was still working.


Thanks,

Armand Grillet

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

[Armand Grillet]

---
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.


Changes
---

Updated `master()` function.


Summary (updated)
-

CLI: Added 'master' key as an acceptable key in config.toml.


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


Repository: mesos


Description (updated)
---

This key is a field that has to be composed of
an `address` or `zookeeper` field, but not both.


Diffs (updated)
-

  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/

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


Testing
---

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


Thanks,

Armand Grillet

Review Request 62544: Updated pylint to increase the maximum number of branches in a function.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

Without this change, pylint will complain when a function is checking
for too many errors in the flags or configuration file.


Diffs
-

  src/python/pylint.config c76036441784543eb9025d0bd562a5628c4bad4e 


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


Testing
---


Thanks,

Armand Grillet

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

[Jan Schlicht]

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


Fix it, then Ship it!




Look great! Only found some nits.


src/Makefile.am
Lines 1140 (patched)


Move this below `resource_provider/registry.proto` but before 
`resource_provider/storage/provider.hpp` to follow the sorting rules in 
`Makefile.am`.



src/resource_provider/registrar.cpp
Lines 20 (patched)


Put this before ``.



src/resource_provider/registrar.cpp
Lines 138-149 (patched)


Any reason why these functions are implemented here? I'd rather have them 
move above the constructor of `AdmitResourceProvider` to be consistent and 
follow the order of classes in the header.


- Jan Schlicht


On Sept. 25, 2017, 12:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61528/
> ---
> 
> (Updated Sept. 25, 2017, 12:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7555
> https://issues.apache.org/jira/browse/MESOS-7555
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a registry and registrar interface for resource
> provider managers. The registrar interface is modelled after the
> master registrar and supports similar operations. Currently a single,
> LevelDB-backed registrar is implemented which we plan to use for
> resource provider managers in agents.
> 
> Current the registry allows to add and remove resource provider IDs.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/resource_provider/registry.hpp PRE-CREATION 
>   src/resource_provider/registry.proto PRE-CREATION 
>   src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
>   src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/61528/diff/11/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Jan Schlicht]

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 6619 (patched)


This is missing a `return;`.



src/slave/slave.cpp
Lines 6620 (patched)


Remove this line.


- Jan Schlicht


On Sept. 25, 2017, 12:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Sept. 25, 2017, 12:28 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> This patch also adjusts a number of tests to no expect two
> 'UpdateSlaveMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 
>   src/tests/oversubscription_tests.cpp 
> 02b10d6689dd6f01510cd2d5db2bb76b4b190eca 
>   src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/12/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 62285: Added tombstone flag to NOP log action.

[Ilya Pronin]

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

(Updated Sept. 25, 2017, 2:05 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

NOP action is used as a filler for holes in the "fill" procedure and as
the action in a response to a promise request for a truncated position.
The tombstone flag is introduced so that replicas are able to
distinguish the truncated action from a real NOP.


Diffs (updated)
-

  src/log/leveldb.cpp 5310a123b0fb25f240429722b676fe46174cb2ce 
  src/log/replica.cpp 39f2879b2e37c1ca9a9f987ce0a3b83e8dbc9b43 
  src/messages/log.proto ca740bd011147f8f48cc5dfcacefa5fdbd95ccd0 
  src/tests/log_tests.cpp f9f9400c901152779ae0ebfe74cf8f7aac1d3396 


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

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


Testing
---

Added a test that verifies that a replica correctly handles the tombstone NOP. 
Ran `make check`.


Thanks,

Ilya Pronin

Re: Review Request 62285: Added tombstone flag to NOP log action.

[Ilya Pronin]

> On Sept. 25, 2017, 5:12 a.m., Jie Yu wrote:
> > src/log/leveldb.cpp
> > Lines 247 (patched)
> > 
> >
> > I would combine this condition with the condition in `else if` above.

Done!


> On Sept. 25, 2017, 5:12 a.m., Jie Yu wrote:
> > src/log/leveldb.cpp
> > Lines 364-366 (patched)
> > 
> >
> > Can you explain this a bit more. When I see the code in `restore` 
> > above, I thought the invariant you want to maintain is: there is no 
> > tombstone entry in the replica's [begin,end]. Why the recovery code needs 
> > to see this position?

Yes, there should not be tombstones in replica's `[begin, end]` and the replica 
should recover the same `begin` position as it had when it terminated. While 
the latter is not technically required, I think it is nice to follow the 
principle of least surprise (especially when it doesn't require much effort): 
the replica has already learned that `begin > 0`, why does it recover with 
`begin = 0`?

Currently, `LevelDBStorage` recovery code calculates replica's `begin` position 
like this (C++ish pseudocode):
```c++
begin = 0;
for (const auto& action : actions)
  if (action.type() == TRUNCATE)
begin = max(begin, action.truncate().to());
  else if (action.type() == NOP)
begin = max(begin, action.position() + 1);
```
If it doesn't see a tombstone or a `TRUNCATE` action, it will recover `begin = 
0`.

We can modify the recovery code to recover `begin` as the first position that 
we read from the disk. But we still need to persist the tombstone, because 
currently LevelDB truncations are best-effort.


> On Sept. 25, 2017, 5:12 a.m., Jie Yu wrote:
> > src/log/replica.cpp
> > Lines 735-736 (patched)
> > 
> >
> > Ditto on combining considtions into a single `else if`

Done!


- Ilya


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


On Sept. 13, 2017, 5:40 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62285/
> ---
> 
> (Updated Sept. 13, 2017, 5:40 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7973
> https://issues.apache.org/jira/browse/MESOS-7973
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> NOP action is used as a filler for holes in the "fill" procedure and as
> the action in a response to a promise request for a truncated position.
> The tombstone flag is introduced so that replicas are able to
> distinguish the truncated action from a real NOP.
> 
> 
> Diffs
> -
> 
>   src/log/leveldb.cpp 5310a123b0fb25f240429722b676fe46174cb2ce 
>   src/log/replica.cpp 39f2879b2e37c1ca9a9f987ce0a3b83e8dbc9b43 
>   src/messages/log.proto ca740bd011147f8f48cc5dfcacefa5fdbd95ccd0 
>   src/tests/log_tests.cpp f9f9400c901152779ae0ebfe74cf8f7aac1d3396 
> 
> 
> Diff: https://reviews.apache.org/r/62285/diff/1/
> 
> 
> Testing
> ---
> 
> Added a test that verifies that a replica correctly handles the tombstone 
> NOP. Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 62438: Ignored redundant agent resources updates in master.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Sept. 22, 2017, 5:03 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62438/
> ---
> 
> (Updated Sept. 22, 2017, 5:03 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the future, agents will send updates on their total, e.g., when
> resource providers are added or removed. As an update to the agent's
> total resources currently triggers rescinding of all offered agent
> resources, spurious updates can negatively affect in-flight offer
> operations.
> 
> This patch changes the master so that updates introducing no changes
> (i.e., the new resources are identical to the old resources) are
> dropped and do not trigger rescinding of offers anymore. We also
> adjust the handling of oversubscribed agent resources to drop
> redundant updates.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
> 
> 
> Diff: https://reviews.apache.org/r/62438/diff/2/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 62158: Rescinded offers possibly affected by updates to agent total resources.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Sept. 20, 2017, 4:24 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62158/
> ---
> 
> (Updated Sept. 20, 2017, 4:24 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-7757
> https://issues.apache.org/jira/browse/MESOS-7757
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When an agent changes its resources, the master should rescind any
> offers affected by the change. We already performed the rescind for
> updates to the agent's oversubscribed resources; this patch adds offer
> rescinding when an update an agent's total resources is processed.
> 
> While for updates to an agent's oversubscribed resources we currently
> only rescind offers containing revocable resources to e.g., reduce
> offer churn, we for updates to the total we here currently rescind all
> offers for resources on the agent.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
> 
> 
> Diff: https://reviews.apache.org/r/62158/diff/4/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 62353: Added a master-registry backed resource provider manager registry.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 62282.

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

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

Relevant logs:

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

```
error: patch failed: src/common/resources.cpp:174
error: src/common/resources.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 25, 2017, 12:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62353/
> ---
> 
> (Updated Sept. 25, 2017, 12:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds an implementation of the resource provider registrar
> backed by the master's registrar. With that it becomes possible to
> persist resource provider manager state in a single master registrar,
> but use the narrowly defined resource provider registry.
> 
> 
> Diffs
> -
> 
>   src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
>   src/resource_provider/registrar.hpp PRE-CREATION 
>   src/resource_provider/registrar.cpp PRE-CREATION 
>   src/tests/resource_provider_manager_tests.cpp 
> 3bc56b51526e9dd188423f7349e74246c3295c77 
> 
> 
> Diff: https://reviews.apache.org/r/62353/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 61528: Implemented a registrar for resource provider manager state.

[Benjamin Bannier]

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

(Updated Sept. 25, 2017, 12:27 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds a registry and registrar interface for resource
provider managers. The registrar interface is modelled after the
master registrar and supports similar operations. Currently a single,
LevelDB-backed registrar is implemented which we plan to use for
resource provider managers in agents.

Current the registry allows to add and remove resource provider IDs.


Diffs (updated)
-

  src/CMakeLists.txt 1a828c4351ded36f51ccbbe67147da2f50b9cdb1 
  src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/resource_provider/registry.hpp PRE-CREATION 
  src/resource_provider/registry.proto PRE-CREATION 
  src/slave/paths.hpp d021e6b85a2e5823ea088d65faf9cd85cfb57e28 
  src/slave/paths.cpp e8724bf3e382211b5882728b86575de75981307f 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


Diff: https://reviews.apache.org/r/61528/diff/11/

Changes: https://reviews.apache.org/r/61528/diff/10-11/


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Benjamin Bannier]

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

(Updated Sept. 25, 2017, 12:28 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Repository: mesos


Description
---

The agent's resource provider manager sends a
'ResourceProviderMessage' when its managed resources change. This
commit adds handling in the agent so that an 'UpdateSlaveMessage' is
sent to the master to update the total resource available on the
agent.

In order to provide push-like handling of the resource provider
manager's message queue, we chain recursive calls to the handler for
continuous processing. Initially, processing is kicked off from
'Slave::initialize'. In this simple implementation we e.g., provide no
direct way to stop processing of messages, yet, but it can be achieved
by e.g., replacing the manager with a new instance (this would also
require updating routes).

Since the agent can only send an 'UpdateSlaveMessage' when it is
registered with a master, a simple back-off of 5 s is implemented which
will defer processing of a ready message should the agent not yet have
registered.

To facilitate logging we add a stringification function for
'ResourceProviderMessage's.

This patch also adjusts a number of tests to no expect two
'UpdateSlaveMessage's.


Diffs (updated)
-

  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 
  src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca 
  src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 


Diff: https://reviews.apache.org/r/61183/diff/12/

Changes: https://reviews.apache.org/r/61183/diff/11-12/


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 62353: Added a master-registry backed resource provider manager registry.

[Benjamin Bannier]

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

(Updated Sept. 25, 2017, 12:27 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Repository: mesos


Description
---

This patch adds an implementation of the resource provider registrar
backed by the master's registrar. With that it becomes possible to
persist resource provider manager state in a single master registrar,
but use the narrowly defined resource provider registry.


Diffs (updated)
-

  src/master/registry.proto 362a9fab946e9fb0411df2277f7f3edbadccb61a 
  src/resource_provider/registrar.hpp PRE-CREATION 
  src/resource_provider/registrar.cpp PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
3bc56b51526e9dd188423f7349e74246c3295c77 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 62531: Added authorization for 'MARK_AGENT_GONE' call.

[Mesos Reviewbot]

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



Patch looks great!

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

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

- Mesos Reviewbot


On Sept. 24, 2017, 10:22 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62531/
> ---
> 
> (Updated Sept. 24, 2017, 10:22 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7446
> https://issues.apache.org/jira/browse/MESOS-7446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the relevant ACL's for doing AuthZ (any or none
> access).
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9109283d81a3d25332fd74234099b5e6c9264c06 
>   include/mesos/authorizer/authorizer.proto 
> 38f0e0b48e2a243a594b5d81bddb2ce6fe4cf6bd 
>   src/authorizer/local/authorizer.cpp 
> 82ae846fd39de64704f0e8bd0fe2972f3750d2e6 
>   src/master/http.cpp 28d0393fb5962df4d731521265efd81a54e1e655 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/tests/api_tests.cpp d260a1c9560e8ff6b46eea7f2f4ddb11e18653e3 
> 
> 
> Diff: https://reviews.apache.org/r/62531/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62531: Added authorization for 'MARK_AGENT_GONE' call.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

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

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/62531

Relevant logs:

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

```
[ RUN  ] ContentType/MasterAPITest.CreateAndDestroyVolumes/0
[   OK ] ContentType/MasterAPITest.CreateAndDestroyVolumes/0 (963 ms)
[ RUN  ] ContentType/MasterAPITest.CreateAndDestroyVolumes/1
[   OK ] ContentType/MasterAPITest.CreateAndDestroyVolumes/1 (1110 ms)
[ RUN  ] ContentType/MasterAPITest.GetWeights/0
[   OK ] ContentType/MasterAPITest.GetWeights/0 (249 ms)
[ RUN  ] ContentType/MasterAPITest.GetWeights/1
[   OK ] ContentType/MasterAPITest.GetWeights/1 (234 ms)
[ RUN  ] ContentType/MasterAPITest.UpdateWeights/0
[   OK ] ContentType/MasterAPITest.UpdateWeights/0 (289 ms)
[ RUN  ] ContentType/MasterAPITest.UpdateWeights/1
[   OK ] ContentType/MasterAPITest.UpdateWeights/1 (301 ms)
[ RUN  ] ContentType/MasterAPITest.ReadFile/0
[   OK ] ContentType/MasterAPITest.ReadFile/0 (327 ms)
[ RUN  ] ContentType/MasterAPITest.ReadFile/1
[   OK ] ContentType/MasterAPITest.ReadFile/1 (337 ms)
[ RUN  ] ContentType/MasterAPITest.ReadFileInvalidPath/0
[   OK ] ContentType/MasterAPITest.ReadFileInvalidPath/0 (211 ms)
[ RUN  ] ContentType/MasterAPITest.ReadFileInvalidPath/1
[   OK ] ContentType/MasterAPITest.ReadFileInvalidPath/1 (216 ms)
[ RUN  ] ContentType/MasterAPITest.Teardown/0
[   OK ] ContentType/MasterAPITest.Teardown/0 (939 ms)
[ RUN  ] ContentType/MasterAPITest.Teardown/1
[   OK ] ContentType/MasterAPITest.Teardown/1 (1091 ms)
[ RUN  ] ContentType/MasterAPITest.MarkRegisteredAgentGone/0

C:\mesos\mesos\3rdparty\libprocess\include\process/gmock.hpp(235): ERROR: this 
mock object (used in test ContentType/MasterAPITest.MarkRegisteredAgentGone/0) 
should be deleted but never is. Its address is @026D5BF22CF8.
C:\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object (used 
in test ContentType/MasterAPITest.MarkRegisteredAgentGone/0) should be deleted 
but never is. Its address is @026D5D295500.
ERROR: 2 leaked mock objects found at program exit.
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62531/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: 54003


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

- Mesos Reviewbot Windows


On Sept. 25, 2017, 7:22 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62531/
> ---
> 
> (Updated Sept. 25, 2017, 7:22 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7446
> https://issues.apache.org/jira/browse/MESOS-7446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the relevant ACL's for doing AuthZ (any or none
> access).
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9109283d81a3d25332fd74234099b5e6c9264c06 
>   include/mesos/authorizer/authorizer.proto 
> 38f0e0b48e2a243a594b5d81bddb2ce6fe4cf6bd 
>   src/authorizer/local/authorizer.cpp 
> 82ae846fd39de64704f0e8bd0fe2972f3750d2e6 
>   src/master/http.cpp 28d0393fb5962df4d731521265efd81a54e1e655 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/tests/api_tests.cpp d260a1c9560e8ff6b46eea7f2f4ddb11e18653e3 
> 
> 
> Diff: https://reviews.apache.org/r/62531/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>