Re: Review Request 72161: Added patch for RapidJSON.

2020-03-05 Thread Qian Zhang


> On Feb. 28, 2020, 12:59 a.m., Benjamin Bannier wrote:
> > Is the plan to upstream this patch? We usually try to enable building 
> > against unbundled dependencies and depending on custom behavior makes that 
> > impossible. My hunch would be to not make functional changes like the one 
> > in this patch to dependencies (changes for building are usually fine).
> 
> Greg Mann wrote:
> Yea, attempting to upstream is a good idea. Qian would you like to open a 
> PR, or would you like me to? https://github.com/Tencent/rapidjson

I am not sure if it is something which will be accepted by upstream, what I did 
in this patch is to make RapidJSON confirm to the rule defined by ProtoBuf 
(https://developers.google.com/protocol-buffers/docs/proto3#json), I am not 
sure if that is a general rule that RapidJSON will care about.


- Qian


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


On March 2, 2020, 2:27 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72161/
> ---
> 
> (Updated March 2, 2020, 2:27 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10064
> https://issues.apache.org/jira/browse/MESOS-10064
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit updates the writer of RapidJSON to write infinite
> floating point numbers as "Infinity" and "-Infinity" (i.e.,
> with double quotes) rather than Infinity and -Infinity. This
> is to ensure the strings converted from JSON objects conform
> to the rule defined by Protobuf:
> https://developers.google.com/protocol-buffers/docs/proto3#json
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt c45d742684ba4b3b4abc57ae0bcb85a879c68bfd 
>   3rdparty/rapidjson-1.1.0.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72161/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 72202: Fixed the broken PathTest.PathIteration on windows.

2020-03-05 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [72201, 72202]

Passed command: export OS='ubuntu:16.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers 
--disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/jenkins/buildbot.sh

- Mesos Reviewbot


On March 5, 2020, 11:11 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72202/
> ---
> 
> (Updated March 5, 2020, 11:11 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, and Joseph Wu.
> 
> 
> Bugs: MESOS-10100
> https://issues.apache.org/jira/browse/MESOS-10100
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test uses only linux style paths and fails on windows. This
> happened because this code was introduced recently but there was
> no testing or CI running on windows to catch it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/path_tests.cpp 
> d418d1b53886042400365d32f7999e84059911c3 
> 
> 
> Diff: https://reviews.apache.org/r/72202/diff/1/
> 
> 
> Testing
> ---
> 
> Ran through a windows CI.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Review Request 72202: Fixed the broken PathTest.PathIteration on windows.

2020-03-05 Thread Benjamin Mahler

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

Review request for mesos, Andrei Sekretenko, Greg Mann, and Joseph Wu.


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


Repository: mesos


Description
---

This test uses only linux style paths and fails on windows. This
happened because this code was introduced recently but there was
no testing or CI running on windows to catch it.


Diffs
-

  3rdparty/stout/tests/path_tests.cpp d418d1b53886042400365d32f7999e84059911c3 


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


Testing
---

Ran through a windows CI.


Thanks,

Benjamin Mahler



Review Request 72201: Fixed the broken PathTest.Relative on Windows.

2020-03-05 Thread Benjamin Mahler

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

Review request for mesos, Andrei Sekretenko, Greg Mann, and Joseph Wu.


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


Repository: mesos


Description
---

This test uses only linux style paths and fails on windows. This
happened because this code was introduced recently but there was
no testing or CI running on windows to catch it.


Diffs
-

  3rdparty/stout/tests/path_tests.cpp d418d1b53886042400365d32f7999e84059911c3 


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


Testing
---

Ran through a windows CI.


Thanks,

Benjamin Mahler



Re: Review Request 71983: Added a test `CgroupsIsolatorTest.ROOT_CGROUPS_CFS_TaskGroupLimits`.

2020-03-05 Thread Greg Mann

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




src/tests/containerizer/cgroups_isolator_tests.cpp
Lines 716 (patched)


s/memory's/memory/



src/tests/containerizer/cgroups_isolator_tests.cpp
Lines 961 (patched)


This looks racy; we shouldn't rely on the running wall clock to trigger 
offers in a test like this. Could you pause the clock and advance manually to 
trigger offers? There are some existing examples in the tests of this; you 
could check 
'SchedulerTest.OperationFeedbackValidationWithResourceProviderCapability'.


- Greg Mann


On Jan. 13, 2020, 8:06 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71983/
> ---
> 
> (Updated Jan. 13, 2020, 8:06 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10050
> https://issues.apache.org/jira/browse/MESOS-10050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `CgroupsIsolatorTest.ROOT_CGROUPS_CFS_TaskGroupLimits`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> f72e6cdab417368e63349915114aeed586e0ef0e 
> 
> 
> Diff: https://reviews.apache.org/r/71983/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71956: Added a test `ROOT_CGROUPS_CFS_CommandTaskLimits`.

2020-03-05 Thread Greg Mann

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


Fix it, then Ship it!





src/tests/containerizer/cgroups_isolator_tests.cpp
Lines 527 (patched)


Isn't the agent here currently using all of the host memory?


- Greg Mann


On Jan. 15, 2020, 2:24 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71956/
> ---
> 
> (Updated Jan. 15, 2020, 2:24 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10047
> https://issues.apache.org/jira/browse/MESOS-10047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ROOT_CGROUPS_CFS_CommandTaskLimits`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> f72e6cdab417368e63349915114aeed586e0ef0e 
> 
> 
> Diff: https://reviews.apache.org/r/71956/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71955: Add a new parameter `resourceLimits` to the `createTask` methods.

2020-03-05 Thread Greg Mann

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



Please change the commit message to past-tense.

- Greg Mann


On Jan. 6, 2020, 8:49 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71955/
> ---
> 
> (Updated Jan. 6, 2020, 8:49 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10047
> https://issues.apache.org/jira/browse/MESOS-10047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new parameter `resourceLimits` to the `createTask` methods.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 73b18663d4dbf0ee179c298ea77b548d5de40921 
> 
> 
> Diff: https://reviews.apache.org/r/71955/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71953: Updated the test `ROOT_CGROUPS_CFS_EnableCfs` to check CFS quota.

2020-03-05 Thread Greg Mann

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




src/tests/containerizer/cgroups_isolator_tests.cpp
Lines 474 (patched)


Instead of using `path::join()`, is there a cgroup helper function we can 
use to get this path?



src/tests/containerizer/cgroups_isolator_tests.cpp
Lines 489-496 (patched)


Does it make sense to put this part in either the 'MemoryForward' or 
'MemoryBackward' tests, instead of here? Or to create a new test which verifies 
basic memory isolator behavior?


- Greg Mann


On Jan. 15, 2020, 2:24 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71953/
> ---
> 
> (Updated Jan. 15, 2020, 2:24 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10047
> https://issues.apache.org/jira/browse/MESOS-10047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the test `ROOT_CGROUPS_CFS_EnableCfs` to check CFS quota.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> f72e6cdab417368e63349915114aeed586e0ef0e 
> 
> 
> Diff: https://reviews.apache.org/r/71953/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71955: Add a new parameter `resourceLimits` to the `createTask` methods.

2020-03-05 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On Jan. 6, 2020, 8:49 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71955/
> ---
> 
> (Updated Jan. 6, 2020, 8:49 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10047
> https://issues.apache.org/jira/browse/MESOS-10047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new parameter `resourceLimits` to the `createTask` methods.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 73b18663d4dbf0ee179c298ea77b548d5de40921 
> 
> 
> Diff: https://reviews.apache.org/r/71955/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71952: Set resource limits when updating executor container.

2020-03-05 Thread Greg Mann

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




src/common/protobuf_utils.cpp
Lines 398 (patched)


Could you move this to the patch where you add the limits field to the 
`Task` message?



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


Seems like you could just change this entire function into a helper which 
we use in two places?

```
google::protobuf::Map computeExecutorLimits(
const std::vector& tasks,
const Resources& executorResources,
bool enableCfs);
```


- Greg Mann


On Feb. 26, 2020, 12:25 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71952/
> ---
> 
> (Updated Feb. 26, 2020, 12:25 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10050
> https://issues.apache.org/jira/browse/MESOS-10050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set resource limits when updating executor container.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 7fe4a44b1e7ded998dffb0490c1d61ced697ebd5 
>   src/slave/slave.hpp 03279db082bdba23dbfeb2d93081a908e609aec2 
>   src/slave/slave.cpp cce275a504effae7a6b71dd333ce8a300d1ce5be 
> 
> 
> Diff: https://reviews.apache.org/r/71952/diff/6/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71952: Set resource limits when updating executor container.

2020-03-05 Thread Greg Mann


> On Feb. 28, 2020, 5:25 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 11198 (patched)
> > 
> >
> > The similarity of this function body to some of the code in 
> > https://reviews.apache.org/r/71858/ is unfortunate... do you think we 
> > should factor it out into some helper which looks through a list of tasks 
> > and calculates the limits and requests? Maybe something like:
> > 
> > ```
> > struct TaskResourceMetadata
> > {
> >   Resources totalTaskResources;
> >   map> totalLimits;
> >   map totalRequests;
> > };
> > 
> > TaskResourceMetadata computeTaskResourceMetadata(vector tasks)
> > {
> >   TaskResourceMetadata result;
> >   std::vector names({"cpus", "mem"});
> > 
> >   foreach (const TaskInfo& _task, tasks) {
> > result.totalTaskResources += _task.resources();
> > 
> > foreach (const std::string _name, names) {
> >   // Lazily initialize requests and limits with `operator[]`.
> >   if (_task.limits().count(_name)) {
> > setLimit(result.totalLimits[_name], _task.limits().at(_name));
> >   } else {
> > Option taskRequest =
> >   Resources(_task.resources()).get(_name);
> > 
> > if (taskRequest.isSome()) {
> >   result.totalRequests[_name] += taskRequest.get();
> > }
> >   }
> > }
> >   }
> > 
> >   return result;
> > }
> > ```
> > 
> > WDYT? Is it worth it?

See my comment below regarding changing this method into a function helper.


- Greg


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


On Feb. 26, 2020, 12:25 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71952/
> ---
> 
> (Updated Feb. 26, 2020, 12:25 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10050
> https://issues.apache.org/jira/browse/MESOS-10050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set resource limits when updating executor container.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 7fe4a44b1e7ded998dffb0490c1d61ced697ebd5 
>   src/slave/slave.hpp 03279db082bdba23dbfeb2d93081a908e609aec2 
>   src/slave/slave.cpp cce275a504effae7a6b71dd333ce8a300d1ce5be 
> 
> 
> Diff: https://reviews.apache.org/r/71952/diff/6/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71951: Added resource limits into the `Task` protobuf message.

2020-03-05 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On Jan. 5, 2020, 2:07 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71951/
> ---
> 
> (Updated Jan. 5, 2020, 2:07 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10050
> https://issues.apache.org/jira/browse/MESOS-10050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added resource limits into the `Task` protobuf message.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto b0f590545df720df644d049e9d8f1e81bdbe814c 
>   include/mesos/v1/mesos.proto 53a7b9bc6190d826868a1633c11c9a0ecf9acf0a 
> 
> 
> Diff: https://reviews.apache.org/r/71951/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71950: Updated containerizer's `update()` method to handle resource limits.

2020-03-05 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On March 4, 2020, 1:43 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71950/
> ---
> 
> (Updated March 4, 2020, 1:43 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10050
> https://issues.apache.org/jira/browse/MESOS-10050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated containerizer's `update()` method to handle resource limits.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 0f838fa63b02d38606a6c484f01d24952d569721 
>   src/slave/containerizer/composing.cpp 
> d854794fc4775fb8a05efc233d488a64b9ef620a 
>   src/slave/containerizer/containerizer.hpp 
> 6a9ebed5e55ad1cfed6340479743ef2bb4c05dab 
>   src/slave/containerizer/docker.hpp 0349f537cef9651427e0e3ed33fd693107e07aa0 
>   src/slave/containerizer/docker.cpp 2a9b2ffcbd01ae916839ae43c8342285ac3e14a2 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 9e86ff43535c4be937baa238442e8f0db8857a27 
>   src/slave/containerizer/mesos/containerizer.cpp 
> d034c02c0f73a2745a063561430a1bce7b552420 
>   src/tests/containerizer.hpp e05ce0323e032911d46d8682661ffe4463fdc276 
>   src/tests/containerizer.cpp 3ac992f8599e8a48f428b506f5bdecc722050c6a 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 689a7220a09f2a58dffdf0dc9fd9f0548600be0e 
>   src/tests/containerizer/mock_containerizer.hpp 
> 8700257a9347199ed6e32b8b6b2a58787d68af03 
>   src/tests/master_draining_tests.cpp 
> a91201e0bfb6f53f70dbdc4649cc344076ef474b 
>   src/tests/master_tests.cpp d0f53b2139bf36ff15a27e438f058f4914df5caa 
>   src/tests/mock_docker.hpp 4a2266fb1239cbc96c7df74b997212e3b3b01c75 
>   src/tests/registrar_zookeeper_tests.cpp 
> 9d3ea5f194374fb3492fdfa8bd2efeef55fc4743 
>   src/tests/scheduler_tests.cpp 299d3a06ca1fd55ef55dc9be323b98a4da3be31a 
>   src/tests/slave_tests.cpp 92fa2996f0e39dded79aa372ddf2390b68b885a2 
> 
> 
> Diff: https://reviews.apache.org/r/71950/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71943: Set container's `memory.limit_in_bytes` to its memory limit.

2020-03-05 Thread Greg Mann

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




src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
Lines 239-240 (patched)


What exactly do you mean here by "must be infinite"? It seems possible that 
it's not infinite? Why do we check this for memory but not for CFS quota?

I might suggest that we just set 'mem.limit_in_bytes' to '-1' here the same 
way we do for CFS quota, and rely on the operator configuring the cgroup root 
correctly.



src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp
Lines 282-290 (original), 310-318 (patched)


Isn't it possible that the memory limit may be lowered multiple times? If 
so, I think `setFunctions` would be empty the second time it is lowered?


- Greg Mann


On Feb. 26, 2020, 12:16 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71943/
> ---
> 
> (Updated Feb. 26, 2020, 12:16 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10048
> https://issues.apache.org/jira/browse/MESOS-10048
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set container's `memory.limit_in_bytes` to its memory limit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> 27d88e91fb784179effd54781f84000fe85c13eb 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
> 
> 
> Diff: https://reviews.apache.org/r/71943/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71886: Set container's `cpu.cfs_quota_us` to its CPU resource limit.

2020-03-05 Thread Greg Mann

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp
Lines 124 (patched)


Maybe store this value to reuse it here and below?

```
const double& effectiveLimit =
  cpuLimit.isSome() ? cpuLimit.get() : cpuRequest;
```


- Greg Mann


On Feb. 26, 2020, 12:12 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71886/
> ---
> 
> (Updated Feb. 26, 2020, 12:12 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10047
> https://issues.apache.org/jira/browse/MESOS-10047
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set container's `cpu.cfs_quota_us` to its CPU resource limit.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 960bd141430387e076a8fab1948d07719613ed90 
> 
> 
> Diff: https://reviews.apache.org/r/71886/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71885: Updated the `update()` method of subsystem to handle resource limits.

2020-03-05 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On Jan. 5, 2020, 2:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71885/
> ---
> 
> (Updated Jan. 5, 2020, 2:03 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10067
> https://issues.apache.org/jira/browse/MESOS-10067
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the `update()` method of subsystem to handle resource limits.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b12b73d8e0161d448075378765e77867521de04e 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> a311ab4495f71bedacd2e99c84c765f0e5fe99d3 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> dc6c7aa1c998c30c8b17db04a38e7a1e28a6a6c1 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.hpp 
> 02e7163b9a7a928352715848947e608885c5242a 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/cpu.cpp 
> 960bd141430387e076a8fab1948d07719613ed90 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.hpp 
> 27d88e91fb784179effd54781f84000fe85c13eb 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystems/memory.cpp 
> 0896d37761a11f55ba4b866d235c3bd2b79dcfba 
> 
> 
> Diff: https://reviews.apache.org/r/71885/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



Re: Review Request 71884: Updated the `update()` method of isolator to handle resource limits.

2020-03-05 Thread Greg Mann

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


Ship it!




Ship It!

- Greg Mann


On Jan. 5, 2020, 2:02 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71884/
> ---
> 
> (Updated Jan. 5, 2020, 2:02 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Greg Mann.
> 
> 
> Bugs: MESOS-10065
> https://issues.apache.org/jira/browse/MESOS-10065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the `update()` method of isolator to handle resource limits.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/isolator.hpp 65893e176d34d72c7d05a064a3de91217ef769c9 
>   src/slave/containerizer/mesos/isolator.hpp 
> a5586799c47edff9b9db4af70bfd43772bfb7da1 
>   src/slave/containerizer/mesos/isolator.cpp 
> ac333984db5ef0a9fed412c19152edfa4183b0e4 
>   src/slave/containerizer/mesos/isolator_tracker.hpp 
> 563db10a5bfb78b5476100ec430ede69bdb02b31 
>   src/slave/containerizer/mesos/isolator_tracker.cpp 
> c78e883fa49069fbc4ca439e612b62adafa081d9 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 4bd3d6dad37dee031660c15e957cc36f63e21fcb 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> b12b73d8e0161d448075378765e77867521de04e 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> e6fe688a247358f30bed0492ebd1baddac8809e8 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> ab192305b53c829fbf6cbf6367ce1686507f9c3d 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> 9db6ddd2d457956f3020a11ba73d96b35b0d8a15 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
> dbf149ad9c8fb900bb3b56f6e90b3a8643097d25 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
> cc3cab51f7dab4393b101aff8b481a5cd0a01af9 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> 5d1821087491cedd0cfbec5d57340bd927aae725 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> c14a8cca2b049104f3529b0c38c0ea6a5ccafe11 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 65b23f00a351d63bdf3fc315191946e9a69adfd6 
>   src/slave/containerizer/mesos/isolators/network/ports.hpp 
> c0d08589d32ffeb2d94c2f3db7added6b0cd36bf 
>   src/slave/containerizer/mesos/isolators/network/ports.cpp 
> fec74d3f47b033a402649658c0cef1ca92c76263 
>   src/slave/containerizer/mesos/isolators/posix.hpp 
> f8de3d2315554be3840911324ac645e21dfc04a0 
>   src/slave/containerizer/mesos/isolators/posix/disk.hpp 
> 2e9b48189d0b6b1a7def66414a58ccef5a1a66e3 
>   src/slave/containerizer/mesos/isolators/posix/disk.cpp 
> 4d8047b1f5966428911d095c734a0155b1f5148a 
>   src/slave/containerizer/mesos/isolators/windows/cpu.hpp 
> ef58d67c119fea6ba492ed19a953cb767289a605 
>   src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
> 0d3706f4a34a912aa49885a2811cdd0b818788fb 
>   src/slave/containerizer/mesos/isolators/windows/mem.hpp 
> 1c87764c4b0dc8caac928f0e592e2853c7cff67d 
>   src/slave/containerizer/mesos/isolators/windows/mem.cpp 
> 4fabb934df0398a44b3a20dcab237730a9218039 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 16bb432d8529f9ced3794fdb08089398659fc09f 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 1680a5923734a6b1a42b8288aa81c5179b206cba 
>   src/tests/containerizer/isolator.hpp 
> 5861b8b8abbba65c499aa57fbf489d0c74414a5b 
>   src/tests/slave_tests.cpp fd4fd6bd1baca0e4521999d2c08e6eab78e57a4f 
> 
> 
> Diff: https://reviews.apache.org/r/71884/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>