Re: Review Request 68654: Enabled isort for src/python/lib.

[Eric Chung]

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

(Updated Sept. 19, 2018, 6:30 a.m.)


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


Repository: mesos


Description
---

Enabled isort for src/python/lib.


Diffs (updated)
-

  src/python/lib/mesos/http.py cd1587797db7d75c6b839851f0f3e5671269307c 
  src/python/lib/setup.cfg PRE-CREATION 
  src/python/lib/setup.py 08f854f43681d1f694bb48604773256be7ce927b 
  src/python/lib/tests/test_exceptions.py 
096eab82311e02b0fa829101b6ad1b23e42ce088 
  src/python/lib/tests/test_http.py 41a52f511318cb69c8e9976f09e2f142327544ab 
  src/python/lib/tox.ini 3ee77681a9b802cd5b4a7910779b8d50aac4cf69 


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

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


Testing
---


Thanks,

Eric Chung

Re: Review Request 68758: Added unit tests for adding/updating invalid resource provider configs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68758 was successfully built and tested.

Reviews applied: `['68755', '68756', '68757', '68758']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2354/mesos-review-68758

- Mesos Reviewbot Windows


On Sept. 19, 2018, 4:50 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68758/
> ---
> 
> (Updated Sept. 19, 2018, 4:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9228
> https://issues.apache.org/jira/browse/MESOS-9228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added unit tests for adding/updating invalid resource provider configs.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> e6a68bae1a9e3e773ea45deae4951664ab81a857 
> 
> 
> Diff: https://reviews.apache.org/r/68758/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68755: Removed unnecessary failure handling in agent HTTP API handlers.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Sept. 18, 2018, 9:47 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68755/
> ---
> 
> (Updated Sept. 18, 2018, 9:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9228
> https://issues.apache.org/jira/browse/MESOS-9228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current agent HTTP API handlers either unnecessarily handle failures
> on always-ready futures, or return "500 Internal Server Error"
> unnecessarily. This patch removes those unnecessarily code.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp f8199af50cf0b438c29afe751fcd8949d7e8 
> 
> 
> Diff: https://reviews.apache.org/r/68755/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68763: WIP: Stopped resource providers when removing resource provider configs.

[Chun-Hung Hsiao]

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

(Updated Sept. 19, 2018, 5:14 a.m.)


Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


Summary (updated)
-

WIP: Stopped resource providers when removing resource provider configs.


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


Repository: mesos


Description
---

This patch introduces a `LocalResourceProvider::stop` function to
perform RP-specific stop procedures. SLRP uses this function to kill
plugin container before its config is removed.


Diffs
-

  src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55 
  src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29 
  src/resource_provider/storage/provider.hpp 
5a371b19289c6e39fedd4cda65fa8be432d095e6 
  src/resource_provider/storage/provider.cpp 
6475f653263337c381b6080695d09c49e5ea8fcf 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 68763: Stopped resource providers when removing resource provider configs.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

This patch introduces a `LocalResourceProvider::stop` function to
perform RP-specific stop procedures. SLRP uses this function to kill
plugin container before its config is removed.


Diffs
-

  src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55 
  src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29 
  src/resource_provider/storage/provider.hpp 
5a371b19289c6e39fedd4cda65fa8be432d095e6 
  src/resource_provider/storage/provider.cpp 
6475f653263337c381b6080695d09c49e5ea8fcf 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 68762: Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

Tested container cleanup in `AgentResourceProviderConfigApiTest.Remove`.


Diffs
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
e6a68bae1a9e3e773ea45deae4951664ab81a857 


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


Testing
---

make check

The test will fail without the next patch.


Thanks,

Chun-Hung Hsiao

Review Request 68761: Made the actors in resource providers managed by libprocess.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

Since the local resource provider daemon actively destructs local
resource providers, we should make the actors managed by libprocess
and avoid blocking waits during resource provider destructions.


Diffs
-

  include/mesos/v1/resource_provider.hpp 
15ede9faf96a981427ed8b3f28b170c893507bc5 
  src/resource_provider/driver.cpp eda019c1ad8893e14acc5979b350af4a099138be 
  src/resource_provider/storage/provider.hpp 
5a371b19289c6e39fedd4cda65fa8be432d095e6 
  src/resource_provider/storage/provider.cpp 
6475f653263337c381b6080695d09c49e5ea8fcf 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 68760: Added a `terminate` function to `ContainerDaemon`.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

The `terminate` function will send a kill to its monitoring container
and cause the monitor loop to stop. The loop will be stopped after
the `postStopHook` is called.


Diffs
-

  src/slave/container_daemon.hpp a58140dd787524b0438ba4719fa7e3365ab2fa17 
  src/slave/container_daemon.cpp e1b0812b8b467ee4061b23d39552e2417d65a14a 
  src/slave/container_daemon_process.hpp 
a5d19a04223f6f595e97ec83962558639fd51f7b 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 68759: Cleaned up the `ContainerDaemon` implementation.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

This patch made the following changes to the container daemon:
1. Made it stores the launch parameters instead of an `agent::Call`.
2. Cleaned up the log messages.
3. Made the process managed by libprocess.


Diffs
-

  src/slave/container_daemon.hpp a58140dd787524b0438ba4719fa7e3365ab2fa17 
  src/slave/container_daemon.cpp e1b0812b8b467ee4061b23d39552e2417d65a14a 
  src/slave/container_daemon_process.hpp 
a5d19a04223f6f595e97ec83962558639fd51f7b 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 68758: Added unit tests for adding/updating invalid resource provider configs.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

Added unit tests for adding/updating invalid resource provider configs.


Diffs
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
e6a68bae1a9e3e773ea45deae4951664ab81a857 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Review Request 68757: Removed `ROOT` requirements for `AgentResourceProviderConfigApiTest`.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

These tests required `ROOT` in order to use `filesystem/linux`
isolation this is not a requirement anymore so we can run the tests in
general. These tests appear to be able to run in parallel as well.

We also changed the `AddConflict` test a bit to make it more robust.


Diffs
-

  src/tests/agent_resource_provider_config_api_tests.cpp 
e6a68bae1a9e3e773ea45deae4951664ab81a857 


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


Testing
---

make check
sudo make check


Thanks,

Chun-Hung Hsiao

Review Request 68756: Performed RP-specific validations when adding/updating RP configs.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

Each type of RP might have some specific validations for RP info. For
example, SLRP requires the `storage` field to be set. This patch makes
the local RP daemon to perform such validations when adding/updating
configs, so the `ADD_RESOURCE_PROVIDER_CONFIG` and
`UPDATE_RESOURCE_PROVIDER_CONFIG` calls can fail fast.


Diffs
-

  src/resource_provider/daemon.cpp 0a76cc6d1d34413674d1af1aa514679a4d2b7b55 
  src/resource_provider/local.hpp 20bcc78d3fe847e03526fa59116bdbac92ec1e29 
  src/resource_provider/local.cpp 801e6c430ed91315d87f8a45b8f3ed128beca4fc 
  src/resource_provider/storage/provider.hpp 
5a371b19289c6e39fedd4cda65fa8be432d095e6 
  src/resource_provider/storage/provider.cpp 
6475f653263337c381b6080695d09c49e5ea8fcf 
  src/slave/http.cpp f8199af50cf0b438c29afe751fcd8949d7e8 


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


Testing
---

make check

New tests added in the next patch.


Thanks,

Chun-Hung Hsiao

Review Request 68755: Removed unnecessary failure handling in agent HTTP API handlers.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


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


Repository: mesos


Description
---

The current agent HTTP API handlers either unnecessarily handle failures
on always-ready futures, or return "500 Internal Server Error"
unnecessarily. This patch removes those unnecessarily code.


Diffs
-

  src/slave/http.cpp f8199af50cf0b438c29afe751fcd8949d7e8 


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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 68749: Fixed the CSI protobuf build dependencies.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68749 was successfully built and tested.

Reviews applied: `['68749']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2353/mesos-review-68749

