mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mesos ReviewBot <revi...@mesos.apache.org>
Subject Re: Review Request 50017: WIP: Validated the resources when parsing it.
Date Mon, 18 Jul 2016 15:34:22 GMT

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



Bad patch!

Reviews applied: [50017]

Failed command: ./support/apply-review.sh -n -r 50017

Error:
2016-07-18 15:34:20 URL:https://reviews.apache.org/r/50017/diff/raw/ [7416/7416] -> "50017.patch"
[1]
Total errors found: 0
Checking 5 files
Error: No line in the commit message summary may exceed 72 characters.

Full log: https://builds.apache.org/job/mesos-reviewbot/14374/console

- Mesos ReviewBot


On July 18, 2016, 2:24 p.m., Guangya Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50017/
> -----------------------------------------------------------
> 
> (Updated July 18, 2016, 2:24 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The "validation" API was called in a huge number of times, but this
> was only needed when parsing resources and we do not need to do
> other validation when doing resources operations, such as add etc.
> 
> This patch is a WIP patch and was now only removing the `validation`
> from `operator+=` and `operator-=` for `Resource`.
> 
> Now there are 4 places where we can constrcut resoures.
> 
> 1) From framework, the resources here will be validated when launching
> task, so it is ok even if the framework developer made some mistake to
> construct some invalid resources.
> 2) From agent flags, we can validate this resource when parsing it.
> 3) From agent resources estimator, we can validate those resources when
> got those resources from estimator in agent. We need to update agent code https://github.com/apache/mesos/blob/master/src/slave/slave.cpp#L5035
> to validate the resources.
> 4) From agent resources hook, we can validate this in hook manager when
> got those resoures from resources hooks. We need to update agent code here https://github.com/apache/mesos/blob/master/src/hook/manager.cpp#L366
> to validate the resources.
> 
> After the above finished, we can simply remove the `validate()` from
> `operator+=` and `operator-=` for `Resource`
> 
> 
> Diffs
> -----
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
>   src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
>   src/tests/resources_tests.cpp 40d290ac540d26373c5fb7c2a93d27d1aa61d722 
>   src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 
> 
> Diff: https://reviews.apache.org/r/50017/diff/
> 
> 
> Testing
> -------
> 
> The `qcachegrind` result here: https://docs.google.com/document/d/1oilen04e8trIOgYbj-jxAd7esTRDaySll8y2BQ6p5nU/edit?usp=sharing
> 
> Sorter test, the sorter add allocation increased 10x.
> 
> With validation:
> ```
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test
> [ RUN      ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35
> Using 50000 agents and 1000 clients
> Added 1000 clients in 19572us
> Added 50000 agents in 915932us
> Added allocations for 50000 agents in 40.303108secs
> Full sort of 1000 clients took 32702us
> No-op sort of 1000 clients took 314us
> [       OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (42449 ms)
> [----------] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (42449 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (42466 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> Without validation:
> ```
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test
> [ RUN      ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35
> Using 50000 agents and 1000 clients
> Added 1000 clients in 22764us
> Added 50000 agents in 862959us
> Added allocations for 50000 agents in 3.934831secs
> Full sort of 1000 clients took 30187us
> No-op sort of 1000 clients took 322us
> [       OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (6031 ms)
> [----------] 1 test from AgentAndClientCount/Sorter_BENCHMARK_Test (6031 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (6049 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> The performance of resources operations with port resources operations.
> 
> With validation
> ```
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN      ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> Took 11.212199secs to perform 1000 'total += r' operations on ports(*):[1-2, 4-5, 7-8,
10-11, 13-14, 16-17, 1...
> Took 8.145903secs to perform 1000 'total -= r' operations on ports(*):[1-2, 4-5, 7-8,
10-11, 13-14, 16-17, 1...
> Took 11.154993secs to perform 1000 'total = total + r' operations on ports(*):[1-2, 4-5,
7-8, 10-11, 13-14, 16-17, 1...
> Took 7.974004secs to perform 1000 'total = total - r' operations on ports(*):[1-2, 4-5,
7-8, 10-11, 13-14, 16-17, 1...
> [       OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (38490 ms)
> [----------] 1 test from ResourcesOperators/Resources_BENCHMARK_Test (38490 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (38512 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> Without validation
> ```
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN      ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> Took 2.90743secs to perform 1000 'total += r' operations on ports(*):[1-2, 4-5, 7-8,
10-11, 13-14, 16-17, 1...
> Took 9166us to perform 1000 'total -= r' operations on ports(*):[1-2, 4-5, 7-8, 10-11,
13-14, 16-17, 1...
> Took 3.038083secs to perform 1000 'total = total + r' operations on ports(*):[1-2, 4-5,
7-8, 10-11, 13-14, 16-17, 1...
> Took 9253us to perform 1000 'total = total - r' operations on ports(*):[1-2, 4-5, 7-8,
10-11, 13-14, 16-17, 1...
> [       OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (5966 ms)
> [----------] 1 test from ResourcesOperators/Resources_BENCHMARK_Test (5966 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (5988 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message