Re: Review Request 65566: Returned profiles based on provider selectors in UriDiskProfileAdaptor.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65566 was successfully built and tested.

Reviews applied: `['65553', '65554', '65538', '65558', '65559', '65566']`

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

- Mesos Reviewbot Windows


On Feb. 8, 2018, 6:39 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65566/
> ---
> 
> (Updated Feb. 8, 2018, 6:39 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8510
> https://issues.apache.org/jira/browse/MESOS-8510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Now the URI disk profile adaptor module will return the set of profiles
> in which each profile is either known to a storage resource provider or
> applies to it (based on the resource provider selector) when it watches
> for profiles.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile.hpp 
> 2f4fc7cededeb41afe329e1315569e097b1a60f9 
>   src/resource_provider/storage/uri_disk_profile.cpp 
> 7372b290d4fc3e45dc83fb3dd759150d08b7541f 
> 
> 
> Diff: https://reviews.apache.org/r/65566/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 65566: Returned profiles based on provider selectors in UriDiskProfileAdaptor.

[Chun-Hung Hsiao]

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

Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Now the URI disk profile adaptor module will return the set of profiles
in which each profile is either known to a storage resource provider or
applies to it (based on the resource provider selector) when it watches
for profiles.


Diffs
-

  src/resource_provider/storage/uri_disk_profile.hpp 
2f4fc7cededeb41afe329e1315569e097b1a60f9 
  src/resource_provider/storage/uri_disk_profile.cpp 
7372b290d4fc3e45dc83fb3dd759150d08b7541f 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65559: Updated tests for resource provider selector support.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, 6:38 a.m.)


Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


Summary (updated)
-

Updated tests for resource provider selector support.


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


Repository: mesos


Description (updated)
---

Updated tests for resource provider selector support.


Diffs (updated)
-

  src/tests/disk_profile_tests.cpp b6b35ef3641b2a5b59ee2190223e1e75ef3abaaf 
  src/tests/storage_local_resource_provider_tests.cpp 
f3903089f626a0ac72268424699f642e2df32e6c 


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

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


Testing
---

sudo make check

NOTE: make check will fail for r65558 without this patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65559: Updated disk profile tests for resource provider selector support.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65559 was successfully built and tested.

Reviews applied: `['65553', '65554', '65538', '65558', '65559']`

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

- Mesos Reviewbot Windows


On Feb. 8, 2018, 4:38 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65559/
> ---
> 
> (Updated Feb. 8, 2018, 4:38 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8510
> https://issues.apache.org/jira/browse/MESOS-8510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated disk profile tests for profile selector support.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_profile_tests.cpp b6b35ef3641b2a5b59ee2190223e1e75ef3abaaf 
> 
> 
> Diff: https://reviews.apache.org/r/65559/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> NOTE: make check will fail for r65558 without this patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65559: Updated disk profile tests for profile selector support.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, 4:38 a.m.)


Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Updated disk profile tests for profile selector support.


Diffs (updated)
-

  src/tests/disk_profile_tests.cpp b6b35ef3641b2a5b59ee2190223e1e75ef3abaaf 


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

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


Testing
---

sudo make check

NOTE: make check will fail for r65558 without this patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65558: Added helpers for supporting profile selectors.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, 4:37 a.m.)


Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

This patches validates that a profile's manifest must have a selector
and adds a helper function for selecting resource providers.


Diffs (updated)
-

  src/resource_provider/storage/disk_profile_utils.hpp 
fa2c7ec5055577b12e5e795064bf848b9a887def 
  src/resource_provider/storage/disk_profile_utils.cpp 
c84b6e18665086968808fb5cb0a313c1050ea587 


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

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


Testing
---

Tests done later in chain.

NOTE: The next patch in chain only include necessary changes for the validation 
tests. Tests for the helper function will be added in a separated patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65538: Added resource provider selectors in `disk_profile.proto`.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, 4:36 a.m.)


Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


Summary (updated)
-

Added resource provider selectors in `disk_profile.proto`.


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


Repository: mesos


Description
---

This patch adds two selectors to the `CSIManifest` protobuf so that the
URI disk profile adaptor can be customized to notify each resource
provider with a different set of profiles.


Diffs (updated)
-

  src/resource_provider/storage/disk_profile.proto 
6cf1f8abcd24e45a292efc95a395f90bb2140da2 


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

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


Testing
---

Tests done later in chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65316: Added test for delayed authorization during operator events.

[Greg Mann]

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