- Mesos Reviewbot Windows


On Sept. 18, 2018, 8:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> ---
> 
> (Updated Sept. 18, 2018, 8:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory as necessary.
> 
> 
> Diffs
> -
> 
>   Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa 
>   src/Makefile.am b0d63e2feef4186443ff9620a96473799a9f 
> 
> 
> Diff: https://reviews.apache.org/r/68749/diff/2/
> 
> 
> Testing
> ---
> 
> Verified that `./support/docker-build.sh` fails after updating 
> `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
> 
> 
> Thanks,
> 
> James Peach
> 
>

[GitHub] Gilbert88 opened a new pull request #313: BugFix backporting 1.6.x

[GitBox]

Gilbert88 opened a new pull request #313: BugFix backporting 1.6.x
URL: https://github.com/apache/mesos/pull/313
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] Gilbert88 opened a new pull request #312: BugFix backporting 1.5.x

[GitBox]

Gilbert88 opened a new pull request #312: BugFix backporting 1.5.x
URL: https://github.com/apache/mesos/pull/312
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] Gilbert88 opened a new pull request #311: BugFix backporting 1.4.x

[GitBox]

Gilbert88 opened a new pull request #311: BugFix backporting 1.4.x
URL: https://github.com/apache/mesos/pull/311
 
 
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: Review Request 68730: Reserve sort output vector in the sorters.

[Meng Zhu]

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


Ship it!




Ship It!

- Meng Zhu


On Sept. 18, 2018, 2:20 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68730/
> ---
> 
> (Updated Sept. 18, 2018, 2:20 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reserves the sort output vector consistently based on
> the number of clients, in both the DRF and random sorters.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 07e5482f37da8585d0313e9f70bd8105efa19533 
>   src/master/allocator/sorter/random/sorter.cpp 
> d17f8afa96b66e63f56ccaa472ad398f8f517b65 
> 
> 
> Diff: https://reviews.apache.org/r/68730/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68729: Simplified the weight lookup logic in the sorters.

[Meng Zhu]

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


Ship it!




Ship It!

- Meng Zhu


On Sept. 18, 2018, 2:20 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68729/
> ---
> 
> (Updated Sept. 18, 2018, 2:20 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Simplified the weight lookup logic in the sorters.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 5a4fa5e2dca61168923261230b1f5c245354cbb7 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 07e5482f37da8585d0313e9f70bd8105efa19533 
>   src/master/allocator/sorter/random/sorter.hpp 
> 7f6c0de70e3ae03d7362fb9e140b93435e530499 
>   src/master/allocator/sorter/random/sorter.cpp 
> d17f8afa96b66e63f56ccaa472ad398f8f517b65 
> 
> 
> Diff: https://reviews.apache.org/r/68729/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68729: Simplified the weight lookup logic in the sorters.

[Benjamin Mahler]

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

(Updated Sept. 18, 2018, 9:20 p.m.)


Review request for mesos, Gastón Kleiman and Meng Zhu.


Changes
---

Updated the random sorter as well.


Summary (updated)
-

Simplified the weight lookup logic in the sorters.


Repository: mesos


Description (updated)
---

Simplified the weight lookup logic in the sorters.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.hpp 
5a4fa5e2dca61168923261230b1f5c245354cbb7 
  src/master/allocator/sorter/drf/sorter.cpp 
07e5482f37da8585d0313e9f70bd8105efa19533 
  src/master/allocator/sorter/random/sorter.hpp 
7f6c0de70e3ae03d7362fb9e140b93435e530499 
  src/master/allocator/sorter/random/sorter.cpp 
d17f8afa96b66e63f56ccaa472ad398f8f517b65 


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

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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 68731: Added a ScalarResourceQuantities type to improve sorters performance.

[Benjamin Mahler]

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

(Updated Sept. 18, 2018, 9:20 p.m.)


Review request for mesos, Gastón Kleiman and Meng Zhu.


Changes
---

Updated the random sorter as well.


Summary (updated)
-

Added a ScalarResourceQuantities type to improve sorters performance.


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


Repository: mesos


Description
---

This type replaces the use of hashmaps keyed by resource names in
favor of storing vectors of `pair`, in order
to avoid the performance penalty of using hashmaps.

Running *HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/21 shows
the following improvement:

Using 1 agents and 1000 frameworks
Added 1000 frameworks in 42.49ms -> 42.85ms (no change)
Added 1 agents in 7.69secs -> 4.89secs (normalized: 1 -> 0.64)
round 0 allocate() took 5.42secs -> 3.53secs (nomralized: 1 -> 0.65)


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.hpp 
5a4fa5e2dca61168923261230b1f5c245354cbb7 
  src/master/allocator/sorter/random/sorter.hpp 
7f6c0de70e3ae03d7362fb9e140b93435e530499 
  src/master/allocator/sorter/sorter.hpp 
432ccfe24ed2854df9cc186a8691009cbdb763c7 


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

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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 68730: Reserve sort output vector in the sorters.

[Benjamin Mahler]

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

(Updated Sept. 18, 2018, 9:20 p.m.)


Review request for mesos, Gastón Kleiman and Meng Zhu.


Changes
---

Updated the random sorter as well.


Summary (updated)
-

Reserve sort output vector in the sorters.


Repository: mesos


Description (updated)
---

This reserves the sort output vector consistently based on
the number of clients, in both the DRF and random sorters.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.cpp 
07e5482f37da8585d0313e9f70bd8105efa19533 
  src/master/allocator/sorter/random/sorter.cpp 
d17f8afa96b66e63f56ccaa472ad398f8f517b65 


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

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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 68749: Fixed the CSI protobuf build dependencies.

[James Peach]

> On Sept. 18, 2018, 8:14 p.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Lines 483 (patched)
> > 
> >
> > Can we use `$(MKDIR_P) $(@D)` for consistency and easier maintenance? 
> > The only difference between this rule and e.g., the Java protobuf rules 
> > below seems to be that this one uses multiple targets.

I verified that when you have multiple targets `$(@D)` gives you the dirname of 
the first target.


- James


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


On Sept. 18, 2018, 8:57 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> ---
> 
> (Updated Sept. 18, 2018, 8:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory as necessary.
> 
> 
> Diffs
> -
> 
>   Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa 
>   src/Makefile.am b0d63e2feef4186443ff9620a96473799a9f 
> 
> 
> Diff: https://reviews.apache.org/r/68749/diff/2/
> 
> 
> Testing
> ---
> 
> Verified that `./support/docker-build.sh` fails after updating 
> `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 68749: Fixed the CSI protobuf build dependencies.

[James Peach]

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

(Updated Sept. 18, 2018, 8:57 p.m.)


Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description (updated)
---

Generating the CSI protobufs depends on the `$(builddir)/include/csi`
directory being created at configuration time. This only happens
when automake build dependencies are enabled, however. In some
distributions, rpmbuild will pass `--disable-dependency-tracking`,
which will disable automake dependency tracking and prevent this
directory being created. The fix is to simply ensure the CSI protobuf
generation rules create the output directory as necessary.


Diffs (updated)
-

  Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa 
  src/Makefile.am b0d63e2feef4186443ff9620a96473799a9f 


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

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


Testing
---

Verified that `./support/docker-build.sh` fails after updating 
`AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.


Thanks,

James Peach

Re: Review Request 68749: Fixed the CSI protobuf build dependencies.

[Benjamin Bannier]

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


Fix it, then Ship it!





Makefile.am
Lines 55 (patched)


`s/we don't have/we do not/`



src/Makefile.am
Lines 483 (patched)


Can we use `$(MKDIR_P) $(@D)` for consistency and easier maintenance? The 
only difference between this rule and e.g., the Java protobuf rules below seems 
to be that this one uses multiple targets.



src/Makefile.am
Lines 487 (patched)


Ditto.


- Benjamin Bannier


On Sept. 18, 2018, 7:39 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> ---
> 
> (Updated Sept. 18, 2018, 7:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory is necessary.
> 
> 
> Diffs
> -
> 
>   Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa 
>   src/Makefile.am b0d63e2feef4186443ff9620a96473799a9f 
> 
> 
> Diff: https://reviews.apache.org/r/68749/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that `./support/docker-build.sh` fails after updating 
> `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 68732: Cached weights in the sorters nodes.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2352/mesos-review-68732

Relevant logs:

- 
[apply-review-68732.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2352/mesos-review-68732/logs/apply-review-68732.log):

```
error: patch failed: src/master/allocator/sorter/random/sorter.hpp:117
error: src/master/allocator/sorter/random/sorter.hpp: patch does not apply
error: patch failed: src/master/allocator/sorter/random/sorter.cpp:540
error: src/master/allocator/sorter/random/sorter.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 18, 2018, 7:58 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68732/
> ---
> 
> (Updated Sept. 18, 2018, 7:58 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This avoids making excessive map lookups each time we calculate the
> share for a node in the tree. Now, when the weight is needed, the
> value is cached. If the weight gets updated, we update the cached
> value. This approach proved cleaner than trying to ensure freshly
> constructed nodes have the right weight.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 5a4fa5e2dca61168923261230b1f5c245354cbb7 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 07e5482f37da8585d0313e9f70bd8105efa19533 
>   src/master/allocator/sorter/random/sorter.hpp 
> 7f6c0de70e3ae03d7362fb9e140b93435e530499 
>   src/master/allocator/sorter/random/sorter.cpp 
> d17f8afa96b66e63f56ccaa472ad398f8f517b65 
> 
> 
> Diff: https://reviews.apache.org/r/68732/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68679: Fixed stout `FsTest.Used` test.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['68679']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2351/mesos-review-68679

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2351/mesos-review-68679/logs/mesos-tests.log):

```
I0918 20:00:42.301631 21752 exec.cpp:445] Executor asked to shutdown
I0918 20:00:42.301631 36384 executor.cpp:182] Received SHUTDOWN event
I0918 20:00:42.301631 36384 executor.cpp:796] Shutting down
I0918 20:00:42.301631 36384 executor.cpp:909] Sending SIGTERM to process tree 
at pid 34cpp:11030] Removing task aa9ac3b5-34d5-4b84-ab73-04dacb2087e9 with 
resources cpus(allocated: *):4; mem(allocated: *):2048; disk(allocated: 
*):1024; ports(allocated: *):[31000-32000] of framework 
117b3789-f2a2-4891-a353-eafe9e80195b- on agent 
117b3789-f2a2-4891-a353-eafe9e80195b-S0 at slave(462)@192.10.1.5:49430 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
W0918 20:00:42.300631 35304 slave.cpp:3917] Ignoring shutdown framework 
117b3789-f2a2-4891-a353-eafe9e80195b- because it is terminating
I0918 20:00:42.303628 35172 master.cpp:1251] Agent 
117b3789-f2a2-4891-a353-eafe9e80195b-S0 at slave(462)@192.10.1.5:49430 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I0918 20:00:42.303628 35172 master.cpp:3267] Disconnecting agent 
117b3789-f2a2-4891-a353-eafe9e80195b-S0 at slave(462)@192.10.1.5:49430 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0918 20:00:42.303628 35172 master.cpp:3286] Deactivating agent 
117b3789-f2a2-4891-a353-eafe9e80195b-[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (687 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (708 ms total)

[--] Global test environment tear-down
[==] 1051 tests from 103 test cases ran. (493012 ms total)
[  PASSED  ] 1050 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchImage

 1 FAILED TEST
  YOU HAVE 231 DISABLED TESTS

S0 at slave(462)@192.10.1.5:49430 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I0918 20:00:42.304633 35248 hierarchical.cpp:359] Removed framework 
117b3789-f2a2-4891-a353-eafe9e80195b-
I0918 20:00:42.304633 35248 hierarchical.cpp:795] Agent 
117b3789-f2a2-4891-a353-eafe9e80195b-S0 deactivated
I0918 20:00:42.305627 33900 containerizer.cpp:2455] Destroying container 
6d3760b7-6f1b-490f-98f8-987aa4dd2bfb in RUNNING state
I0918 20:00:42.305627 33900 containerizer.cpp:3118] Transitioning the state of 
container 6d3760b7-6f1b-490f-98f8-987aa4dd2bfb from RUNNING to DESTROYING
I0918 20:00:42.305627 33900 launcher.cpp:166] Asked to destroy container 
6d3760b7-6f1b-490f-98f8-987aa4dd2bfb
I0918 20:00:42.398115 35304 containerizer.cpp:2957] Container 
6d3760b7-6f1b-490f-98f8-987aa4dd2bfb has exited
I0918 20:00:42.427196 37196 master.cpp:1093] Master terminating
I0918 20:00:42.428109 35116 hierarchical.cpp:637] Removed agent 
117b3789-f2a2-4891-a353-eafe9e80195b-S0
I0918 20:00:42.954097 37156 process.cpp:926] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Sept. 18, 2018, 5:46 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68679/
> ---
> 
> (Updated Sept. 18, 2018, 5:46 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9214
> https://issues.apache.org/jira/browse/MESOS-9214
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `f_bsize` elemement denotes the preferred filesystem block
> size, not the unit that `f_blocks` is counting in. We need to
> use `f_frsize` for that.
> 
> Remove the check for used space on `/dev` since that is not even
> portable across different Linux distributions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> be15395fc38f7b106ddcd886ba426197f2e783d1 
> 
> 
> Diff: https://reviews.apache.org/r/68679/diff/4/
> 
> 
> Testing
> ---
> 
> Ran stout tests on macOS 10.13.2, Fedora 28, Centos 6.9 and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 68732: Cached weights in the sorters nodes.

[Benjamin Mahler]

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

Review request for mesos, Gastón Kleiman and Meng Zhu.


Repository: mesos


Description
---

This avoids making excessive map lookups each time we calculate the
share for a node in the tree. Now, when the weight is needed, the
value is cached. If the weight gets updated, we update the cached
value. This approach proved cleaner than trying to ensure freshly
constructed nodes have the right weight.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
5a4fa5e2dca61168923261230b1f5c245354cbb7 
  src/master/allocator/sorter/drf/sorter.cpp 
07e5482f37da8585d0313e9f70bd8105efa19533 
  src/master/allocator/sorter/random/sorter.hpp 
7f6c0de70e3ae03d7362fb9e140b93435e530499 
  src/master/allocator/sorter/random/sorter.cpp 
d17f8afa96b66e63f56ccaa472ad398f8f517b65 


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


Testing
---

make check


Thanks,

Benjamin Mahler

Re: Review Request 68729: Simplified the weight lookup logic in the sorter.

[Benjamin Mahler]

> On Sept. 18, 2018, 5:22 p.m., Meng Zhu wrote:
> > Random sorter shares the same code. Want to update that as well?

Ah yes, good catch!


- Benjamin


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


On Sept. 17, 2018, 2:23 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68729/
> ---
> 
> (Updated Sept. 17, 2018, 2:23 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 5a4fa5e2dca61168923261230b1f5c245354cbb7 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 07e5482f37da8585d0313e9f70bd8105efa19533 
> 
> 
> Diff: https://reviews.apache.org/r/68729/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68730: Reserve sort output vector in the DRF sorter.

[Benjamin Mahler]

> On Sept. 18, 2018, 5:33 p.m., Meng Zhu wrote:
> > src/master/allocator/sorter/drf/sorter.cpp
> > Lines 553 (patched)
> > 
> >
> > This is assuming all clients are active? What if many clients are 
> > deactivated (suppressed)? Then this may make things worse?
> > 
> > Can we keep tracking of the number of active/inactive nodes?
> > 
> > Also, I find `clients.size()` to be more readable.

Hm.. yeah I will replace this with a TODO to reserve the number of active 
leaves instead.


- Benjamin


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


On Sept. 17, 2018, 2:24 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68730/
> ---
> 
> (Updated Sept. 17, 2018, 2:24 a.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 07e5482f37da8585d0313e9f70bd8105efa19533 
> 
> 
> Diff: https://reviews.apache.org/r/68730/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68749: Fixed the CSI protobuf build dependencies.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68749 was successfully built and tested.

Reviews applied: `['68749']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2350/mesos-review-68749

- Mesos Reviewbot Windows


On Sept. 18, 2018, 5:39 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68749/
> ---
> 
> (Updated Sept. 18, 2018, 5:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-9240
> https://issues.apache.org/jira/browse/MESOS-9240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Generating the CSI protobufs depends on the `$(builddir)/include/csi`
> directory being created at configuration time. This only happens
> when automake build dependencies are enabled, however. In some
> distributions, rpmbuild will pass `--disable-dependency-tracking`,
> which will disable automake dependency tracking and prevent this
> directory being created. The fix is to simply ensure the CSI protobuf
> generation rules create the output directory is necessary.
> 
> 
> Diffs
> -
> 
>   Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa 
>   src/Makefile.am b0d63e2feef4186443ff9620a96473799a9f 
> 
> 
> Diff: https://reviews.apache.org/r/68749/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that `./support/docker-build.sh` fails after updating 
> `AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 68711: Added `task ls` command to new CLI.

[Eric Chung]

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




src/python/cli_new/lib/cli/tasks.py
Lines 28 (patched)


now that we're using python3, why not start adding type annotations? e.g.
```
def get_task(tasks: dict, task_id: str):
  ...
```
this will help greatly with avoiding unnecessary bugs


- Eric Chung


On Sept. 17, 2018, 10:37 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68711/
> ---
> 
> (Updated Sept. 17, 2018, 10:37 a.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Bugs: MESOS-8795
> https://issues.apache.org/jira/browse/MESOS-8795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `task ls` command to new CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/agents.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> 644e256ce898784ce3511c30a373595ace149db9 
>   src/python/cli_new/lib/cli/tasks.py PRE-CREATION 
>   src/python/cli_new/lib/cli/tests/task.py 
> b54ade557f579a489e459f6022807146e0211fb0 
> 
> 
> Diff: https://reviews.apache.org/r/68711/diff/6/
> 
> 
> Testing
> ---
> 
> Tested manually and:
> ```
> (mesos-cli) bash-3.2$ mesos-cli-tests
> Running the Mesos CLI unit tests
> 
> TestAgentPlugin
> test_list (cli.tests.agent.TestAgentPlugin) ... ok
> 
> TestInfrastructure
> test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
> test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok
> 
> TestTaskPlugin
> test_list (cli.tests.task.TestTaskPlugin) ... ok
> test_ls (cli.tests.task.TestTaskPlugin) ... ok
> 
> --
> Ran 5 tests in 9.656s
> 
> OK
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 68679: Fixed stout `FsTest.Used` test.

[James Peach]

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

(Updated Sept. 18, 2018, 5:46 p.m.)


Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

The `f_bsize` elemement denotes the preferred filesystem block
size, not the unit that `f_blocks` is counting in. We need to
use `f_frsize` for that.

Remove the check for used space on `/dev` since that is not even
portable across different Linux distributions.


Diffs (updated)
-

  3rdparty/stout/tests/os/filesystem_tests.cpp 
be15395fc38f7b106ddcd886ba426197f2e783d1 


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

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


Testing
---

Ran stout tests on macOS 10.13.2, Fedora 28, Centos 6.9 and Ubuntu 14.04.


Thanks,

James Peach

Review Request 68749: Fixed the CSI protobuf build dependencies.

[James Peach]

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

Review request for mesos, Benjamin Bannier, Joseph Wu, and Vinod Kone.


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


Repository: mesos


Description
---

Generating the CSI protobufs depends on the `$(builddir)/include/csi`
directory being created at configuration time. This only happens
when automake build dependencies are enabled, however. In some
distributions, rpmbuild will pass `--disable-dependency-tracking`,
which will disable automake dependency tracking and prevent this
directory being created. The fix is to simply ensure the CSI protobuf
generation rules create the output directory is necessary.


Diffs
-

  Makefile.am 25028a949b9a3420bf3a379d65bcbae8e028aeaa 
  src/Makefile.am b0d63e2feef4186443ff9620a96473799a9f 


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


Testing
---

Verified that `./support/docker-build.sh` fails after updating 
`AM_DISTCHECK_CONFIGURE_FLAGS` and then succeeds.


Thanks,

James Peach

Re: Review Request 68730: Reserve sort output vector in the DRF sorter.

[Meng Zhu]

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




src/master/allocator/sorter/drf/sorter.cpp
Lines 553 (patched)


This is assuming all clients are active? What if many clients are 
deactivated (suppressed)? Then this may make things worse?

Can we keep tracking of the number of active/inactive nodes?

Also, I find `clients.size()` to be more readable.


- Meng Zhu


On Sept. 16, 2018, 7:24 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68730/
> ---
> 
> (Updated Sept. 16, 2018, 7:24 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 07e5482f37da8585d0313e9f70bd8105efa19533 
> 
> 
> Diff: https://reviews.apache.org/r/68730/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68729: Simplified the weight lookup logic in the sorter.

[Meng Zhu]

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


Ship it!




Random sorter shares the same code. Want to update that as well?

- Meng Zhu


On Sept. 16, 2018, 7:23 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68729/
> ---
> 
> (Updated Sept. 16, 2018, 7:23 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 5a4fa5e2dca61168923261230b1f5c245354cbb7 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 07e5482f37da8585d0313e9f70bd8105efa19533 
> 
> 
> Diff: https://reviews.apache.org/r/68729/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 62187: Fixed broken pipe error in IOSwitchboard.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62187 was successfully built and tested.

Reviews applied: `['68232', '68230', '68231', '65168', '62187']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2349/mesos-review-62187

- Mesos Reviewbot Windows


On Sept. 10, 2018, 6 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62187/
> ---
> 
> (Updated Sept. 10, 2018, 6 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8545
> https://issues.apache.org/jira/browse/MESOS-8545
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We force IOSwitchboard to return a final response to the client for the
> `ATTACH_CONTAINER_INPUT` call after IO redirects are finished. In this
> case, we don't read remaining messages from the input stream. So the
> agent might send an acknowledgment for the request before IOSwitchboard
> has received remaining messages. We need to delay termination of
> IOSwitchboard to give it a chance to read the remaining messages.
> Otherwise, the agent might get `HTTP 500` "broken pipe" while
> attempting to write the final message.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/62187/diff/6/
> 
> 
> Testing
> ---
> 
> internal CI (3x times)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 62187: Fixed broken pipe error in IOSwitchboard.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Sept. 10, 2018, 6 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62187/
> ---
> 
> (Updated Sept. 10, 2018, 6 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8545
> https://issues.apache.org/jira/browse/MESOS-8545
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We force IOSwitchboard to return a final response to the client for the
> `ATTACH_CONTAINER_INPUT` call after IO redirects are finished. In this
> case, we don't read remaining messages from the input stream. So the
> agent might send an acknowledgment for the request before IOSwitchboard
> has received remaining messages. We need to delay termination of
> IOSwitchboard to give it a chance to read the remaining messages.
> Otherwise, the agent might get `HTTP 500` "broken pipe" while
> attempting to write the final message.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 52b0e521ed1c651c90b3a3df7c4df576288bf400 
> 
> 
> Diff: https://reviews.apache.org/r/62187/diff/5/
> 
> 
> Testing
> ---
> 
> internal CI (3x times)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 65168: Fixed HTTP errors caused by dropped HTTP responses by IOSwitchboard.

[Alexander Rukletsov]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/io/switchboard.cpp
Lines 1379-1389 (patched)


Let's extract it into a separate method



src/slave/http.cpp
Lines 3138 (patched)


s/Since/After



src/slave/http.cpp
Lines 3157 (patched)


please `snake_case` instead of `camelCase`


- Alexander Rukletsov


On Sept. 6, 2018, 3:22 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65168/
> ---
> 
> (Updated Sept. 6, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Qian Zhang.
> 
> 
> Bugs: MESOS-8545
> https://issues.apache.org/jira/browse/MESOS-8545
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, IOSwitchboard process could terminate before all HTTP
> responses had been sent to the agent. In the case of
> `ATTACH_CONTAINER_INPUT` call, we could drop a final HTTP `200 OK`
> response, so the agent got broken HTTP connection for the call.
> This patch introduces an acknowledgment for the received response
> for the `ATTACH_CONTAINER_INPUT` call. This acknowledgment is a new
> type of control messages for the `ATTACH_CONTAINER_INPUT` call. When
> IOSwitchboard receives an acknowledgment, and io redirects are
> finished, it terminates itself. That guarantees that the agent always
> receives a response for the `ATTACH_CONTAINER_INPUT` call.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> 52b0e521ed1c651c90b3a3df7c4df576288bf400 
>   src/slave/http.hpp 78200874cb03a3d5af4ee0443c03a1d94cd494cc 
>   src/slave/http.cpp fb923686e92ff635e99cc2ccc6316f0282ab3e3a 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> c00f6a9018801e6b8144ff3beae549c15c7d3917 
> 
> 
> Diff: https://reviews.apache.org/r/65168/diff/10/
> 
> 
> Testing
> ---
> 
> internal CI (3x times)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 68720: Added new member function to stout PicoJSON context.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68720 was successfully built and tested.

Reviews applied: `['68718', '68720']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2348/mesos-review-68720

- Mesos Reviewbot Windows


On Sept. 18, 2018, 9:32 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68720/
> ---
> 
> (Updated Sept. 18, 2018, 9:32 a.m.)
> 
> 
> Review request for Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit adds the new `parse_object_stop()` callback,
> which was added in the previous patch, to stout's internal
> PicoJSON context.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/json.hpp 
> 62a813996d2d45b01a96b09286e8723025d84f07 
>   3rdparty/stout/tests/json_tests.cpp 
> ee44edfa3be655b6779e25376aa3751d95df4c41 
> 
> 
> Diff: https://reviews.apache.org/r/68720/diff/2/
> 
> 
> Testing
> ---
> 
> https://jenkins.mesosphere.com/service/jenkins/view/Mesos/job/mesos/job/Mesos_CI-build/4421/
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 68720: Added new member function to stout PicoJSON context.

[Benno Evers]

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

(Updated Sept. 18, 2018, 9:32 a.m.)


Review request for Alexander Rukletsov.


Changes
---

Added test in stout; reversed depth counting order


Repository: mesos


Description
---

This commit adds the new `parse_object_stop()` callback,
which was added in the previous patch, to stout's internal
PicoJSON context.


Diffs (updated)
-

  3rdparty/stout/include/stout/json.hpp 
62a813996d2d45b01a96b09286e8723025d84f07 
  3rdparty/stout/tests/json_tests.cpp ee44edfa3be655b6779e25376aa3751d95df4c41 


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

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


Testing
---

https://jenkins.mesosphere.com/service/jenkins/view/Mesos/job/mesos/job/Mesos_CI-build/4421/


Thanks,

Benno Evers