(Updated Feb. 8, 2018, 2:26 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, and Vinod Kone.


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


Repository: mesos


Description
---

Until the fix for MESOS-8469, it was possible for the master
operator event stream to drop events, if event-related state in
the master changed in between asynchronous calls.

This patch adds `MasterAPITest.EventAuthorizationDelayed` to
verify the fix for that issue.


Diffs
-

  src/tests/api_tests.cpp b639a4bafbf69d3ea53aa0573e9f0489c3a1a57f 


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


Testing
---

Confirmed that the test fails without the fix for MESOS-8469.

When the fix is included, the test passes. Successfully ran `bin/mesos-tests.sh 
--gtest_filter="ContentType/MasterAPITest.EventAuthorizationDelayed*" 
--gtest_repeat=1000 --gtest_break_on_failure` to rule out any flakiness.


Thanks,

Greg Mann

Re: Review Request 65561: WIP: Added more type util tests.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65540', '65541', '65561']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[ RUN  ] ResourcesOperationTest.FlattenResources
[   OK ] ResourcesOperationTest.FlattenResources (1 ms)
[--] 12 tests from ResourcesOperationTest (27 ms total)

[--] 6 tests from RevocableResourceTest
[ RUN  ] RevocableResourceTest.Equals
[   OK ] RevocableResourceTest.Equals (1 ms)
[ RUN  ] RevocableResourceTest.Addition
[   OK ] RevocableResourceTest.Addition (1 ms)
[ RUN  ] RevocableResourceTest.Subtraction
[   OK ] RevocableResourceTest.Subtraction (0 ms)
[ RUN  ] RevocableResourceTest.Contains
[   OK ] RevocableResourceTest.Contains (1 ms)
[ RUN  ] RevocableResourceTest.Filter
[   OK ] RevocableResourceTest.Filter (1 ms)
[ RUN  ] RevocableResourceTest.Find
[   OK ] RevocableResourceTest.Find (1 ms)
[--] 6 tests from RevocableResourceTest (9 ms total)

[--] 6 tests from ResourceFormatTest
[ RUN  ] ResourceFormatTest.PreReservationRefinement
[   OK ] ResourceFormatTest.PreReservationRefinement (0 ms)
[ RUN  ] ResourceFormatTest.PostReservationRefinement
[   OK ] ResourceFormatTest.PostReservationRefinement (0 ms)
[ RUN  ] ResourceFormatTest.Endpoint
[   OK ] ResourceFormatTest.Endpoint (1 ms)
[ RUN  ] ResourceFormatTest.DowngradeWithoutResources
[   OK ] ResourceFormatTest.DowngradeWithoutResources (1 ms)
[ RUN  ] ResourceFormatTest.DowngradeWithResourcesWithoutRefinedReservations
```

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

```
@   7FF706844521  google::LogMessageFatal::~LogMessageFatal
@   7FF7049B4C48  mesos::Resources::isEmpty
@   7FF7049B0568  mesos::Resources::Resource_::isEmpty
@   7FF7049BA97D  mesos::Resources::add
@   7FF7049BAD06  mesos::Resources::operator+=
@   7FF7049B9DF6  mesos::Resources::operator+=
@   7FF7049B6710  mesos::Resources::Resources
@   7FF703EBE110  mesos::MesosFieldComparator::Compare
@   7FF7071E1E43  
google::protobuf::util::MessageDifferencer::GetFieldComparisonResult
@   7FF7071E1B2A  
google::protobuf::util::MessageDifferencer::CompareFieldValueUsingParentFields
@   7FF7071E1612  
google::protobuf::util::MessageDifferencer::CompareRepeatedField
@   7FF7071E1153  
google::protobuf::util::MessageDifferencer::CompareWithFieldsInternal
@   7FF7071E0978  
google::protobuf::util::MessageDifferencer::CompareRequestedFieldsUsingSettings
@   7FF7071DF995  google::protobuf::util::MessageDifferencer::Compare
@   7FF7071DD661  google::protobuf::util::MessageDifferencer::Compare
@   7FF703EB88F3  mesos::operator==
@   7FF7038C7594  
testing::internal::CmpHelperEQ
@   7FF7038C9EC8  
testing::internal::EqHelper<0>::Compare
@   7FF7038A2C48  
mesos::internal::tests::ResourceFormatTest_DowngradeWithResourcesWithoutRefinedReservations_Test::TestBody
@   7FF70734FC61  
testing::internal::HandleSehExceptionsInMethodIfSupported
@   7FF70734F86D  
testing::internal::HandleExceptionsInMethodIfSupported
@   7FF70732E5C3  testing::Test::Run
@   7FF70732F1C6  testing::TestInfo::Run
@   7FF70732F976  testing::TestCase::Run
@   7FF707336B0B  testing::internal::UnitTestImpl::RunAllTests
@   7FF70734FED1  
testing::internal::HandleSehExceptionsInMethodIfSupported
@   7FF70734FB8D  
testing::internal::HandleExceptionsInMethodIfSupported
@   7FF707330075  testing::UnitTest::Run
@   7FF7028F9BE2  RUN_ALL_TESTS
```

- Mesos Reviewbot Windows


On Feb. 8, 2018, 1:36 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65561/
> ---
> 
> (Updated Feb. 8, 2018, 1:36 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-2487
> https://issues.apache.org/jira/browse/MESOS-2487
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These are meant 

Re: Review Request 65497: Added test to verify task-less executor is shutdown when re-subscribing.

[Benjamin Mahler]

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



Looks good, just a few minor comments.


src/tests/mock_slave.cpp
Lines 107-108 (original), 107-109 (patched)


What's going on here?



src/tests/slave_tests.cpp
Lines 4926-4927 (patched)


even after the exeutor has been registered..? I think you want to remove 
that?



src/tests/slave_tests.cpp
Lines 4945 (patched)


"NotSlave"?

We should probably have found a way to use the logic from 
Master::newSlaveId here.



src/tests/slave_tests.cpp
Lines 5032-5038 (patched)


Instead of using a slave mock here, can we just expect the executorLost 
signal from the scheduler?


- Benjamin Mahler


On Feb. 3, 2018, 1:18 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65497/
> ---
> 
> (Updated Feb. 3, 2018, 1:18 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8411
> https://issues.apache.org/jira/browse/MESOS-8411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the v1 executor is shutdown if all of its
> initial tasks could not be delivered when re-subscribing with
> a recovered agent. See MESOS-8411.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp a35c68e8645384f6244d17e37cad71373aba6893 
>   src/tests/mesos.cpp d751b2e9c635eb6a5039678de426467176cda908 
>   src/tests/mock_slave.cpp 8357edc7b3a35624c813eccb9ecca9d3b5dbe07c 
>   src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 
> 
> 
> Diff: https://reviews.apache.org/r/65497/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65369: Added test to ensure v1 executor is shutdown upon initial task all-kill.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Feb. 1, 2018, 9:24 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65369/
> ---
> 
> (Updated Feb. 1, 2018, 9:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the v1 executor is shutdown if all of its
> initial tasks could not be delivered, even after the executor has been
> subscribed. See MESOS-8411.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 
> 
> 
> Diff: https://reviews.apache.org/r/65369/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65110: Added two additional mock methods in mock slave.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Jan. 12, 2018, 2:34 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65110/
> ---
> 
> (Updated Jan. 12, 2018, 2:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two additional mock methods in mock slave.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
>   src/tests/mock_slave.hpp 29ce7140501888d95d5f2d6c26b752ad276b484a 
>   src/tests/mock_slave.cpp 8357edc7b3a35624c813eccb9ecca9d3b5dbe07c 
> 
> 
> Diff: https://reviews.apache.org/r/65110/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65496: Fixed a bug relating to lingering executors [2/2].

[Benjamin Mahler]

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


Ship it!




This description doesn't seem to capture the cause of the issue here? 
Mentioning something about the everSentTask check occurs before we've removed 
the dropped tasks would be helpful for posterity.

- Benjamin Mahler


On Feb. 6, 2018, 5:42 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65496/
> ---
> 
> (Updated Feb. 6, 2018, 5:42 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8411
> https://issues.apache.org/jira/browse/MESOS-8411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A task-less v1 executor could linger if the agent fails before
> any of the executor's initial tasks got delivered.
> 
> This patch fixes this issue by checking if an executor is
> task-less during the executor `subscribe()` and shutdown
> the executor accordingly.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
> 
> 
> Diff: https://reviews.apache.org/r/65496/diff/2/
> 
> 
> Testing
> ---
> 
> make check.
> Dedicated test #65497
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65109: Fixed a bug relating to lingering executors [1/2].

[Benjamin Mahler]

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


Ship it!





src/slave/slave.cpp
Lines 3361-3362 (patched)


// TODO(mzhu): Consider shutting down the executor here
  // if all of its initial tasks are killed rather than
  // waiting for it to register.


- Benjamin Mahler


On Feb. 6, 2018, 5:45 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65109/
> ---
> 
> (Updated Feb. 6, 2018, 5:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-8411
> https://issues.apache.org/jira/browse/MESOS-8411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An executor should be shutdown if all of its tasks are
> killed while the executor is launching.
> 
> This patch fixes and issue where the executor is left
> running when the task(s) get killed between the executor
> registration/subscription and `Slave::___run()`. See
> MESOS-8411 for more details. There is an additional race
> in the agent failover case that is addressed in this patch.
> 
> The fix here is to fix the race by checking an executor's various
> tasks queues during task kill and executor (re-)registration,
> and shutting down executors that had never received any tasks.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp e6cb7cc0ccdaaf981eb66defa21b38720f4e1de9 
>   src/slave/slave.hpp 09c01ebd1b5e8008ba9e7d412042f1db76a2c5a5 
>   src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
> 
> 
> Diff: https://reviews.apache.org/r/65109/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> new tests in #65111
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 65561: WIP: Added more type util tests.

[Joseph Wu]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

These are meant to exercise some rules defined by the
operator== refactor, but should also pass without the
refactor applied.


Diffs
-

  src/tests/common/type_utils_tests.cpp 
a962b397a2340694232ab13ea9eca7fff78d35e2 


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


Testing
---


Thanks,

Joseph Wu

Re: Review Request 65541: WIP: Experimented with the MessageDifferencer.

[Joseph Wu]

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

(Updated Feb. 7, 2018, 5:36 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Added some consideration for the `Resources` object, still has some edge cases 
that I haven't figured out how to work around yet (see a TODO).


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


Repository: mesos


Description
---

This is an attempt to use the MessageDifferencer class (introduced in 
protobuf 3.0) to replace our handwritten protobuf equality operators.

This patch currently:
  * Ignores style rules (mostly).
  * Has TODOs to deal with the Resources and Attributes classes,
which have their own wrappers and may need to be special-cased.
  * Only deals with non-versioned protobufs at the moment.
  * Can be made faster by making the MessageDifferencer into a
`static thread_local` object guarded by a Once.


Diffs (updated)
-

  include/mesos/type_utils.hpp af2b187b9b59552e4ba515ad640fd4419eaf5075 
  src/common/type_utils.cpp a4d5dcb4e4445e307356d9b0c16dd39f00f6a8e2 


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

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


Testing
---


Thanks,

Joseph Wu

Re: Review Request 65559: Updated disk profile tests for profile selector support.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65559 was successfully built and tested.

Reviews applied: `['65553', '65554', '65538', '65558', '65559']`

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

- Mesos Reviewbot Windows


On Feb. 8, 2018, 12:29 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65559/
> ---
> 
> (Updated Feb. 8, 2018, 12:29 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8510
> https://issues.apache.org/jira/browse/MESOS-8510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated disk profile tests for profile selector support.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_profile_tests.cpp b6b35ef3641b2a5b59ee2190223e1e75ef3abaaf 
> 
> 
> Diff: https://reviews.apache.org/r/65559/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> NOTE: make check will fail for r65558 without this patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 65559: Updated disk profile tests for profile selector support.

[Chun-Hung Hsiao]

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

Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Updated disk profile tests for profile selector support.


Diffs
-

  src/tests/disk_profile_tests.cpp b6b35ef3641b2a5b59ee2190223e1e75ef3abaaf 


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


Testing
---

sudo make check

NOTE: make check will fail for r65558 without this patch.


Thanks,

Chun-Hung Hsiao

Review Request 65558: Added helpers for supporting profile selectors.

[Chun-Hung Hsiao]

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

Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

This patches validates that a profile's manifest must have a selector
and adds a helper function for selecting resource providers.


Diffs
-

  src/resource_provider/storage/disk_profile_utils.hpp 
fa2c7ec5055577b12e5e795064bf848b9a887def 
  src/resource_provider/storage/disk_profile_utils.cpp 
c84b6e18665086968808fb5cb0a313c1050ea587 


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


Testing
---

Tests done later in chain.

NOTE: The next patch in chain only include necessary changes for the validation 
tests. Tests for the helper function will be added in a separated patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65538: WIP: Added profile selectors in `disk_profile.proto`.

[James DeFelice]

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


Ship it!




Ship It!

- James DeFelice


On Feb. 7, 2018, 9:07 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65538/
> ---
> 
> (Updated Feb. 7, 2018, 9:07 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8510
> https://issues.apache.org/jira/browse/MESOS-8510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds two selectors to the `CSIManifest` protobuf so that the
> URI disk profile adaptor can be customized to notify each resource
> provider with a different set of profiles.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/disk_profile.proto 
> 6cf1f8abcd24e45a292efc95a395f90bb2140da2 
> 
> 
> Diff: https://reviews.apache.org/r/65538/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65061: Speeded up tests for resource provider config agent API.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, 12:02 a.m.)


Review request for mesos, Greg Mann and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Speeded up tests for resource provider config agent API.


Diffs (updated)
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
7cbee5cceef2963b736f3e6ce3f52fcf0b3c029c 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65523: Fixed the flakiness in the SLRP metrics test.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, 12:01 a.m.)


Review request for mesos, Greg Mann and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

The metrics test unnecessarily set up the disk profile module without
providing the profile config file. This may cause the profile module to
pull a non-existent file. Also, it could kill the plugin before SLRP
connecting to the endpoint, making SLRP wait for 1 minute.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ce241617ee555e5a67e552dd30b0e7afee161386 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65499: Fixed the flakiness of the ROOT_ConvertPreExistingVolume test.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, midnight)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jie 
Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This unit test is flaky because the master might send out an offer
before both offer operations of a previous `ACCEPT` call are finished.
This is fixed by manipulating the clock such that no offer won't be
sent out until the master receives status updates for both operations.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ce241617ee555e5a67e552dd30b0e7afee161386 


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

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


Testing
---

sudo make check
Ran in repetition.


Thanks,

Chun-Hung Hsiao

Re: Review Request 65520: Reverted plugin name randomization in SLRP tests.

[Chun-Hung Hsiao]

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

(Updated Feb. 8, 2018, 12:01 a.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


Changes
---

Rebased.


Repository: mesos


Description
---

The plugin name is no longer randomized since it is no longer needed
for test isolation. Also removed an unnecessary member variable in the
SLRP test class.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ce241617ee555e5a67e552dd30b0e7afee161386 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65060: Cleaned up endpoint directories after SLRP tests.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 7, 2018, 6:37 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65060/
> ---
> 
> (Updated Feb. 7, 2018, 6:37 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8427
> https://issues.apache.org/jira/browse/MESOS-8427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up endpoint directories after SLRP tests.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ce241617ee555e5a67e552dd30b0e7afee161386 
> 
> 
> Diff: https://reviews.apache.org/r/65060/diff/7/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65062: Cleaned up endpoint directories after RP config agent API tests.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Feb. 7, 2018, 6:37 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65062/
> ---
> 
> (Updated Feb. 7, 2018, 6:37 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8427
> https://issues.apache.org/jira/browse/MESOS-8427
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Cleaned up endpoint directories after RP config agent API tests.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> 7cbee5cceef2963b736f3e6ce3f52fcf0b3c029c 
> 
> 
> Diff: https://reviews.apache.org/r/65062/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Greg Mann]

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

(Updated Feb. 7, 2018, 11:04 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
and Jie Yu.


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


Repository: mesos


Description (updated)
---

This patch adds
'StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation'
in order to verify that reconciliation is performed correctly
when an operation is dropped on its way from the master to the
agent.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ce241617ee555e5a67e552dd30b0e7afee161386 


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

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


Testing
---

sudo make check


Thanks,

Greg Mann

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: patch failed: src/tests/storage_local_resource_provider_tests.cpp:27
error: src/tests/storage_local_resource_provider_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Feb. 7, 2018, 3:04 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65039/
> ---
> 
> (Updated Feb. 7, 2018, 3:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
> and Jie Yu.
> 
> 
> Bugs: MESOS-8373
> https://issues.apache.org/jira/browse/MESOS-8373
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> 'StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation'
> in order to verify that reconciliation is performed correctly
> when an operation is dropped on its way from the master to the
> agent.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ce241617ee555e5a67e552dd30b0e7afee161386 
> 
> 
> Diff: https://reviews.apache.org/r/65039/diff/6/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 65039: Tested reconciliation when operation is dropped en route to agent.

[Greg Mann]

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

(Updated Feb. 7, 2018, 11:04 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, 
and Jie Yu.


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


Repository: mesos


Description (updated)
---

This patch adds
'StorageLocalResourceProviderTest.ROOT_ReconcileDroppedOperation'
in order to verify that reconciliation is performed correctly
when an operation is dropped on its way from the master to the
agent.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ce241617ee555e5a67e552dd30b0e7afee161386 


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

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


Testing
---

sudo make check


Thanks,

Greg Mann

Re: Review Request 65556: Made the default executor treat agent disconnections more gracefully.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65556 was successfully built and tested.

Reviews applied: `['65548', '65549', '65550', '65551', '65552', '65556']`

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

- Mesos Reviewbot Windows


On Feb. 7, 2018, 9:55 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65556/
> ---
> 
> (Updated Feb. 7, 2018, 9:55 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Qian Zhang, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-8468
> https://issues.apache.org/jira/browse/MESOS-8468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the default executor not shutdown if there it there are
> active child containers, and it fails to connect or is not subscribed to
> the agent when starting to launch a task group.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
> 
> 
> Diff: https://reviews.apache.org/r/65556/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65363: Improved the validation of `ACKNOWLEDGE_OPERATION_STATUS` calls.

[Greg Mann]

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


Fix it, then Ship it!





src/master/validation.cpp
Lines 614 (patched)


s/affcting/affecting/

I can fix this while committing.


- Greg Mann


On Feb. 1, 2018, 11:50 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65363/
> ---
> 
> (Updated Feb. 1, 2018, 11:50 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
> https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved the validation of `ACKNOWLEDGE_OPERATION_STATUS` calls.
> 
> 
> Diffs
> -
> 
>   src/master/validation.cpp f0b86775b7919ba6aa4a73038edb78a0adca68f4 
> 
> 
> Diff: https://reviews.apache.org/r/65363/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65362: Added a method to increment invalid scheduler API call counters.

[Gaston Kleiman]

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

(Updated Feb. 7, 2018, 10:18 p.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

Added a method to increment invalid scheduler API call counters.


Diffs
-

  src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990 
  src/master/master.cpp cc2685a6bc14103c639ce776cf1c912361e93381 
  src/master/metrics.hpp f701efec0a82d6ba72b2414f739fcd1cd7ee2491 
  src/master/metrics.cpp 64fc829ac3b58d95fc0bd074571a46518a80bbba 


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


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65362: Added a method to increment invalid scheduler API call counters.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Feb. 1, 2018, 11:48 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65362/
> ---
> 
> (Updated Feb. 1, 2018, 11:48 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8184
> https://issues.apache.org/jira/browse/MESOS-8184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a method to increment invalid scheduler API call counters.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990 
>   src/master/master.cpp cc2685a6bc14103c639ce776cf1c912361e93381 
>   src/master/metrics.hpp f701efec0a82d6ba72b2414f739fcd1cd7ee2491 
>   src/master/metrics.cpp 64fc829ac3b58d95fc0bd074571a46518a80bbba 
> 
> 
> Diff: https://reviews.apache.org/r/65362/diff/3/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65402: Windows: Used Unicode API to duplicate sockets.

[Andrew Schwartzmeyer]

> On Feb. 7, 2018, 12:49 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/os/windows/dup.hpp
> > Line 30 (original), 33 (patched)
> > 
> >
> > It's not a change for this patch, but we should probably look to make 
> > `os::dup` not inheritable by default and hunt down cases that we actually 
> > need inheritance.

Totally agree, even left a TODO for that.


- Andrew


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


On Feb. 7, 2018, 11:22 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65402/
> ---
> 
> (Updated Feb. 7, 2018, 11:22 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The CRT API for duplicating file handles is left as-is because
> `WindowsFD` currently gets a CRT `int` file descriptor, which
> necessitates using `_dup` instead of `DuplicateHandle`. Furthermore, the
> latter API does not appear compatible with handles used to redirect
> stdout/stderr/stdin for subprocesses.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> b37aaa6429e0e7b4340e86c078f5c543e443bdcd 
> 
> 
> Diff: https://reviews.apache.org/r/65402/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65522: Fixed some log messages in `slave.cpp`.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 65520.

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

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

Relevant logs:

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

```
error: patch failed: src/tests/storage_local_resource_provider_tests.cpp:73
error: src/tests/storage_local_resource_provider_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Feb. 7, 2018, 9:49 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65522/
> ---
> 
> (Updated Feb. 7, 2018, 9:49 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed some log messages in `slave.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
> 
> 
> Diff: https://reviews.apache.org/r/65522/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65538: WIP: Added profile selectors in `disk_profile.proto`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65538 was successfully built and tested.

Reviews applied: `['65553', '65554', '65538']`

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

- Mesos Reviewbot Windows


On Feb. 7, 2018, 1:07 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65538/
> ---
> 
> (Updated Feb. 7, 2018, 1:07 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8510
> https://issues.apache.org/jira/browse/MESOS-8510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds two selectors to the `CSIManifest` protobuf so that the
> URI disk profile adaptor can be customized to notify each resource
> provider with a different set of profiles.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/disk_profile.proto 
> 6cf1f8abcd24e45a292efc95a395f90bb2140da2 
> 
> 
> Diff: https://reviews.apache.org/r/65538/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 65556: Made the default executor treat agent disconnections more gracefully.

[Gaston Kleiman]

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

Review request for mesos, Anand Mazumdar, Greg Mann, Qian Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

This patch makes the default executor not shutdown if there it there are
active child containers, and it fails to connect or is not subscribed to
the agent when starting to launch a task group.


Diffs
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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


Testing
---

`sudo make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65522: Fixed some log messages in `slave.cpp`.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 9:49 p.m.)


Review request for mesos, Gaston Kleiman and Greg Mann.


Repository: mesos


Description
---

Fixed some log messages in `slave.cpp`.


Diffs (updated)
-

  src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65522: Fixed some log messages in `slave.cpp`.

[Chun-Hung Hsiao]

> On Feb. 6, 2018, 6:29 p.m., Gaston Kleiman wrote:
> > src/slave/slave.cpp
> > Line 9530 (original), 9529 (patched)
> > 
> >
> > I noticed that if a single task is passed, we surround its ID in single 
> > quotes, but we don't do that if a task groups is passed.
> > 
> > We might want to do this consistently.
> 
> Chun-Hung Hsiao wrote:
> Would you prefer iterating throuch each ID and qouting them, or add a 
> pair of brackets surrounding them?
> 
> Gaston Kleiman wrote:
> I'd vote for the former.

Reverted this change for now since not everyone agrees on this one.


- Chun-Hung


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


On Feb. 7, 2018, 6:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65522/
> ---
> 
> (Updated Feb. 7, 2018, 6:45 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed some log messages in `slave.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
> 
> 
> Diff: https://reviews.apache.org/r/65522/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65469: Windows: Made `IO::OWNED` file descriptors inheritable.

[Akash Gupta]

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




3rdparty/libprocess/src/subprocess.cpp
Lines 259 (patched)


Actually, should we enable inheritance in 
`internal::windows::create_process` and then immediately disable it after the 
proccess finshes initializing? That way, we keep the handle inheritable as 
little as possible to avoid leaks. I think `create_process` is the only case 
where you actually need inheritable handles.


- Akash Gupta


On Feb. 2, 2018, 8:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65469/
> ---
> 
> (Updated Feb. 2, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On Windows, we make all file handles non-inheritable by default.
> Therefore when creating a `Subprocess::FD` for a subprocess to inherit a
> file descriptor, we have to explicitly make it inheritable. This was
> already happening as a side-effect of `os::dup` for `IO::DUPLICATED`
> file descriptors, but not for `IO::OWNED`. Both, however, are meant to
> be inherited.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> 785e2e1083d115d25fffde2df74ed8bc1726511c 
> 
> 
> Diff: https://reviews.apache.org/r/65469/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65408: Windows: Ported `slave_recovery_tests.cpp`.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 2, 2018, 12:14 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65408/
> ---
> 
> (Updated Feb. 2, 2018, 12:14 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6713
> https://issues.apache.org/jira/browse/MESOS-6713
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit enables the unit tests found in `slave_recovery_tests.cpp`
> to test agent recovery on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 6f1c67a6b4003ada1a0635a68fb7471895a2a6f5 
>   src/tests/slave_recovery_tests.cpp 77aa60c953bd0769eaba05f001755e4cec9ba028 
> 
> 
> Diff: https://reviews.apache.org/r/65408/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65522: Fixed some log messages in `slave.cpp`.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 65520.

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

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

Relevant logs:

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

```
error: patch failed: src/tests/storage_local_resource_provider_tests.cpp:73
error: src/tests/storage_local_resource_provider_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Feb. 7, 2018, 6:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65522/
> ---
> 
> (Updated Feb. 7, 2018, 6:45 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed some log messages in `slave.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 
> 
> 
> Diff: https://reviews.apache.org/r/65522/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65538: WIP: Added profile selectors in `disk_profile.proto`.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 9:07 p.m.)


Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


Changes
---

Addressed Jie's comments.


Summary (updated)
-

WIP: Added profile selectors in `disk_profile.proto`.


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


Repository: mesos


Description (updated)
---

This patch adds two selectors to the `CSIManifest` protobuf so that the
URI disk profile adaptor can be customized to notify each resource
provider with a different set of profiles.


Diffs (updated)
-

  src/resource_provider/storage/disk_profile.proto 
6cf1f8abcd24e45a292efc95a395f90bb2140da2 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Review Request 65554: Updated disk profile tests due to changes in the module interface.

[Chun-Hung Hsiao]

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

Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Updated disk profile tests due to changes in the module interface.


Diffs
-

  src/tests/disk_profile_tests.cpp b6b35ef3641b2a5b59ee2190223e1e75ef3abaaf 


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


Testing
---

sudo make check

NOTE: make check will fail for r65553 without this patch.


Thanks,

Chun-Hung Hsiao

Review Request 65553: Passed `ResourceProviderInfo` to disk profile adaptor modules.

[Chun-Hung Hsiao]

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

Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Passed `ResourceProviderInfo` to disk profile adaptor modules.


Diffs
-

  include/mesos/resource_provider/storage/disk_profile.hpp 
d1a522ad23a7d8a53d5c57e9d4191e1e7652f0c8 
  src/resource_provider/storage/disk_profile.cpp 
053fb7ac0857cbd1c57e3d75f1445ac6f083852b 
  src/resource_provider/storage/provider.cpp 
604cadff20a6156639db10a28a180e53a959ff29 
  src/resource_provider/storage/uri_disk_profile.hpp 
2f4fc7cededeb41afe329e1315569e097b1a60f9 
  src/resource_provider/storage/uri_disk_profile.cpp 
7372b290d4fc3e45dc83fb3dd759150d08b7541f 


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


Testing
---

Tests done later in chain


Thanks,

Chun-Hung Hsiao

Re: Review Request 65407: Windows: Enabled tests that were blocked by MESOS-7604.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 2, 2018, 12:03 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65407/
> ---
> 
> (Updated Feb. 2, 2018, 12:03 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-7604
> https://issues.apache.org/jira/browse/MESOS-7604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit enables tests that were previously disabled on Windows due
> to MESOS-7604.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp b639a4bafbf69d3ea53aa0573e9f0489c3a1a57f 
>   src/tests/gc_tests.cpp 0e34c017f2c629b527915218f517b88d8392aa4b 
>   src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 
>   src/tests/slave_tests.cpp f393a8433a984267adc4db307ef07fcbafd1e62f 
> 
> 
> Diff: https://reviews.apache.org/r/65407/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65406: Added `TYPED_TEST_TEMP_DISABLED_ON_WINDOWS` macro.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Jan. 29, 2018, 8:39 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65406/
> ---
> 
> (Updated Jan. 29, 2018, 8:39 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This lets us temporarily disable `TYPED_TEST` unit tests on Windows but
> not elsewhere.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/gtest.hpp 
> a8ae3d90519aae7788e0874a22776cff682173ba 
> 
> 
> Diff: https://reviews.apache.org/r/65406/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62800 was successfully built and tested.

Reviews applied: `['62798', '62799', '62800']`

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

- Mesos Reviewbot Windows


On Feb. 7, 2018, 8:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Feb. 7, 2018, 8:01 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> c2e319894874e628beda6dc305462af0c274fd7b 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 65405: Implemented `net::socket()` for Windows using `WSASocket`.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 1, 2018, 11:30 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65405/
> ---
> 
> (Updated Feb. 1, 2018, 11:30 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5882
> https://issues.apache.org/jira/browse/MESOS-5882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than reusing the existing `net::socket()` wrapper for Windows,
> the original wrapper was moved to `posix/socket.hpp`, and a new wrapper
> was written in `windows/socket.hpp` which uses `::WSASocket()` instead
> of `::socket()` and takes an optional set of Windows socket flags.
> 
> We enable `WSA_FLAG_OVERLAPPED` so that the socket supports overlapped
> I/O operations (the default behavior of `::socket()`; MSDN states that
> most sockets should be created with this flag set, so it is a reasonable
> default.
> 
> Furthermore, we enable `WSA_FLAG_NO_HANDLE_INHERIT`, which provides the
> semantics of close-on-exec. This is set by default because (a) it is not
> possible to change afterwards, and (b) we never use inheritable sockets.
> Therefore it reasonable to also disable inheritance.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/socket.hpp 
> bab0b808f53abd1314a7d13fc0cba75e5717f96f 
>   3rdparty/stout/include/stout/os/socket.hpp 
> 2ca4465ca23c9ca59239947c9babf8dd0212fafd 
>   3rdparty/stout/include/stout/os/windows/socket.hpp 
> 020c5e2367217cd2b4439ae5e2d5be4b31b4226b 
> 
> 
> Diff: https://reviews.apache.org/r/65405/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65552: Added a regression test for MESOS-8468.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65548', '65549', '65550', '65551', '65552']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (102 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (1004 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (32 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (37 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (70 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2284 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2307 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2250 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2274 ms total)

[--] Global test environment tear-down
[==] 852 tests from 85 test cases ran. (309053 ms total)
[  PASSED  ] 851 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.ROOT_LaunchGroupFailure/0, 
where GetParam() = "mesos"

 1 FAILED TEST
  YOU HAVE 213 DISABLED TESTS

```

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

```
I0207 20:57:04.589033  5876 executor.cpp:171] Received SUBSCRIBED event
I0207 20:57:04.593070  5876 executor.cpp:175] Subscribed executor on 
build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0207 20:57:04.594066  5876 executor.cpp:171] Received LAUNCH event
I0207 20:57:04.598068  5876 executor.cpp:638] Starting task 
c6317f1b-f8c3-4e2e-b15e-923ea21bc48f
I0207 20:57:04.670063  5876 executor.cpp:478] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0207 20:57:05.175050  5876 executor.cpp:651] Forked command at 2288
I0207 20:57:05.204072  6708 exec.cpp:445] Executor asked to shutdown
I0207 20:57:05.205040  5876 executor.cpp:171] Received SHUTDOWN event
I0207 20:57:05.205040  5876 executor.cpp:748] Shutting down
I0207 20:57:05.205040  5876 executor.cpp:863] Sending SIGTERM to process tree 
at pid 247  2068 master.cpp:3239] Deactivating framework 
b9f2a006-df38-46e0-b8be-85efde5447c3- (default) at 
scheduler-bf610ad9-c067-4614-b82b-f632e1568bc6@10.3.1.5:59751
I0207 20:57:05.202073  3272 hierarchical.cpp:405] Deactivated framework 
b9f2a006-df38-46e0-b8be-85efde5447c3-
I0207 20:57:05.202073  2068 master.cpp:10204] Updating the state of task 
c6317f1b-f8c3-4e2e-b15e-923ea21bc48f of framework 
b9f2a006-df38-46e0-b8be-85efde5447c3- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0207 20:57:05.202073  7332 slave.cpp:3479] Shutting down framework 
b9f2a006-df38-46e0-b8be-85efde5447c3-
I0207 20:57:05.202073  7332 slave.cpp:6178] Shutting down executor 
'c6317f1b-f8c3-4e2e-b15e-923ea21bc48f' of framework 
b9f2a006-df38-46e0-b8be-85efde5447c3- at executor(1)@10.3.1.5:59772
I0207 20:57:05.203073  7332 slave.cpp:931] Agent terminating
W0207 20:57:05.204072  7332 slave.cpp:3475] Ignoring shutdown framework 
b9f2a006-df38-46e0-b8be-85efde5447c3- because it is terminating
I0207 20:57:05.205040  2068 master.cpp:10303] Removing task 
c6317f1b-f8c3-4e2e-b15e-923ea21bc48f with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework b9f2a006-df38-46e0-b8be-85efde5447c3- on 
agent b9f2a006-df38-46e0-b8be-85efde5447c3-S0 at slave(330)@10.3.1.5:59751 
(build-srv-04.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0207 20:57:05.206046  8624 containerizer.cpp:2338] Destroying container 
99c11c94-aea8-4a79-959b-c6b208d02c59 in RUNNING state
I0207 20:57:05.206046  8624 containerizer.cpp:2952] Transitioning the state of 
container 99c11c94-aea8-4a79-959b-c6b208d02c59 from RUNNING to DESTROYING
I0207 20:57:05.207041  8624 launcher.cpp:156] Asked to destroy container 
99c11c94-aea8-4a79-959b-c6b208d02c59
I0207 20:57:05.208039  2068 master.cpp:1307] Agent 

Re: Review Request 65403: Windows: Disabled `O_CLOEXEC` semantic mapping on Windows.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 1, 2018, 11:23 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65403/
> ---
> 
> (Updated Feb. 1, 2018, 11:23 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-5882
> https://issues.apache.org/jira/browse/MESOS-5882
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously we mapped `O_CLOEXEC` to `O_NOINHERIT`, which is close to the
> intended semantics, but not close enough. Instead, we want all Windows
> handles by default to be non-inheritable, and not tie any Windows
> inheritance semantics to the close-on-exec semantics. So, we define
> `O_CLOEXEC` on Windows to be `0`, and remove the logging from the
> `os::cloexec` family of functions because do not ever intend to
> implement them.
> 
> We achieve the "close" part of the semantics by making all handles
> non-inheritable, as the intent is to avoid leaking handles.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/open.hpp 
> 6711f22d44b0e7c13da3abd51f9fb7b71779e868 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> 5800ec92f85401a80cb813afd880be2e5a24a3af 
> 
> 
> Diff: https://reviews.apache.org/r/65403/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65402: Windows: Used Unicode API to duplicate sockets.

[Akash Gupta]

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


Ship it!





3rdparty/stout/include/stout/os/windows/dup.hpp
Line 30 (original), 33 (patched)


It's not a change for this patch, but we should probably look to make 
`os::dup` not inheritable by default and hunt down cases that we actually need 
inheritance.


- Akash Gupta


On Feb. 7, 2018, 7:22 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65402/
> ---
> 
> (Updated Feb. 7, 2018, 7:22 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The CRT API for duplicating file handles is left as-is because
> `WindowsFD` currently gets a CRT `int` file descriptor, which
> necessitates using `_dup` instead of `DuplicateHandle`. Furthermore, the
> latter API does not appear compatible with handles used to redirect
> stdout/stderr/stdin for subprocesses.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/dup.hpp 
> b37aaa6429e0e7b4340e86c078f5c543e443bdcd 
> 
> 
> Diff: https://reviews.apache.org/r/65402/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65401: Windows: Enabled `Flags::runtime_directory` for checkpointing.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 1, 2018, 11:22 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65401/
> ---
> 
> (Updated Feb. 1, 2018, 11:22 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6713
> https://issues.apache.org/jira/browse/MESOS-6713
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This had previously been compiled out on Windows, but we bring it back
> in order to support checkpointing.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.hpp 
> 49eb9128ae5a1e4ec02e2b9d9e3cb67d7a8f7663 
>   src/slave/containerizer/mesos/launch.cpp 
> 91016ed417428e3a5b21a132a96b9d7760d13aa3 
> 
> 
> Diff: https://reviews.apache.org/r/65401/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65400: Windows: Tied task lifetimes to executors.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Jan. 29, 2018, 8:34 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65400/
> ---
> 
> (Updated Jan. 29, 2018, 8:34 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6713
> https://issues.apache.org/jira/browse/MESOS-6713
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To enable recovery of checkpointed tasks, the agent must be able to die
> without also killing the executors and tasks, thus we cannot set the
> "job object kill on close" limit unconditionally. However, the executors
> must still be able to kill their tasks when they die, so we explicitly
> enable this limit through a parent hook when launching the container for
> the task. In this way, the agent can be restarted (e.g. for an upgrade)
> without killing the executors, but the executors are still capable of
> killing their tasks on catastrophic death.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 050f5a057f360873e2b4738b126289bcd1bd0c7f 
> 
> 
> Diff: https://reviews.apache.org/r/65400/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65399: Windows: Moved "kill on close" job object flag to own function.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Feb. 7, 2018, 7:20 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65399/
> ---
> 
> (Updated Feb. 7, 2018, 7:20 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6713
> https://issues.apache.org/jira/browse/MESOS-6713
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of setting this flag unconditionally when we create a job
> object, this patch abstracts it to the the function
> `set_job_kill_on_close_limit(pid_t)` so that users can choose how to tie
> job object lifetimes together.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> ec935209fc1532ebe087ac20933c99eed393506d 
> 
> 
> Diff: https://reviews.apache.org/r/65399/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 65552: Added a regression test for MESOS-8468.

[Gaston Kleiman]

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

Review request for mesos, Anand Mazumdar, Greg Mann, Qian Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

Added a regression test for MESOS-8468.


Diffs
-

  src/tests/default_executor_tests.cpp cc97e0d1fea7f4d0bc544d850593d8d91921b552 


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


Testing
---

`GLOG_v=1 sudo bin/mesos-tests.sh --gtest_filter='*ROOT_LaunchGroupFailure*' 
--verbose --gtest_repeat=650 --gtest_break_on_failure` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 62798: Added named cgroup hierarchy support.

[Jie Yu]

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

(Updated Feb. 7, 2018, 8:01 p.m.)


Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch add a helper to get the cgroup associated with the given
pid for a named cgroup hierarchy.


Diffs (updated)
-

  src/linux/cgroups.hpp 154e5b6c1adef4f4e77f1747c56dfd83a9967ed4 
  src/linux/cgroups.cpp cdd0f40ec5748b909e529f21b15523122bf90d41 


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

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


Testing
---

make check


Thanks,

Jie Yu

Re: Review Request 62799: Fixed an issue for the I/O switchboard process lifetime.

[Jie Yu]

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

(Updated Feb. 7, 2018, 8:01 p.m.)


Review request for mesos, Joseph Wu and Kevin Klues.


Changes
---

Rebased.


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


Repository: mesos


Description
---

We expect the I/O switchboard process to last across agent restarts
(similar to log rotate process or executor processes). Therefore, we
should put it into 'mesos_executor.slice' like others.


Diffs (updated)
-

  src/slave/containerizer/mesos/io/switchboard.cpp 
aeb0b3e4cf75b19efd9b3922cc4707d3c5cb 


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

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

[Jie Yu]

> On Oct. 7, 2017, 12:23 a.m., James Peach wrote:
> > src/slave/containerizer/mesos/linux_launcher.cpp
> > Lines 402 (patched)
> > 
> >
> > Would we really exit? I'm not sure we should expect users to be able to 
> > manually deal with that case.

This is not a TODO from me:) I'd keep it here and probably follow up with a 
different patch.


- Jie


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


On Oct. 6, 2017, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 10:04 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> |[jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> | |-31737 /usr/libexec/mesos/mesos-containerizer launch
> | |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> | |-mesos
> ||-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

[Jie Yu]

> On Oct. 7, 2017, 12:23 a.m., James Peach wrote:
> > src/slave/containerizer/mesos/linux_launcher.cpp
> > Lines 189 (patched)
> > 
> >
> > This is racy, but we can fix it by ensuring that `os::mkdir` checks for 
> > `EEXIST` even when it is non-recursive.

This is under mesos cgroup root, i don't race is a big concern. I've been 
following what we did in cgroups isolator. I'd prefer consistency here.


- Jie


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


On Oct. 6, 2017, 10:04 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Oct. 6, 2017, 10:04 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> |[jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> | |-31737 /usr/libexec/mesos/mesos-containerizer launch
> | |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> | |-mesos
> ||-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/linux/systemd.hpp f760e2c02683f0169ba5d3b0a3da53f66a7f3d91 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 554c598e2a7a53aede9d8761740d8efceb4a7e39 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 65551: Stopped shutting down the whole default executor on task launch failure.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65548', '65549', '65550', '65551']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(414): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(415): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(437): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(438): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(460): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(461): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(500): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\java\jni\convert.cpp(501): warning C4267: 
'argument': conversion from 'size_t' to 'jsize', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 
D:\DCOS\mesos\mesos\src\java\jni\org_apache_mesos_state_LogState.cpp(70): 
warning C4244: 'initializing': conversion from 'jlong' to 'long', possible loss 
of data [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 
D:\DCOS\mesos\mesos\src\java\jni\org_apache_mesos_state_Variable.cpp(44): 
warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss 
of data [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 
D:\DCOS\mesos\mesos\src\java\jni\org_apache_mesos_state_Variable.cpp(45): 
warning C4267: 'argument': conversion from 'size_t' to 'jsize', possible loss 
of data [D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(95): warning C4267: '=': 
conversion from 'size_t' to 'jint', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(463): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(493): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(654): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(693): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(770): warning C4244: 
'initializing': conversion from 'jchar' to 'char', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]
 D:\DCOS\mesos\mesos\src\jvm\jvm.cpp(809): warning C4244: 
'initializing': conversion from 'jlong' to 'long', possible loss of data 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]


   "D:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
   "D:\DCOS\mesos\src\java\mesos-java.vcxproj" (default target) (13) ->
   (ClCompile target) -> 
 D:\DCOS\mesos\mesos\include\mesos/v1/scheduler/scheduler.hpp(45): 
error C2679: binary '<<': no operator found which takes a right-hand operand of 
type 'const mesos::v1::TaskStatus' (or there is no acceptable conversion) 
(compiling source file D:\DCOS\mesos\mesos\src\java\jni\convert.cpp) 
[D:\DCOS\mesos\src\java\mesos-java.vcxproj]

214 Warning(s)
1 Error(s)

Time Elapsed 00:07:02.04
```

- Mesos Reviewbot Windows


On Feb. 7, 2018, 7:05 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65551/
> 

Re: Review Request 65402: Windows: Used Unicode API to duplicate sockets.

[Andrew Schwartzmeyer]

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

(Updated Feb. 7, 2018, 11:22 a.m.)


Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


Summary (updated)
-

Windows: Used Unicode API to duplicate sockets.


Repository: mesos


Description
---

The CRT API for duplicating file handles is left as-is because
`WindowsFD` currently gets a CRT `int` file descriptor, which
necessitates using `_dup` instead of `DuplicateHandle`. Furthermore, the
latter API does not appear compatible with handles used to redirect
stdout/stderr/stdin for subprocesses.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/windows/dup.hpp 
b37aaa6429e0e7b4340e86c078f5c543e443bdcd 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65399: Windows: Moved "kill on close" job object flag to own function.

[Andrew Schwartzmeyer]

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

(Updated Feb. 7, 2018, 11:20 a.m.)


Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

Instead of setting this flag unconditionally when we create a job
object, this patch abstracts it to the the function
`set_job_kill_on_close_limit(pid_t)` so that users can choose how to tie
job object lifetimes together.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows/os.hpp 
ec935209fc1532ebe087ac20933c99eed393506d 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 65538: Added provider assignments into `disk_profile.proto`.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 65520.

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

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

Relevant logs:

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

```
error: patch failed: src/tests/storage_local_resource_provider_tests.cpp:73
error: src/tests/storage_local_resource_provider_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Feb. 7, 2018, 6:51 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65538/
> ---
> 
> (Updated Feb. 7, 2018, 6:51 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8510
> https://issues.apache.org/jira/browse/MESOS-8510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A new `assignments` field is added to the `DiskProfileMapping` protobuf
> so that the URI disk profile adaptor can be customized to notify each
> resource provider a different set of profiles.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/disk_profile.proto 
> 6cf1f8abcd24e45a292efc95a395f90bb2140da2 
> 
> 
> Diff: https://reviews.apache.org/r/65538/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65085: WIP: Added a unit test for fd leakage with blocking HTTP calls.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65085 was successfully built and tested.

Reviews applied: `['65085']`

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

- Mesos Reviewbot Windows


On Jan. 10, 2018, 3:30 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65085/
> ---
> 
> (Updated Jan. 10, 2018, 3:30 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Jie Yu.
> 
> 
> Bugs: MESOS-8428
> https://issues.apache.org/jira/browse/MESOS-8428
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When running `ContainerDaemonTest.FileDescriptorLeakage` in repetition,
> the test will use up all fds.
> 
> 
> Diffs
> -
> 
>   src/tests/container_daemon_tests.cpp 
> 1396f25bf7aa9fbc62af9111ad412238faff432e 
> 
> 
> Diff: https://reviews.apache.org/r/65085/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 65551: Stopped shutting down the whole default executor on task launch failure.

[Gaston Kleiman]

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

Review request for mesos, Anand Mazumdar, Greg Mann, Qian Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

The default executor would be completely shutdown on a
`LAUNCH_NESTED_CONTAINER` failure.

This patch makes it kill the affected task group instead of shutting
down and killing all task groups.


Diffs
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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


Testing
---

`sudo make check` on GNU/Linux

Regression test on https://reviews.apache.org/r/65552/


Thanks,

Gaston Kleiman

Re: Review Request 65467: Windows: Added `internal::windows::enable_inherit(WindowsFD)`.

[Andrew Schwartzmeyer]

> On Feb. 6, 2018, 1:55 p.m., Joseph Wu wrote:
> > 3rdparty/stout/include/stout/internal/windows/inherit.hpp
> > Lines 31-43 (patched)
> > 
> >
> > This is basically what the ChildHook `UNSET_CLOEXEC` 
> > (libprocess/src/subprocess.cpp) should be doing.
> 
> Andrew Schwartzmeyer wrote:
> Pretty much, but we don't have any child hook support on Windows 
> whatsoever. Adding support for child hooks would be a non-trivial undertaking 
> (we don't have a pseudo-program written to run child hooks and launch a 
> process like on Linux, we're just launching the process directly).
> 
> Joseph Wu wrote:
> Hum... on second though, there appears to be no equivalent for a 
> `UNSET_CLOEXEC` ChildHook on Windows.
> 
> 
> (This comment also applies to the following review: 
> https://reviews.apache.org/r/65469/ )
> 
> We still want to minimize the time each FD spends in an inheritable 
> state, in order to minimize leaks to other forks/processes in other threads.  
> Ideally, any calls to this method should be made right before calls to 
> `subprocess`.  The Linux code does this implicitly, by __not__ cloexec-ing 
> certain FDs in some methods.
> 
> Correct me if I'm wrong, but FDs do not need to be inheritable in order 
> for `CreateProcessW` to use them as stdout/err FDs.

Correcting: The handles _must_ be inheritable for `CreateProcessW` to use them 
as standard handles.

> STARTF_USESTDHANDLES: If this flag is specified when calling one of the 
> process creation functions, the handles must be inheritable and the 
> function's bInheritHandles parameter must be set to TRUE.

To redirect the stdin/stdout/stderr of a process made by `CreateProcessW`, the 
procedure is to set that flag in the `STARTUPINFO`, and then pass the handles 
via `hStdInput`, `hStdOutput`, and `hStdError`.

I agree that we want to minimize the time a handle is inheritable; so I don't 
think this is the best solution yet. I think we would want to set the handles 
as inheritable in the `create_process` wrapper itself, so they're marked 
inheritable only if they exist and are being passed to a child process. The 
plus to this is that they will always be inheritable if they are meant to be 
inherited. The minus to this is that the programmer may not be aware their 
handle is about to modified and made inheritable (though it should be fairly 
obvious as it is being passed to a child process). We could then call 
`CreateProcess` and mark the handles uninheritable after the process has been 
fully initialized. Tricky part, however, will be avoiding a race, as "the 
function (`CreateProcess`) returns before the process has finished 
initialization. The calling thread can use the `WaitForInputIdle` function to 
wait until the new process has finished its initialization and is waiting for 
user input with no 
 input pending." So it is doable.

Additionally, if we do this, we'll want to make sure `os::dup` does not create 
inheritable handles (and also fix the locations where `::dup` is used).


- Andrew


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


On Feb. 2, 2018, 12:13 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65467/
> ---
> 
> (Updated Feb. 2, 2018, 12:13 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This function is used on Windows to explicitly enable inheritance on a
> handle.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65467/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 65550: Made default executor not shutdown if unsubscribed during task launch.

[Gaston Kleiman]

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

Review request for mesos, Anand Mazumdar, Qian Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

The default executor would unnecessarily shutdown if, while launching a
task group, it gets unsubscribed after having successfully launched the
task group's containers.


Diffs
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Review Request 65549: Improved some default executor log messages.

[Gaston Kleiman]

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

Review request for mesos, Greg Mann, Qian Zhang, and Vinod Kone.


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


Repository: mesos


Description
---

Improved some default executor log messages.


Diffs
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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


Testing
---

None, this patch doesn't contain functional changes.


Thanks,

Gaston Kleiman

Review Request 65548: Added `Event::Update` and `v1::scheduler::TaskStatus` ostream operators.

[Gaston Kleiman]

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

Review request for mesos, Alexander Rukletsov, Greg Mann, Qian Zhang, and Vinod 
Kone.


Repository: mesos


Description
---

This operators make gtest print a human-readable representation of the
protos on test failures.


Diffs
-

  include/mesos/v1/mesos.hpp d4c354ab596a6ea361f2fe45afa46089f8c1a543 
  include/mesos/v1/scheduler/scheduler.hpp 
2fdd8f265d8dd5e3a334055ab8777a175688f620 
  src/v1/mesos.cpp 8abeae06e0fa87b50933df1c222ad4724a0b0116 


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


Testing
---

Example test failure without the patch:

```
Unexpected mock function call - returning directly.
Function call: update(0x7ffedfa94eb0, @0x7f8638002c30 32-byte object <50-92 
EA-29 87-7F 00-00 00-00 00-00 00-00 00-00 01-00 00-00 00-00 00-00 60-30 00-38 
86-7F 00-00>)
Google Mock tried the following 8 expectations, but none matched:

../../src/tests/default_executor_tests.cpp:3315: tried expectation #0: 
EXPECT_CALL(*scheduler, update(_, AllOf( 
TaskStatusUpdateTaskIdEq(sleepTaskInfo1), 
TaskStatusUpdateStateEq(v1::TASK_ERROR...
[...]
) and (task status update state eq TASK_ERROR)
   Actual: 32-byte object <50-92 EA-29 87-7F 00-00 00-00 00-00 00-00 
00-00 01-00 00-00 00-00 00-00 60-30 00-38 86-7F 00-00>
 Expected: to be called once
   Actual: never called - unsatisfied and active
```

After applying the patch:

```
Unexpected mock function call - returning directly.
Function call: update(0x7ffc6b036a20, @0x7f4af4000bb0 TASK_STARTING (Status 
UUID: f379eb50-1163-442a-8e30-a0c2f5247575) for task 'sleepTask1')
Google Mock tried the following 8 expectations, but none matched:

../../src/tests/default_executor_tests.cpp:3315: tried expectation #0: 
EXPECT_CALL(*scheduler, update(_, AllOf( 
TaskStatusUpdateTaskIdEq(sleepTaskInfo1), 
TaskStatusUpdateStateEq(v1::TASK_ERROR...
[...]
) and (task status update state eq TASK_ERROR)
   Actual: TASK_STARTING (Status UUID: 
f379eb50-1163-442a-8e30-a0c2f5247575) for task 'sleepTask1'
```


Thanks,

Gaston Kleiman

Re: Review Request 65538: Added provider assignments into `disk_profile.proto`.

[Jie Yu]

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


Fix it, then Ship it!




Can you update the summary and description of this review accordingly?


src/resource_provider/storage/disk_profile.proto
Line 32 (original), 51 (patched)


I don't change the tag number.


- Jie Yu


On Feb. 7, 2018, 6:51 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65538/
> ---
> 
> (Updated Feb. 7, 2018, 6:51 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8510
> https://issues.apache.org/jira/browse/MESOS-8510
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A new `assignments` field is added to the `DiskProfileMapping` protobuf
> so that the URI disk profile adaptor can be customized to notify each
> resource provider a different set of profiles.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/disk_profile.proto 
> 6cf1f8abcd24e45a292efc95a395f90bb2140da2 
> 
> 
> Diff: https://reviews.apache.org/r/65538/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65538: Added provider assignments into `disk_profile.proto`.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 6:51 p.m.)


Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


Changes
---

Addressed James' comment.


Summary (updated)
-

Added provider assignments into `disk_profile.proto`.


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


Repository: mesos


Description
---

A new `assignments` field is added to the `DiskProfileMapping` protobuf
so that the URI disk profile adaptor can be customized to notify each
resource provider a different set of profiles.


Diffs (updated)
-

  src/resource_provider/storage/disk_profile.proto 
6cf1f8abcd24e45a292efc95a395f90bb2140da2 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 65522: Fixed some log messages in `slave.cpp`.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 6:45 p.m.)


Review request for mesos, Gaston Kleiman and Greg Mann.


Changes
---

Addressed Gaston's comments.


Repository: mesos


Description
---

Fixed some log messages in `slave.cpp`.


Diffs (updated)
-

  src/slave/slave.cpp a6a5c93ab2d541c870cb52587495de20ed5ac1f4 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65062: Cleaned up endpoint directories after RP config agent API tests.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 6:37 p.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


Changes
---

Handled multiple `CreateSlaveFlags()` calls.


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


Repository: mesos


Description
---

Cleaned up endpoint directories after RP config agent API tests.


Diffs (updated)
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
7cbee5cceef2963b736f3e6ce3f52fcf0b3c029c 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65060: Cleaned up endpoint directories after SLRP tests.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 6:37 p.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


Changes
---

Handled multiple `CreateSlaveFlags()` calls.


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


Repository: mesos


Description
---

Cleaned up endpoint directories after SLRP tests.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ce241617ee555e5a67e552dd30b0e7afee161386 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65062: Cleaned up endpoint directories after RP config agent API tests.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 6:18 p.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


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


Repository: mesos


Description
---

Cleaned up endpoint directories after RP config agent API tests.


Diffs (updated)
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
7cbee5cceef2963b736f3e6ce3f52fcf0b3c029c 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65521: Improved isolation for agent RP API config tests.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 6:16 p.m.)


Review request for mesos, Greg Mann and Jie Yu.


Changes
---

Fixed a test setup bug.


Repository: mesos


Description
---

This patch makes the tests to use a unique cgroup for each run. It also
changes `CHECK_*` macros to `ASSERT_*` so the tests can be torn down
properly upon assertion failures.


Diffs (updated)
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
7cbee5cceef2963b736f3e6ce3f52fcf0b3c029c 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65060: Cleaned up endpoint directories after SLRP tests.

[Chun-Hung Hsiao]

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

(Updated Feb. 7, 2018, 6:15 p.m.)


Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.


Changes
---

Addressed Jie's comment.


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


Repository: mesos


Description
---

Cleaned up endpoint directories after SLRP tests.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
ce241617ee555e5a67e552dd30b0e7afee161386 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

[Benno Evers]

> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 630 (patched)
> > 
> >
> > Use `profilingTimer->timeout().remaining()` directly.

There are two issues with that:
1) The redirectTime is currently selected to be 2 seconds shorter than the time 
remaining in the profiling timer, so by default it is the redirect that is 
stopping the profiling
2) Aesthetically, if a user is selecting `duration=5secs` it seems odd to 
display `You will be redirected in 4.3 seconds`.


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 650-653 (patched)
> > 
> >
> > Why do you need to check if you can read the key? Can this be omitted 
> > or maybe moved to the bottom of the function?

The key must be read *before* we stop profiling, since we are interested in the 
previous value of this setting. This should realistically never fail, so if it 
does the circumstances are so weird that we probably shouldn't continue working 
with this jemalloc (e.g. the user is using a custom-compiled version where this 
setting was removed/renamed)


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 680 (patched)
> > 
> >
> > I think the right criteria here is (was_active && not_active_now). If 
> > you want to fo with a single read of a jemalloc setting, then still use the 
> > value right after obtaining it.

We only get here if `stopAndGenerateRawProfile()` did return success, so 
`not_active_now` should always be true.


> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 897-903 (patched)
> > 
> >
> > Maybe this instead?
> > ```
> >   // State unrelated to jemalloc.
> >   JSON::Object state;
> >   {
> > JSON::Object profilerState;
> > profilerState.values["jemalloc_detected"] = detected;
> > profilerState.values["profiling_active"] = heapProfilingActive;
> > 
> > state.values["memory_profiler"] = std::move(profilerState);
> >   }
> > ```

I've rearranged it, but I'm not entirely sure about this because this pattern 
starts nesting in the later parts of this function.


- Benno


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> ---
> 
> (Updated Feb. 6, 2018, 5:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class exposes profiling functionality of jemalloc memory allocator
> when it is detected to be the memory allocator of the current process.
> 
> In particular, it gives developers an easy method to collect and
> access heap profiles which report which pieces of code were
> responsible for allocating memory.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 
> 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

[Benno Evers]

> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 264 (patched)
> > 
> >
> > Say "using std::string" in the beginning of the file and save hundreds 
> > of characters!

I'm not a fan of using `using`-directives just to save some typing, as it puts 
a large cognitive burden on the reader: Is `string` referring to name imported 
to the global namespace via `using`, a class-local or a function-local typedef, 
maybe defined in some `stout`-header? In any case, one has to jump to a 
different context to double-check. Using `std::string`, on the other hand, is 
not ambigous.


- Benno


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> ---
> 
> (Updated Feb. 6, 2018, 5:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class exposes profiling functionality of jemalloc memory allocator
> when it is detected to be the memory allocator of the current process.
> 
> In particular, it gives developers an easy method to collect and
> access heap profiles which report which pieces of code were
> responsible for allocating memory.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 
> 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

[Akash Gupta]

> On Feb. 7, 2018, 1:35 p.m., Alexander Rukletsov wrote:
> > src/checks/checker.cpp
> > Line 86 (original), 89 (patched)
> > 
> >
> > please no abbreviations like this, it is unclear what this function 
> > does. Is it a factory? Shall it be better be lambda rather than a free 
> > function?
> > 
> > Also, I see a similar function in `HealthCheck`. Is there any way you 
> > can move duplicate code into the lower level `CheckerProcess`?

Yeah, it's a factory function. I think both of your issues can be resolved by 
passing the `CheckInfo` struct into the `CheckerProcess` class and having the 
`HealthChecker` use the old `toCheckInfo` function to convert the 
`HealthCheckInfo` to `CheckInfo`. That should avoid the code duplication too.


- Akash


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


On Jan. 30, 2018, 10:06 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65395/
> ---
> 
> (Updated Jan. 30, 2018, 10:06 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored health check code to separate the logic for each check
> type and runtime (Plain/Docker/Nested). Now the matrix of different
> possiblilites is cleanly enumerated, making it easier to add
> future health checks, such as the ones for Windows.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp 93502270f31e80c5f7c94b5b456625e9cdea1837 
>   src/checks/checker.cpp fff0aac504b4283a210f936e00c977fa60d88b3d 
>   src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/checks/health_checker.hpp 019fbd791f250ecc28ff59d779f90e7ccbf0c685 
>   src/checks/health_checker.cpp eaf9a18817eeeff7c29c7a4b9d1b183f398760a3 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
>   src/launcher/executor.cpp 050f5a057f360873e2b4738b126289bcd1bd0c7f 
> 
> 
> Diff: https://reviews.apache.org/r/65395/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65545: Start from merge-base when posting reviews.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65545 was successfully built and tested.

Reviews applied: `['65545']`

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

- Mesos Reviewbot Windows


On Feb. 7, 2018, 3:22 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65545/
> ---
> 
> (Updated Feb. 7, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When posting a review originally created from a branch B, it could happen 
> that the resulting review included garbage changes if B was updated between 
> revisions. (i.e., pulling new master changes)
> 
> This review changes the logic of the `post-reviews.py` script to only include 
> the changes between `HEAD` and the merge base of the tracking branch into the 
> review.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 94b43253f25044a79009010e92363d1536c74299 
> 
> 
> Diff: https://reviews.apache.org/r/65545/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65529: Added --clean flag to bootstrap script for Mesos CLI.

[Armand Grillet]

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

(Updated Feb. 7, 2018, 3:24 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Used Python to make the bash script cross-platform.


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


Repository: mesos


Description
---

Without it, the bootstrap script compares the modification date
of the virtual environment compared to the pip-requirements
and bootstrap script. If the virtualenv is newer, the bootstrap
script will stop instead of building the environment again.


Diffs (updated)
-

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 


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

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


Testing
---

Run bootstrap script three times. With no `.virtualenv`:
```
$ ./bootstrap
```
Creates the virtual environment.
```
$ ./bootstrap
The virtual environment is more recent than its dependencies, no need
to rebuild it. Use 'bootstrap --clean' to rebuild the virtualenv
without comparing its last modification date with its dependencies.
$ ./bootstrap --clean
```
The virtual environment will then be rebuilt again with this third command.


Thanks,

Armand Grillet

Re: Review Request 65545: Start from merge-base when posting reviews.

[Benno Evers]

> On Feb. 7, 2018, 2:35 p.m., Benno Evers wrote:
> > support/post-reviews.py
> > Lines 202 (patched)
> > 
> >
> > I'm tempted to make this fatal, since I cannot imagine a good reason 
> > why anyone would want to still post the review, but I wanted to ask for 
> > other opinions first.

After discussion with @bbannier, changed the behaviour to warn about the issue 
and attempt to do the right thing afterwards.


- Benno


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


On Feb. 7, 2018, 3:22 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65545/
> ---
> 
> (Updated Feb. 7, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When posting a review originally created from a branch B, it could happen 
> that the resulting review included garbage changes if B was updated between 
> revisions. (i.e., pulling new master changes)
> 
> This review changes the logic of the `post-reviews.py` script to only include 
> the changes between `HEAD` and the merge base of the tracking branch into the 
> review.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 94b43253f25044a79009010e92363d1536c74299 
> 
> 
> Diff: https://reviews.apache.org/r/65545/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65545: Start from merge-base when posting reviews.

[Benno Evers]

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

(Updated Feb. 7, 2018, 3:22 p.m.)


Review request for mesos, Armand Grillet and Benjamin Bannier.


Summary (updated)
-

Start from merge-base when posting reviews.


Repository: mesos


Description (updated)
---

When posting a review originally created from a branch B, it could happen that 
the resulting review included garbage changes if B was updated between 
revisions. (i.e., pulling new master changes)

This review changes the logic of the `post-reviews.py` script to only include 
the changes between `HEAD` and the merge base of the tracking branch into the 
review.


Diffs (updated)
-

  support/post-reviews.py 94b43253f25044a79009010e92363d1536c74299 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Eric Chung]

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

(Updated Feb. 7, 2018, 3:13 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


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


Repository: mesos


Description
---

Added mesos.http and mesos.exceptions for CLI.

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.

updated patch according to klueska's recommendations


Diffs
-

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 


Diff: https://reviews.apache.org/r/61172/diff/10/


Testing
---

install tox
cd src/python/lib
tox


Thanks,

Eric Chung

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> updated patch according to klueska's recommendations
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
>   support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/10/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 65545: Add warning in support/post-reviews.py.

[Benno Evers]

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




support/post-reviews.py
Lines 202 (patched)


I'm tempted to make this fatal, since I cannot imagine a good reason why 
anyone would want to still post the review, but I wanted to ask for other 
opinions first.


- Benno Evers


On Feb. 7, 2018, 2:33 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65545/
> ---
> 
> (Updated Feb. 7, 2018, 2:33 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new check warns developers if the specified tracking
> branch is not fast-forwardable to HEAD. The most likely
> cause of this condition is pulling a new version of the `master`
> branch and forgetting to rebase the review before calling
> the `post-reviews.py` script.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 94b43253f25044a79009010e92363d1536c74299 
> 
> 
> Diff: https://reviews.apache.org/r/65545/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Review Request 65545: Add warning in support/post-reviews.py.

[Benno Evers]

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

Review request for mesos, Armand Grillet and Benjamin Bannier.


Repository: mesos


Description
---

The new check warns developers if the specified tracking
branch is not fast-forwardable to HEAD. The most likely
cause of this condition is pulling a new version of the `master`
branch and forgetting to rebase the review before calling
the `post-reviews.py` script.


Diffs
-

  support/post-reviews.py 94b43253f25044a79009010e92363d1536c74299 


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


Testing
---


Thanks,

Benno Evers

Re: Review Request 64211: Added options to build the Python CLI and run unit tests.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 64211 was successfully built and tested.

Reviews applied: `['65529', '64211']`

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

- Mesos Reviewbot Windows


On Feb. 4, 2018, 6:07 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64211/
> ---
> 
> (Updated Feb. 4, 2018, 6:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An update of the discarded review /r/52543.
> 
> Works with Autotools and CMake.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 50cddf9476c8c5196c4824a7b060c2680a96b277 
>   cmake/MesosConfigure.cmake 0954a9cd31fa290ff9099be4b06d69d96b701f1e 
>   configure.ac 30fbadc32d1d96f719d45fa8067f975283c25507 
>   docs/configuration/cmake.md 1e34657ea55fa324f65f865f7d0a67084c6719d9 
>   src/CMakeLists.txt 21fb47e29dd0b19681690b8de5261c68b574a7c8 
>   src/Makefile.am e444b8981093ba6f4e94acb7a060b998f75b542a 
>   src/python/cli_new/CMakeLists.txt PRE-CREATION 
>   src/python/cli_new/tests/CMakeLists.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/64211/diff/4/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25.
> 
> For Autotools:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ ../configure --enable-new-cli --disable-java
> $ make check
> ```
> 
> For CMake:
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1
> $ cmake --build . -- -j16
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> `build/src/cli` was as expected, and that `build/src/mesos/mesos` was 
> correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 65395: Refactored health checks to cleanly separate each different check.

[Alexander Rukletsov]

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




src/checks/checker.hpp
Lines 120-122 (original), 87-89 (patched)


Why do we need to pass these around (and same in `HealthChecker`) if we 
pass `CheckInfo` / `HealthCheck`? Wouldn't be better to extract these just once 
in `CheckerProcess`?



src/checks/checker.cpp
Line 86 (original), 89 (patched)


please no abbreviations like this, it is unclear what this function does. 
Is it a factory? Shall it be better be lambda rather than a free function?

Also, I see a similar function in `HealthCheck`. Is there any way you can 
move duplicate code into the lower level `CheckerProcess`?


- Alexander Rukletsov


On Jan. 30, 2018, 10:06 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65395/
> ---
> 
> (Updated Jan. 30, 2018, 10:06 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored health check code to separate the logic for each check
> type and runtime (Plain/Docker/Nested). Now the matrix of different
> possiblilites is cleanly enumerated, making it easier to add
> future health checks, such as the ones for Windows.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp 93502270f31e80c5f7c94b5b456625e9cdea1837 
>   src/checks/checker.cpp fff0aac504b4283a210f936e00c977fa60d88b3d 
>   src/checks/checker_process.hpp 510f3b2e6e689faaf26595214ce377c2b5518f28 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
>   src/checks/health_checker.hpp 019fbd791f250ecc28ff59d779f90e7ccbf0c685 
>   src/checks/health_checker.cpp eaf9a18817eeeff7c29c7a4b9d1b183f398760a3 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
>   src/launcher/executor.cpp 050f5a057f360873e2b4738b126289bcd1bd0c7f 
> 
> 
> Diff: https://reviews.apache.org/r/65395/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65394: Added separate structs for health check runtime and check types.

[Alexander Rukletsov]

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




src/checks/checks_runtime.hpp
Lines 48 (patched)


I feel that copy is better than composition here. Who knows how these 
runtimes evolve, let's use `namespaces` and `taskPid` directly.



src/checks/checks_runtime.hpp
Lines 55-57 (patched)


This is not quite true: you only mean command health checks. HTTP and TCP 
are still performed by the executor directly.


- Alexander Rukletsov


On Jan. 30, 2018, 10:05 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65394/
> ---
> 
> (Updated Jan. 30, 2018, 10:05 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, and 
> Gaston Kleiman.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch is in preparation of another patch that will refactor the
> health check code using these types.
> 
> 
> Diffs
> -
> 
>   src/checks/checks_runtime.hpp PRE-CREATION 
>   src/checks/checks_types.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65394/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 65419: Fixed quoting issues in docker executor command checks.

[Alexander Rukletsov]

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



Is this related to https://issues.apache.org/jira/browse/MESOS-4812 ?

- Alexander Rukletsov


On Jan. 30, 2018, 10:12 a.m., Akash Gupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65419/
> ---
> 
> (Updated Jan. 30, 2018, 10:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Andrew Schwartzmeyer, Gaston 
> Kleiman, and Joseph Wu.
> 
> 
> Bugs: MESOS-8498
> https://issues.apache.org/jira/browse/MESOS-8498
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The docker executor was wrapping the docker exec command around with
> `sh -c ""`, which was causing quoting issues, especially on Windows.
> Now, the comamnd health check simply runs `docker exec` without any
> wrapping.
> 
> 
> Diffs
> -
> 
>   src/checks/checker_process.cpp ddb197b8cc2c503fef5ae20af32f5881fff9833f 
> 
> 
> Diff: https://reviews.apache.org/r/65419/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Akash Gupta
> 
>

Re: Review Request 64609: Skipped bundled elfio in linting.

[Armand Grillet]

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


Ship it!




Ship It!

- Armand Grillet


On Jan. 25, 2018, 6:36 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64609/
> ---
> 
> (Updated Jan. 25, 2018, 6:36 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Skipped bundled elfio in linting.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 315955e3533fce0cbf820fad03b3707049cdffe0 
> 
> 
> Diff: https://reviews.apache.org/r/64609/diff/1/
> 
> 
> Testing
> ---
> 
> No more diagnostics from elfio are reported after applying this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65045: Tested correct operation handling during master failover.

[Mesos Reviewbot Windows]

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['65044', '65045']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (444 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/1
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/1 (24 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (216 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/1
[   OK ] Scheme/HTTPTest.Accepts/1 (30 ms)
[--] 36 tests from Scheme/HTTPTest (6630 ms total)

[--] 4 tests from SSLVerifyIPAdd/SSLTest
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0 (244 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1 (167 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (994 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (872 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (2282 ms total)

[--] Global test environment tear-down
[==] 225 tests from 35 test cases ran. (35727 ms total)
[  PASSED  ] 224 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLTest.SSLSocket

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I0207 11:12:37.152746  4268 process.cpp:929] Stopped the socket accept loop
I0207 11:12:37.182744  5612 process.cpp:929] Stopped the socket accept loop
I0207 11:12:37.425719  4268 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0207 11:12:37.425719  4268 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0207 11:12:37.425719  4268 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\A721FC\cert.pem
I0207 11:12:37.425719  4268 openssl.cpp:566] Using CA dir: 
C:\Users\mesos\AppData\Local\Temp\A721FC
I0207 11:12:37.594732  4268 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0207 11:12:37.594732  4268 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I0207 11:12:37.594732  4268 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0207 11:12:37.595752  4268 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\LraHMA\cert.pem
I0207 11:12:37.595752  4268 openssl.cpp:566] Using CA dir: 
C:\Users\mesos\AppData\Local\Temp\LraHMA
I0207 11:12:37.902751  4268 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I0207 11:12:37.902751  4268 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0207 11:12:37.902751  4268 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0207 11:12:37.903755  4268 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\GK27KA\cert.pem
I0207 11:12:38.860795  4268 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I0207 11:12:38.860795  4268 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I0207 11:12:38.860795  4268 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I0207 11:12:38.860795  4268 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I0207 11:12:38.861449  4268 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\vB2Taa\cert.pem
I0207 11:12:39.497680  3996 process.cpp:929] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Feb. 7, 2018, 10:57 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65045/
> ---
> 
> 

Re: Review Request 65045: Tested correct operation handling during master failover.

[Jan Schlicht]

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

(Updated Feb. 7, 2018, 11:57 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Removed post-failover offer handling to not run into MESOS-8524.


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


Repository: mesos


Description
---

Tested correct operation handling during master failover.


Diffs (updated)
-

  src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 


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

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


Testing (updated)
---

make check


Thanks,

Jan Schlicht