Re: Review Request 69885: Sped up some resource benchmark test instantiations.

[Klaus Ma]

> On Feb. 5, 2019, 12:31 a.m., Klaus Ma wrote:
> > Ship It!

LGTM overall, but I don-t have enough experience right now to review the 
detail, please wait for BenM's comments :)


- Klaus


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


On Feb. 4, 2019, 8:29 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69885/
> ---
> 
> (Updated Feb. 4, 2019, 8:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Bugs: MESOS-8835
> https://issues.apache.org/jira/browse/MESOS-8835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves computation of some resource benchmark test parameters
> from test instantiation time to test execution time. This prevents us
> from having to perform the expensive calculation of test parameters even
> when not executing the benchmark.
> 
> As a result the startup time of the Mesos tests binary is improved,
> while the total wall time required to run these particular benchmarks is
> degraded accordingly.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp f762d17376cc5c29e8556ef5aa2b981e8fe19985 
> 
> 
> Diff: https://reviews.apache.org/r/69885/diff/2/
> 
> 
> Testing
> ---
> 
> Benchmarked `./src/mesos-tests --gtest_list_tests` with clang-9.0.0, 
> lld-2.27. Overall execution time is improved, especially for not optimized 
> builds.
> 
> ```
> Benchmark #1: Before patch, debug
>   Time (mean ± ?):  2.706 s ±  0.018 s[User: 2.472 s, System: 0.168 s]
>   Range (min … max):2.690 s …  2.732 s10 runs
> Benchmark #2: After patch, debug
>   Time (mean ± ?): 683.7 ms ±  18.1 ms[User: 474.2 ms, System: 152.9 
> ms]
>   Range (min … max):   673.4 ms … 734.2 ms10 runs
> ```
> 
> ```
> Benchmark #3: Before patch, optimized
>   Time (mean ± ?): 783.0 ms ±  15.0 ms[User: 537.4 ms, System: 144.9 
> ms]
>   Range (min … max):   772.2 ms … 815.5 ms10 runs
> Benchmark #4: After patch, optimized
>   Time (mean ± ?): 572.5 ms ±   6.7 ms[User: 343.3 ms, System: 138.4 
> ms]
>   Range (min … max):   562.2 ms … 588.7 ms10 runs
> ```
> 
> Remaining time is due to the long list of filters `mesos-tests` uses.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 69885: Sped up some resource benchmark test instantiations.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 4, 2019, 8:29 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69885/
> ---
> 
> (Updated Feb. 4, 2019, 8:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Bugs: MESOS-8835
> https://issues.apache.org/jira/browse/MESOS-8835
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch moves computation of some resource benchmark test parameters
> from test instantiation time to test execution time. This prevents us
> from having to perform the expensive calculation of test parameters even
> when not executing the benchmark.
> 
> As a result the startup time of the Mesos tests binary is improved,
> while the total wall time required to run these particular benchmarks is
> degraded accordingly.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp f762d17376cc5c29e8556ef5aa2b981e8fe19985 
> 
> 
> Diff: https://reviews.apache.org/r/69885/diff/2/
> 
> 
> Testing
> ---
> 
> Benchmarked `./src/mesos-tests --gtest_list_tests` with clang-9.0.0, 
> lld-2.27. Overall execution time is improved, especially for not optimized 
> builds.
> 
> ```
> Benchmark #1: Before patch, debug
>   Time (mean ± ?):  2.706 s ±  0.018 s[User: 2.472 s, System: 0.168 s]
>   Range (min … max):2.690 s …  2.732 s10 runs
> Benchmark #2: After patch, debug
>   Time (mean ± ?): 683.7 ms ±  18.1 ms[User: 474.2 ms, System: 152.9 
> ms]
>   Range (min … max):   673.4 ms … 734.2 ms10 runs
> ```
> 
> ```
> Benchmark #3: Before patch, optimized
>   Time (mean ± ?): 783.0 ms ±  15.0 ms[User: 537.4 ms, System: 144.9 
> ms]
>   Range (min … max):   772.2 ms … 815.5 ms10 runs
> Benchmark #4: After patch, optimized
>   Time (mean ± ?): 572.5 ms ±   6.7 ms[User: 343.3 ms, System: 138.4 
> ms]
>   Range (min … max):   562.2 ms … 588.7 ms10 runs
> ```
> 
> Remaining time is due to the long list of filters `mesos-tests` uses.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 57984: Fixed indents.

[Klaus Ma]

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

Review request for mesos.


Repository: mesos


Description
---

Fixed indents.


Diffs
-

  src/master/master.cpp 43e9d26167c1f405329ea05224c22e7b8c65315f 


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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

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



Discard this PR for now. Will update it after `Flags.load()` refactor.

- Klaus Ma


On July 16, 2016, 9:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 16, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.

[Klaus Ma]

> On Aug. 11, 2016, 6:16 p.m., Michael Park wrote:
> > Hi Klaus, could you explain what the motivation is for this patch?
> > Currently, your analysis seems correct that reserved resources are always 
> > non-revocable.
> > However, the current code seems that it'll be more future-proof.
> > That is, even after reserved resources becomes revocable it would remain 
> > correct.
> > 
> > Anyway, I'm curiuos as to why this patch is being suggested. Thanks!
> 
> Klaus Ma wrote:
> Try to improve the performance by avoid unnecessary operation :).
> 
> Michael Park wrote:
> That would've been my guess. Are there any numbers to support the patch?
> 
> Klaus Ma wrote:
> The number dependent on cases; anyway, I'll append some number for it.
> 
> Guangya Liu wrote:
> I think that this will not impact performance much as we always need two 
> resources operations here: `nonRevocable()` and `+` , the time consumed in 
> those two calls should be same even with this fix.
> 
> Michael Park wrote:
> Hey Klaus, Just curious if you've determined whether or not this actually 
> has notable improvements?
> If so, could you post some numbers? If not, could you discard the review?

@Michael, let's discard this for now. I'm working on other tasks, wil re-open 
it when the numbers is ready.


- Klaus


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


On April 19, 2016, 12:01 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45081/
> ---
> 
> (Updated April 19, 2016, 12:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-4988
> https://issues.apache.org/jira/browse/MESOS-4988
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator will only allocate non-revocable resources to satify quota. As the 
> reserved resources can not be revocable, it's not necessary to call 
> `nonRevocable()` for reserved resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
> 
> Diff: https://reviews.apache.org/r/45081/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 51027: Track allocation candidates to bound allocator.

[Klaus Ma]

> On Aug. 26, 2016, 2:24 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 275
> > <https://reviews.apache.org/r/51027/diff/2/?file=1481820#file1481820line275>
> >
> > I think we should use `insert(...)` instead of `=`; if the following 
> > event in the queue, are we still going to get the correct result?
> > ```
> > batch -> addFramework(f1) -> addFramework(f2)
> > ```
> 
> Jacob Janco wrote:
> insert in a loop?

No; if multiple frameworks register at almost the same time, will there be 
multiple addFramework events pending in the queue? Did not get a chance to test 
it, but did we consider such kind of race condition?


- Klaus


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


On Aug. 31, 2016, 12:53 a.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51027/
> ---
> 
> (Updated Aug. 31, 2016, 12:53 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus 
> Ma, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Triggered allocations dispatch allocate() only
>   if there is no pending allocation in the queue.
> - Allocation candidates are accumulated and only
>   cleared when enqueued allocations are processed.
> - Batched allocations are handled synchronously.
> 
>   
> - Carrying over work from 
>   https://reviews.apache.org/r/41658/ and added 
>   the previous reviewers
> - Specifically, this patch introduces the boolean 
>   flag pendingAllocation, which when set on event 
>   triggered allocations, will prevent additional 
>   no-op allocations: the flag is cleared when the 
>   enqueued allocation is processed, subsequent 
>   event triggered allocations will update a set
>   of allocation candidates rather than dispatching 
>   an additional allocate().
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
> 
> Diff: https://reviews.apache.org/r/51027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note: check without filters depends on https://reviews.apache.org/r/51028
> 
> With new benchmark https://reviews.apache.org/r/49617: 
> Sample output without 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 57251us
> Added 1 agents in 3.2134535333mins
> allocator settled after  1.6123603833mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (290578 ms)
> 
> Sample output with 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 39817us
> Added 1 agents in 3.2286054167mins
> allocator settled after  25.525654secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (220137 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 51027: Track allocation candidates to bound allocator.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.cpp (line 274)
<https://reviews.apache.org/r/51027/#comment213741>

I think we should use `insert(...)` instead of `=`; if the following event 
in the queue, are we still going to get the correct result?
```
batch -> addFramework(f1) -> addFramework(f2)
```


- Klaus Ma


On Aug. 23, 2016, 4:49 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51027/
> ---
> 
> (Updated Aug. 23, 2016, 4:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus 
> Ma, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-3157
> https://issues.apache.org/jira/browse/MESOS-3157
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Triggered allocations dispatch allocate() only
>   if there is no pending allocation in the queue.
> - Allocation candidates are accumulated and only
>   cleared when enqueued allocations are processed.
> - Batched allocations are handled synchronously.
> 
> - Carrying over work from https://reviews.apache.org/r/41658/ and added the 
> previous reviewers
> - Specifically, this patch introduces the boolean flag pendingAllocation, 
> which when set on event 
>   triggered allocations, will prevent additional no-op allocations: the flag 
> is cleared when 
>   the enqueued allocation is processed, subsequent event triggered 
> allocations will update a set
>   of allocation candidates rather than dispatching an additional allocate().
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> bdbc6d3b5b959990538f4e3b7b1a3b031d9aea05 
>   src/master/allocator/mesos/hierarchical.cpp 
> 234ef98529964a0b6d3f132426a6c8ccbb1263ee 
> 
> Diff: https://reviews.apache.org/r/51027/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> note: check without filters depends on https://reviews.apache.org/r/51028
> 
> With new benchmark https://reviews.apache.org/r/49617: 
> Sample output without 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 57251us
> Added 1 agents in 3.2134535333mins
> allocator settled after  1.6123603833mins
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (290578 ms)
> 
> Sample output with 51027:
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
> Using 1 agents and 3000 frameworks
> Added 3000 frameworks in 39817us
> Added 1 agents in 3.2286054167mins
> allocator settled after  25.525654secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.FrameworkFailover/22
>  (220137 ms)
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.

[Klaus Ma]

> On Aug. 11, 2016, 6:16 p.m., Michael Park wrote:
> > Hi Klaus, could you explain what the motivation is for this patch?
> > Currently, your analysis seems correct that reserved resources are always 
> > non-revocable.
> > However, the current code seems that it'll be more future-proof.
> > That is, even after reserved resources becomes revocable it would remain 
> > correct.
> > 
> > Anyway, I'm curiuos as to why this patch is being suggested. Thanks!
> 
> Klaus Ma wrote:
> Try to improve the performance by avoid unnecessary operation :).
> 
> Michael Park wrote:
> That would've been my guess. Are there any numbers to support the patch?

The number dependent on cases; anyway, I'll append some number for it.


- Klaus


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


On April 19, 2016, 12:01 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45081/
> ---
> 
> (Updated April 19, 2016, 12:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-4988
> https://issues.apache.org/jira/browse/MESOS-4988
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator will only allocate non-revocable resources to satify quota. As the 
> reserved resources can not be revocable, it's not necessary to call 
> `nonRevocable()` for reserved resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
> 
> Diff: https://reviews.apache.org/r/45081/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 45081: Excluded reserved resources when got nonRevocable resources in stage 1.

[Klaus Ma]

> On Aug. 11, 2016, 6:16 p.m., Michael Park wrote:
> > Hi Klaus, could you explain what the motivation is for this patch?
> > Currently, your analysis seems correct that reserved resources are always 
> > non-revocable.
> > However, the current code seems that it'll be more future-proof.
> > That is, even after reserved resources becomes revocable it would remain 
> > correct.
> > 
> > Anyway, I'm curiuos as to why this patch is being suggested. Thanks!

Try to improve the performance by avoid unnecessary operation :).


- Klaus


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


On April 19, 2016, 12:01 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45081/
> ---
> 
> (Updated April 19, 2016, 12:01 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and 
> Michael Park.
> 
> 
> Bugs: MESOS-4988
> https://issues.apache.org/jira/browse/MESOS-4988
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocator will only allocate non-revocable resources to satify quota. As the 
> reserved resources can not be revocable, it's not necessary to call 
> `nonRevocable()` for reserved resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
> 
> Diff: https://reviews.apache.org/r/45081/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

[Klaus Ma]

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

(Updated July 22, 2016, 12:34 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Handle empty Range & Set case.


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


Repository: mesos


Description (updated)
---

Fixed Value parsing code to only accept the canonical formats.
NOTES: This patch isn't doing a complete validation against
the canonical format yet: the set entries is not validated as
conforming to the Values.Text format yet.


Diffs (updated)
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba 
  src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 50290: Did not put latest empty string in strings::split.

[Klaus Ma]

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

(Updated July 21, 2016, 11:44 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Remove depdencies.


Repository: mesos


Description
---

Did not put latest empty string in strings::split.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
7f7f1cffcebfe16cb986917b1d90c1ae4a480989 

Diff: https://reviews.apache.org/r/50290/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

[Klaus Ma]

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

(Updated July 21, 2016, 11:44 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update depdencies.


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


Repository: mesos


Description
---

Fixed Value parsing code to only accept the canonical formats.
NOTES: This patch isn't doing a complete validation against 
the canonical format yet: the set entries is not validated as
conforming to the Values.Text format yet.


Diffs
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba 
  src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

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

(Updated July 21, 2016, 11:43 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
Remoortere.


Changes
---

Update depdencies.


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


Repository: mesos


Description
---

Improve Ranges parsing to handle single values.


Diffs (updated)
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba 
  src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 

Diff: https://reviews.apache.org/r/43561/diff/


Testing
---

make
make check GTEST_FILTER=~"*"
./src/mesos-test


Thanks,

Klaus Ma

Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

[Klaus Ma]

> On July 21, 2016, 7:44 a.m., Benjamin Mahler wrote:
> > src/common/values.cpp, lines 589-594
> > <https://reviews.apache.org/r/49223/diff/6/?file=1445258#file1445258line589>
> >
> > Do we still need this? Seems to me that we want to do the following:
> > 
> > ```
> > // Ranges. E.g. [1-2,4-5]
> > if (strings::startsWith(temp, "[")) {
> >   if (!strings::endsWith(temp, "]")) {
> > return Error("Missing a closing bracket");
> >   }
> > 
> >   ...
> > }
> > 
> > if (strings::startsWith(temp, "{")) {
> >   if (!strings::endsWith(temp, "}")) {
> > return Error("Missing a closing brace");
> >   }
> >   
> >   ..
> > }
> > ```
> > 
> > Also, why is this looking at parentheses? I only see brackets (ranges) 
> > and braces (set) being used.

I think this is not necessary. Removed.


> On July 21, 2016, 7:44 a.m., Benjamin Mahler wrote:
> > src/common/values.cpp, line 611
> > <https://reviews.apache.org/r/49223/diff/6/?file=1445258#file1445258line611>
> >
> > Looks like you want a strings::split here? strings::tokenize will 
> > ignore empty fields:
> > 
> > ```
> > // Using tokenize makes these two equivalent.
> > [1-25-6]
> > [1-2,5-6]
> > ```
> > 
> > Can you add a test that we don't allow the extra commas since it's not 
> > the canonical format?
> > 
> > Also, why did you keep tokenizing on newlines? AFAICT they are not part 
> > of the canonical format?

Replaced tokenize with split, also add related test cases.


- Klaus


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


On July 21, 2016, 10:38 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 21, 2016, 10:38 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed Value parsing code to only accept the canonical formats.
> NOTES: This patch isn't doing a complete validation against 
> the canonical format yet: the set entries is not validated as
> conforming to the Values.Text format yet.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
>   src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba 
>   src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

[Klaus Ma]

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

(Updated July 21, 2016, 10:38 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address Ben's comments.


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


Repository: mesos


Description (updated)
---

Fixed Value parsing code to only accept the canonical formats.
NOTES: This patch isn't doing a complete validation against 
the canonical format yet: the set entries is not validated as
conforming to the Values.Text format yet.


Diffs (updated)
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba 
  src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 50295: Returned empty vector if string is empty.

[Klaus Ma]

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

Review request for mesos.


Repository: mesos


Description
---

Returned empty vector if string is empty.


Diffs
-


Diff: https://reviews.apache.org/r/50295/diff/


Testing
---


Thanks,

Klaus Ma

Re: Review Request 50290: Did not put latest empty string in strings::split.

[Klaus Ma]

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

(Updated July 21, 2016, 10:25 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update


Summary (updated)
-

Did not put latest empty string in strings::split.


Repository: mesos


Description (updated)
---

Did not put latest empty string in strings::split.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
7f7f1cffcebfe16cb986917b1d90c1ae4a480989 

Diff: https://reviews.apache.org/r/50290/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 50290: Returned empty vector if string is empty.

[Klaus Ma]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Returned empty vector if string is empty.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
7f7f1cffcebfe16cb986917b1d90c1ae4a480989 

Diff: https://reviews.apache.org/r/50290/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 50285: Replaced boost::lexical_cast with numify when paring Value.

[Klaus Ma]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Replaced boost::lexical_cast with numify when paring Value.


Diffs
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 

Diff: https://reviews.apache.org/r/50285/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 50280: Simplificed removing whitspace by strings::replace().

[Klaus Ma]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Simplificed removing whitspace by strings::replace().


Diffs
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/v1/values.cpp ff1926f9ab26c3ac3ab62b6df3ce305f5d12f12e 

Diff: https://reviews.apache.org/r/50280/diff/


Testing
---

make && make test


Thanks,

Klaus Ma

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

[Klaus Ma]

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




include/mesos/resources.hpp (line 84)
<https://reviews.apache.org/r/45959/#comment208553>

I think this's dangous, the developer need to check this converting and 
decide which `operator` is used. If miss used, it's hard to debuging.



include/mesos/resources.hpp (line 124)
<https://reviews.apache.org/r/45959/#comment208558>

Seems `Option` is not necessary.



include/mesos/resources.hpp (line 479)
<https://reviews.apache.org/r/45959/#comment208554>

Why not `list`? It may avoid un-necessary resizing.



src/common/resources.cpp (lines 308 - 316)
<https://reviews.apache.org/r/45959/#comment208555>

I think we can check name,type, role firstly, then check SharedInfo.



src/common/resources.cpp (line 882)
<https://reviews.apache.org/r/45959/#comment208557>

one `isShared()` is enough. `isShared` is checked in `subtractable`.


- Klaus Ma


On July 20, 2016, 6:51 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45959/
> ---
> 
> (Updated July 20, 2016, 6:51 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Jiang 
> Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A new class Resource_ is added that allows 'Resources' to group
> identical shared resource objects together into a single 'Resource_'
> object and tracked by its shared count. Non-shared resource objects
> are not grouped.
> 
> For resource addition and subtraction, the shared count is adjusted for
> shared resources as follows:
> a) Addition: If shared resource is absent from original, then the
>resource is added initialized with a consumer count of 1. Otherwise,
>the share count for the shared resource is incremented.
> b) Subtraction: If shared resource's share count is already 1, then
>the shared resource is removed from the original. Otherwise, its
>consumer count is decremented.
> 
> Note that v1 changes for shared resources are in the next commit.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   src/common/resources.cpp b1bd2784aefdebf91892638b40b215373b998574 
>   src/master/validation.cpp 50ba372d1301d16d1738a3e4f4882b51f9ce06cd 
>   src/tests/mesos.hpp e4eccfc3810bed3649a3ab80e252849470de4c72 
>   src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 
> 
> Diff: https://reviews.apache.org/r/45959/diff/
> 
> 
> Testing
> ---
> 
> New tests added to demonstrate arithmetic operations for shared resources 
> with consumer counts.
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

> On July 12, 2016, 4:23 a.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > <https://reviews.apache.org/r/46298/diff/2/?file=1349891#file1349891line175>
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
>     Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?
> 
> Klaus Ma wrote:
> Found a `add` function to add required field; this function need an 
> additional "optional alias" to avoid conflict.
> 
> Jie Yu wrote:
> we can introduce another overload for `add`, similar to this one:
> 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L182-L194
> 
> But, it accepts a validate function:
> ```
> template 
>   void add(
>   T1* t1,
>   const Name& name,
>   const std::string& help,
>   F validate)
>   {
> add(t1,
> name,
> None(),
> help,
> static_cast(nullptr),
> validate);
>   }
> ```
> 
> Klaus Ma wrote:
> @Jie, it will conflict with this one: 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L172-L180
>  . There's not enough information to distinguish `F validate` vs. `const T2& 
> t2`.
> 
> Jie Yu wrote:
> Aha, ic, now I get what you said about 'Option'. Yeah, can we make t2 an 
> optional field here:
> 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L173

@Jie, try `Option` today, but c++ template can not distinguish `Option<...>` 
and validated lamba fuctions. Should we commit firstly or make it dependent on 
MESOS-5841.


- Klaus


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


On July 16, 2016, 9:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 16, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

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

(Updated July 16, 2016, 9:33 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


Changes
---

Rebase


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs (updated)
-

  src/slave/flags.cpp 166a6516362a23bc5012aaa2dd45edfc6446de48 

Diff: https://reviews.apache.org/r/46298/diff/


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma

Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

[Klaus Ma]

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

(Updated July 16, 2016, 10:39 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update v1/value.cpp & add more cases


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


Repository: mesos


Description (updated)
---

Fixed Value parsing code to only accept the canonical formats.


Diffs (updated)
-

  src/common/values.cpp 7b8a8ea00bd4bd1a82c33be4438c26b2731b1dd8 
  src/tests/values_tests.cpp cc17b675d5d3768685b44a1cea64264dcbca80ba 
  src/v1/values.cpp 3e0f739b69e680f9eb29ed36d4501c393758c861 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Klaus Ma]

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




src/master/allocator/sorter/drf/sorter.hpp (line 22)
<https://reviews.apache.org/r/50044/#comment207883>

This's not necessary, `sorter/sorter.hpp` had included it.


- Klaus Ma


On July 15, 2016, 3:35 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50044/
> ---
> 
> (Updated July 15, 2016, 3:35 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now prefer to use vector rather than list in general for
> efficiency reasons, unless we need to take advantage of the
> operations that are efficient on a linked-list.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> e29feebd70277c79f7c3f6fb233e7a36501cf220 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 7df4dd641b21ea0705368861bf4679fed1ef078d 
>   src/master/allocator/sorter/sorter.hpp 
> f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
>   src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 
> 
> Diff: https://reviews.apache.org/r/50044/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> benchmarks: with 1000 clients, sort time is reduced by 72 us
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

> On July 12, 2016, 4:23 a.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > <https://reviews.apache.org/r/46298/diff/2/?file=1349891#file1349891line175>
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
>     Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?
> 
> Klaus Ma wrote:
> Found a `add` function to add required field; this function need an 
> additional "optional alias" to avoid conflict.
> 
> Jie Yu wrote:
> we can introduce another overload for `add`, similar to this one:
> 
> https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L182-L194
> 
> But, it accepts a validate function:
> ```
> template 
>   void add(
>   T1* t1,
>   const Name& name,
>   const std::string& help,
>   F validate)
>   {
> add(t1,
> name,
> None(),
> help,
> static_cast(nullptr),
> validate);
>   }
> ```

@Jie, it will conflict with this one: 
https://github.com/apache/mesos/blob/master/3rdparty/stout/include/stout/flags/flags.hpp#L172-L180
 . There's not enough information to distinguish `F validate` vs. `const T2& 
t2`.


- Klaus


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


On July 13, 2016, 1:45 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 13, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49381: Benchmark for Resources class.

[Klaus Ma]

rator_SubAndAssign/1 (6 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/2
Took 399855us to `SubAndAssign` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 
times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/2 (400 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3
Took 834715us to `SubAndAssign` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3 (835 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4
**Took 2.096096secs to `SubAndAssign` "[1-2, 4-5, ... , 298-299]" 1 times.**
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4 (2096 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0
Took 2750us to `cpus()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1
Took 3223us to `cpus()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2
Took 102915us to `cpus()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2 (103 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3
Took 131005us to `cpus()` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3 (131 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4
Took 1408us to `cpus()` "[1-2, 4-5, ... , 298-299]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4 (2 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0
Took 1453us to `mem()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0 (1 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1
Took 2968us to `mem()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2
Took 28500us to `mem()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2 (29 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3
Took 134330us to `mem()` "cpus(r0):1;mem(r0):100; ... cpus(r99):1;mem(r99):100" 
1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3 (135 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4
Took 1753us to `mem()` "[1-2, 4-5, ... , 298-299]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4 (1 ms)
[--] 30 tests from ResourcesOperators/Resources_BENCHMARK_Test (52070 
ms total)


Thanks,

Klaus Ma

Re: Review Request 49381: Benchmark for Resources class.

[Klaus Ma]

 6091us to `SubAndAssign` "cpus:1;mem:2" 1 times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/1 (6 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/2
Took 399855us to `SubAndAssign` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 
times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/2 (400 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3
Took 834715us to `SubAndAssign` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3 (835 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4
**Took 2.096096secs to `SubAndAssign` "[1-2, 4-5, ... , 298-299]" 1 times.**
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4 (2096 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0
Took 2750us to `cpus()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1
Took 3223us to `cpus()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2
Took 102915us to `cpus()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2 (103 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3
Took 131005us to `cpus()` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3 (131 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4
Took 1408us to `cpus()` "[1-2, 4-5, ... , 298-299]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4 (2 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0
Took 1453us to `mem()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0 (1 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1
Took 2968us to `mem()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2
Took 28500us to `mem()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2 (29 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3
Took 134330us to `mem()` "cpus(r0):1;mem(r0):100; ... cpus(r99):1;mem(r99):100" 
1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3 (135 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4
Took 1753us to `mem()` "[1-2, 4-5, ... , 298-299]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4 (1 ms)
[--] 30 tests from ResourcesOperators/Resources_BENCHMARK_Test (52070 
ms total)


Thanks,

Klaus Ma

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

> On July 12, 2016, 4:23 a.m., Jie Yu wrote:
> > src/slave/main.cpp, lines 175-179
> > <https://reviews.apache.org/r/46298/diff/2/?file=1349891#file1349891line175>
> >
> > Can we do that check in `add` function. `add` function supports an 
> > optional validate lambda to be passed in.
> 
> Klaus Ma wrote:
> @jie, thanks for the comments. Try to use validated lamba today, but 
> there some issues:
> 
> 1. The `work_dir` is required without default value; if using validate 
> lamba, a "default" value is necessary: `add(T1* t1, name, alias, 
> defaultValue, lamba)`
> 2. The default value is pass by const reference currently, we can not 
> pass some empty value to it, e.g. nullptr or 0
> 
> For the default value, I'm thinking to replace const reference with 
> `Option`; any suggestion?
> 
> Jie Yu wrote:
> Oh, I don't realize that we don't have validation support for required 
> field without default value.
> 
> Can we introduce another `add` variant in flags that support the above 
> case? Is that difficult?

Found a `add` function to add required field; this function need an additional 
"optional alias" to avoid conflict.


- Klaus


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


On July 13, 2016, 1:45 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated July 13, 2016, 1:45 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

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

(Updated July 13, 2016, 1:45 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


Changes
---

Using validate lamba to reject relative path.


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs (updated)
-

  src/slave/flags.cpp 84dbb2db41bd9849ab1245969884675d9d16555b 

Diff: https://reviews.apache.org/r/46298/diff/


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma

Re: Review Request 49381: Benchmark for Resources class (cpu, mem & port).

[Klaus Ma]

es_BENCHMARK_Test.Operator_SubAndAssign/1 (6 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/2
Took 399855us to `SubAndAssign` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 
times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/2 (400 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3
Took 834715us to `SubAndAssign` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3 (835 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4
**Took 2.096096secs to `SubAndAssign` "[1-2, 4-5, ... , 298-299]" 1 times.**
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4 (2096 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0
Took 2750us to `cpus()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1
Took 3223us to `cpus()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2
Took 102915us to `cpus()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2 (103 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3
Took 131005us to `cpus()` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3 (131 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4
Took 1408us to `cpus()` "[1-2, 4-5, ... , 298-299]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4 (2 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0
Took 1453us to `mem()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0 (1 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1
Took 2968us to `mem()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2
Took 28500us to `mem()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2 (29 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3
Took 134330us to `mem()` "cpus(r0):1;mem(r0):100; ... cpus(r99):1;mem(r99):100" 
1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3 (135 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4
Took 1753us to `mem()` "[1-2, 4-5, ... , 298-299]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4 (1 ms)
[--] 30 tests from ResourcesOperators/Resources_BENCHMARK_Test (52070 
ms total)


Thanks,

Klaus Ma

Re: Review Request 49381: Benchmark for Resources class (cpu, mem & port)

[Klaus Ma]

Took 400747us to `SubAndAssign` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 
times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/2 (401 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3
Took 796633us to `SubAndAssign` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/3 (797 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4
Took 3709us to `SubAndAssign` "[1-2, 3-4, ... , 199-200]" 1 times.
[   OK ] 
ResourcesOperators/Resources_BENCHMARK_Test.Operator_SubAndAssign/4 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0
Took 2764us to `cpus()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/0 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1
Took 3137us to `cpus()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2
Took 107398us to `cpus()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/2 (108 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3
Took 131399us to `cpus()` "cpus(r0):1;mem(r0):100; ... 
cpus(r99):1;mem(r99):100" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/3 (132 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4
Took 1624us to `cpus()` "[1-2, 3-4, ... , 199-200]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_CPUS/4 (1 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0
Took 1577us to `mem()` "cpus:1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/0 (2 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1
Took 3019us to `mem()` "cpus:1;mem:2" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/1 (3 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2
Took 24423us to `mem()` "cpus(r0):1;cpus(r1):1; ... cpus(r99):1" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/2 (25 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3
Took 129675us to `mem()` "cpus(r0):1;mem(r0):100; ... cpus(r99):1;mem(r99):100" 
1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/3 (129 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4
Took 1405us to `mem()` "[1-2, 3-4, ... , 199-200]" 1 times.
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Operator_MEM/4 (2 ms)
[--] 30 tests from ResourcesOperators/Resources_BENCHMARK_Test (44301 
ms total)


Thanks,

Klaus Ma

Re: Review Request 49381: Benchmark for Resources class (cpu, mem & port)

[Klaus Ma]

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

(Updated July 12, 2016, 12:08 p.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, and Michael Park.


Changes
---

Update code diff.


Summary (updated)
-

Benchmark for Resources class (cpu, mem & port)


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


Repository: mesos


Description (updated)
---

Benchmark for Resources class (cpu, mem & port)


Diffs (updated)
-

  src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e 

Diff: https://reviews.apache.org/r/49381/diff/


Testing
---


Thanks,

Klaus Ma

Re: Review Request 49381: WIP: Benchmark for Resources class.

[Klaus Ma]

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

(Updated July 10, 2016, 9:04 a.m.)


Review request for mesos, Benjamin Mahler and Michael Park.


Changes
---

Fix build error.


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


Repository: mesos


Description
---

WIP: Benchmark for Resources class.


Diffs (updated)
-

  src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e 

Diff: https://reviews.apache.org/r/49381/diff/


Testing
---


Thanks,

Klaus Ma

Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

[Klaus Ma]

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

(Updated July 9, 2016, 8:35 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update the length of description.


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


Repository: mesos


Description (updated)
---

Fixed Value parsing code to only accept the canonical formats defined
in: http://mesos.apache.org/documentation/latest/attributes-resources/


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Fixed Value parsing code to only accept the canonical formats.

[Klaus Ma]

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

(Updated July 9, 2016, 3:54 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated summary based on BenM's comments.


Summary (updated)
-

Fixed Value parsing code to only accept the canonical formats.


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


Repository: mesos


Description (updated)
---

Fixed Value parsing code to only accept the canonical formats defined in: 
http://mesos.apache.org/documentation/latest/attributes-resources/


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 49707: Replaced raw string with const.

[Klaus Ma]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Replaced raw string with const.


Diffs
-

  src/slave/containerizer/fetcher.cpp 8545d9813058e3f7af93ebadccec64a355333b72 

Diff: https://reviews.apache.org/r/49707/diff/


Testing
---

make


Thanks,

Klaus Ma

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

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

(Updated July 6, 2016, 10:40 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address comments.


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


Repository: mesos


Description
---

Enhance value parsing.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

> On July 4, 2016, 7:55 p.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > <https://reviews.apache.org/r/49223/diff/4/?file=1436140#file1436140line673>
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.
> 
> Klaus Ma wrote:
> Only [0-9a-z./] is available for Text in document.
> 
> Guangya Liu wrote:
> OK, then what about "Check `text resource` in the format of 
> `[a-zA-Z0-9_/.-]`.", ditto for above.

1, Text is not resource; 2. I'd like to depdent on document; otherwise, we need 
to align TODO and doc.


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

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

(Updated July 4, 2016, 9:57 p.m.)


Review request for mesos and Michael Park.


Changes
---

Update format.


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


Repository: mesos


Description
---

Enhanced startsWith/endsWith's performance.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/49246/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

> On July 4, 2016, 7:55 p.m., Guangya Liu wrote:
> > src/common/values.cpp, line 673
> > <https://reviews.apache.org/r/49223/diff/4/?file=1436140#file1436140line673>
> >
> > Sorry, I should ask this question in previous patch. Same as above, can 
> > you please show more comments for what do you want to check here? It would 
> > be great if you can make the comment easy to understand.

Only [0-9a-z./] is available for Text in document.


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49586: Used startsWith char version for role validation.

[Klaus Ma]

> On July 4, 2016, 6:51 p.m., Michael Park wrote:
> > This is covered in https://reviews.apache.org/r/49582/

Yes, you're right :). Discard this patch.


- Klaus


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


On July 4, 2016, 4:05 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49586/
> ---
> 
> (Updated July 4, 2016, 4:05 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used startsWith char version for role validation.
> 
> 
> Diffs
> -
> 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
> 
> Diff: https://reviews.apache.org/r/49586/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

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




src/common/values.cpp (line 665)
<https://reviews.apache.org/r/49223/#comment206026>

Only `[0-9a-z./]` is available for Text in document.


- Klaus Ma


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

> On July 1, 2016, 1:27 p.m., Guangya Liu wrote:
> > src/tests/values_tests.cpp, line 204
> > <https://reviews.apache.org/r/49223/diff/3/?file=1433898#file1433898line204>
> >
> > Would it make sense to add some negative case here to test against the 
> > code for error handling?

Will handle in https://issues.apache.org/jira/browse/MESOS-5603 .


- Klaus


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


On July 4, 2016, 6:33 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49223/
> ---
> 
> (Updated July 4, 2016, 6:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5739
> https://issues.apache.org/jira/browse/MESOS-5739
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhance value parsing.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/49223/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

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

(Updated July 4, 2016, 6:33 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address Guangya's comments


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


Repository: mesos


Description (updated)
---

Enhance value parsing.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 49586: Used startsWith char version for role validation.

[Klaus Ma]

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

Review request for mesos and Michael Park.


Repository: mesos


Description
---

Used startsWith char version for role validation.


Diffs
-

  src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 

Diff: https://reviews.apache.org/r/49586/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49581: Used the `char` version of `strings::startsWith` in stout.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On July 4, 2016, 8:38 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49581/
> ---
> 
> (Updated July 4, 2016, 8:38 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-5692
> https://issues.apache.org/jira/browse/MESOS-5692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/path.hpp 
> 3d06ca29ce9d4d133c3ec014aa99a51aa5289bc9 
> 
> Diff: https://reviews.apache.org/r/49581/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

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

(Updated July 4, 2016, 2:49 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address mcypark's comments.


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


Repository: mesos


Description
---

Enhanced startsWith/endsWith's performance.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/49246/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49582: Used the `char` version of `strings::startsWith` in mesos.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On July 4, 2016, 8:39 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49582/
> ---
> 
> (Updated July 4, 2016, 8:39 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-5692
> https://issues.apache.org/jira/browse/MESOS-5692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/authentication/cram_md5/auxprop.cpp 
> 5912b01a80d30b5ad79d71b4a5041635ab8e1b84 
>   src/common/roles.cpp 65c71931a5645d346439c3cdff1c5c8cc3ee01a3 
>   src/slave/containerizer/fetcher.cpp 
> 15ff61ff54d72c951edbd591058ad04f8d1efb58 
> 
> Diff: https://reviews.apache.org/r/49582/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 49381: WIP: Benchmark for Resources class.

[Klaus Ma]

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

(Updated June 30, 2016, 9:54 p.m.)


Review request for mesos, Benjamin Mahler and Michael Park.


Changes
---

Add more benchmark cases.


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


Repository: mesos


Description
---

WIP: Benchmark for Resources class.


Diffs (updated)
-

  src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e 

Diff: https://reviews.apache.org/r/49381/diff/


Testing
---


Thanks,

Klaus Ma

Re: Review Request 48895: Added allocator_fairness_excluded_resource_names flag to master.

[Klaus Ma]

> On June 30, 2016, 1:01 a.m., Klaus Ma wrote:
> > src/master/flags.cpp, line 437
> > <https://reviews.apache.org/r/48895/diff/3/?file=1432431#file1432431line437>
> >
> > s/client/framework
> > 
> > Mesos user does not know client.
> 
> Guangya Liu wrote:
> I think that here not only framework, but also roles, I will udpate it to 
> 
> "A comma-separated list of the resource names that will be ignored\n"
> "by WDRF sorter when calculating share for clients (frameworks or 
> roles).\n");

Only the code reader known what's `clients` means in sorter, we did not need to 
expose it to end user; using `framework/roles` directly is better.


- Klaus


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


On June 30, 2016, 3:51 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48895/
> ---
> 
> (Updated June 30, 2016, 3:51 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5620
> https://issues.apache.org/jira/browse/MESOS-5620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator_fairness_excluded_resource_names flag to master.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp 735367ff5412d611f4eae6cb63dd4829c4338002 
>   src/master/flags.cpp b6b1603f02d3c6f861edad3de770ecb3fcad0057 
> 
> Diff: https://reviews.apache.org/r/48895/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49223: Enhance value parsing.

[Klaus Ma]

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

(Updated June 30, 2016, 1:12 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Address benm's comments; remove checking against Text.


Summary (updated)
-

Enhance value parsing.


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


Repository: mesos


Description (updated)
---

Enhance value parsing.

1. Did not support [1-2, [3-4]] as Ranges; it should be [1-2, 3-4].
2. Did not support {a{b, c}d} as Set; it should be {ab, cd}
3. Add TODO for checking Text against [a-zA-Z0-9_/.-]


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 48904: Updated allocator initialize() to include fairnessExcludeResourceNames.

[Klaus Ma]

> On June 30, 2016, 1:13 a.m., Klaus Ma wrote:
> > src/master/master.cpp, line 830
> > <https://reviews.apache.org/r/48904/diff/3/?file=1432471#file1432471line830>
> >
> > Please check whether the resource name is valid. We only support cpus, 
> > mem, disk, ports, gpus for now.
> 
> Guangya Liu wrote:
> One question is that end user can specify customized resources when agent 
> start up by adding some other resources, such as ` 
> --resources="cpus:8;mem:8000;foo:10;bar:20"`, so here I'm not validating the 
> resources, but just make the sorter ignore the resource names in the 
> `allocator_fairness_excluded_resource_names`, what do you say?

it'll be hard for user to check typo: `cpu` vs. `cpus`; WARN message at least.


- Klaus


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


On June 29, 2016, 8:16 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48904/
> ---
> 
> (Updated June 29, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5620
> https://issues.apache.org/jira/browse/MESOS-5620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated allocator initialize() to include fairnessExcludeResourceNames.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 98025bc182d92098bd4018177a6ae28135d8de95 
>   src/master/allocator/mesos/allocator.hpp 
> f096d2b7813580fb28e77a2803e290edf1ebda31 
>   src/master/allocator/mesos/hierarchical.hpp 
> 98a1f69f14b967c8d01f8a680771c9d28fac14e4 
>   src/master/allocator/mesos/hierarchical.cpp 
> c3639342335499a04a23147a4205f1b475c123fa 
>   src/master/master.cpp 907233b015919f437fb2ebd25875217930b301b4 
>   src/tests/allocator.hpp 41a31ae5d2c8fb8eb902a82d893be570db0da3bd 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
>   src/tests/master_allocator_tests.cpp 
> 7910f5532bf36ed92100839eac6c6f6a18838ffa 
>   src/tests/master_quota_tests.cpp b9bc49b1ef55d6ea57d94971905d70244f982e9f 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 6c85e19eeaa69bf3a4e3077261331191db6eec06 
>   src/tests/reservation_endpoints_tests.cpp 
> 3ee59d5db0089dd59acfe48a77910d069ffc377b 
>   src/tests/reservation_tests.cpp d7e90bc67a55a909be70691a1108493c33743b02 
>   src/tests/resource_offers_tests.cpp 
> 046adaedf9121655f377f503bb30437803bf0005 
>   src/tests/slave_recovery_tests.cpp e6e7b8e3d71886eb8749122bd7b441857983d574 
> 
> Diff: https://reviews.apache.org/r/48904/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49352: Added a flag parser for hashset.

[Klaus Ma]

> On June 30, 2016, 12:59 a.m., Klaus Ma wrote:
> > src/common/parse.hpp, lines 150-152
> > <https://reviews.apache.org/r/49352/diff/1/?file=1432428#file1432428line150>
> >
> > Call `insert` directly should be fine. `set` will help to avoid 
> > duplicated item.
> 
> Guangya Liu wrote:
> Here I need to return error if there are diplicate items, seems I cannot 
> use `set` for this as `set` do not have a method like `contain` which can 
> enable me to check the duplicate items, comments?

Return value of set: The single element versions (1) return a pair, with its 
member pair::first set to an iterator pointing to either the newly inserted 
element or to the equivalent element already in the set. **The pair::second 
element in the pair is set to true if a new element was inserted or false if an 
equivalent element already existed.**

http://www.cplusplus.com/reference/set/set/insert/


- Klaus


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


On June 29, 2016, 8:12 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49352/
> ---
> 
> (Updated June 29, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5743
> https://issues.apache.org/jira/browse/MESOS-5743
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a flag parser for hashset.
> 
> 
> Diffs
> -
> 
>   src/common/parse.hpp 19e56dbeb765f8bec92e0a3615f6f7c12466fa9e 
> 
> Diff: https://reviews.apache.org/r/49352/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48904: Updated allocator initialize() to include fairnessExcludeResourceNames.

[Klaus Ma]

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




src/master/master.cpp (line 830)
<https://reviews.apache.org/r/48904/#comment205346>

Please check whether the resource name is valid. We only support cpus, mem, 
disk, ports, gpus for now.


- Klaus Ma


On June 29, 2016, 8:16 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48904/
> ---
> 
> (Updated June 29, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5620
> https://issues.apache.org/jira/browse/MESOS-5620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated allocator initialize() to include fairnessExcludeResourceNames.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 98025bc182d92098bd4018177a6ae28135d8de95 
>   src/master/allocator/mesos/allocator.hpp 
> f096d2b7813580fb28e77a2803e290edf1ebda31 
>   src/master/allocator/mesos/hierarchical.hpp 
> 98a1f69f14b967c8d01f8a680771c9d28fac14e4 
>   src/master/allocator/mesos/hierarchical.cpp 
> c3639342335499a04a23147a4205f1b475c123fa 
>   src/master/master.cpp 907233b015919f437fb2ebd25875217930b301b4 
>   src/tests/allocator.hpp 41a31ae5d2c8fb8eb902a82d893be570db0da3bd 
>   src/tests/api_tests.cpp b3127b716480223a6f23b7908bf6bc1808120f80 
>   src/tests/master_allocator_tests.cpp 
> 7910f5532bf36ed92100839eac6c6f6a18838ffa 
>   src/tests/master_quota_tests.cpp b9bc49b1ef55d6ea57d94971905d70244f982e9f 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 6c85e19eeaa69bf3a4e3077261331191db6eec06 
>   src/tests/reservation_endpoints_tests.cpp 
> 3ee59d5db0089dd59acfe48a77910d069ffc377b 
>   src/tests/reservation_tests.cpp d7e90bc67a55a909be70691a1108493c33743b02 
>   src/tests/resource_offers_tests.cpp 
> 046adaedf9121655f377f503bb30437803bf0005 
>   src/tests/slave_recovery_tests.cpp e6e7b8e3d71886eb8749122bd7b441857983d574 
> 
> Diff: https://reviews.apache.org/r/48904/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48895: Added allocator_fairness_excluded_resource_names flag to master.

[Klaus Ma]

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




src/master/flags.cpp (line 437)
<https://reviews.apache.org/r/48895/#comment205344>

s/client/framework

Mesos user does not know client.


- Klaus Ma


On June 29, 2016, 8:14 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48895/
> ---
> 
> (Updated June 29, 2016, 8:14 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5620
> https://issues.apache.org/jira/browse/MESOS-5620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator_fairness_excluded_resource_names flag to master.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp 735367ff5412d611f4eae6cb63dd4829c4338002 
>   src/master/flags.cpp b6b1603f02d3c6f861edad3de770ecb3fcad0057 
> 
> Diff: https://reviews.apache.org/r/48895/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 48581: Removed duplicated code in 'strings::tokenize()'.

[Klaus Ma]

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

(Updated June 30, 2016, 12:57 a.m.)


Review request for mesos, Benjamin Mahler and Joris Van Remoortere.


Changes
---

Add Joris as reviewers


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


Repository: mesos


Description
---

Removed duplicated code in 'strings::tokenize()'.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/48581/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 49381: WIP: Benchmark for Resources class.

[Klaus Ma]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

WIP: Benchmark for Resources class.


Diffs
-

  src/tests/resources_tests.cpp dc12bd8f1e2da6972bc8aed598811c55d664036e 

Diff: https://reviews.apache.org/r/49381/diff/


Testing
---


Thanks,

Klaus Ma

Re: Review Request 49223: Enhanced Value parsing.

[Klaus Ma]

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

(Updated June 29, 2016, 7:58 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update JIRA #.


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


Repository: mesos


Description
---

Enhanced Value parsing:

1. Did not support `[1-2, [3-4]]` as Ranges; it should be `[1-2, 3-4]`.
2. Did not support `{a{b, c}d}` as Set; it should be `{ab, cd}`
3. Add check for Text against `[a-zA-Z0-9_/.-]`


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/attributes_tests.cpp cb71be5ecead322d90943146f54f8a0c915eba1c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

> On June 27, 2016, 4:25 p.m., Neil Conway wrote:
> > Can you provide some benchmarks to validate whether these changes improve 
> > performance, and for what kinds of inputs? (e.g., short vs. long strings, 
> > found match vs. no-match-found).

@neil, thanks for your comments; benchmarks maybe not necessary for this case:

1. In `startsWith`, `find` will try to find the first position of `prefix`, for 
example: s is "aab", prefix is "b", `find` will return 2; the performance 
improvement dependent on the length of `s`.
2. In `endsWith`, similar cases with `startsWith`.


- Klaus


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


On June 27, 2016, 2:16 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49246/
> ---
> 
> (Updated June 27, 2016, 2:16 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5715
> https://issues.apache.org/jira/browse/MESOS-5715
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced startsWith/endsWith's performance.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/49246/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

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

(Updated June 27, 2016, 2:16 p.m.)


Review request for mesos and Michael Park.


Changes
---

Update JIRA


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


Repository: mesos


Description
---

Enhanced startsWith/endsWith's performance.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/49246/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Enhanced startsWith/endsWith's performance.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/49246/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Enhanced Value parsing.

[Klaus Ma]

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

(Updated June 27, 2016, 11:22 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

typo


Repository: mesos


Description (updated)
---

Enhanced Value parsing:

1. Did not support `[1-2, [3-4]]` as Ranges; it should be `[1-2, 3-4]`.
2. Did not support `{a{b, c}d}` as Set; it should be `{ab, cd}`
3. Add check for Text against `[a-zA-Z0-9_/.-]`


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/attributes_tests.cpp cb71be5ecead322d90943146f54f8a0c915eba1c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Enhanced Value parsing.

[Klaus Ma]

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

(Updated June 27, 2016, 11:18 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Enhance UT cases.


Summary (updated)
-

Enhanced Value parsing.


Repository: mesos


Description (updated)
---

Enhanced Value parsing:

1. Did not support `[1-2, [3-4]]` as Ranges; it should be `[1-2, 3-4]`.
2. Did not support `{a{b, c}d}` as Set; it should be `{ab, cd}`
3. Add check for Test agains `[a-zA-Z0-9_/.-]`


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/attributes_tests.cpp cb71be5ecead322d90943146f54f8a0c915eba1c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/49223/diff/


Testing (updated)
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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



Sync up with mycpark offline, I'll also update the string version of 
`startsWith/endsWith` to improve the performance.

- Klaus Ma


On June 26, 2016, 5:46 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49140/
> ---
> 
> (Updated June 26, 2016, 5:46 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5692
> https://issues.apache.org/jira/browse/MESOS-5692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added startsWith/endsWith to support char.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/49140/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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

(Updated June 26, 2016, 5:46 p.m.)


Review request for mesos and Michael Park.


Changes
---

rebase


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


Repository: mesos


Description
---

Added startsWith/endsWith to support char.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/49140/diff/


Testing
---

make


Thanks,

Klaus Ma

Re: Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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

(Updated June 26, 2016, 5:40 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address mycpark's comments.


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


Repository: mesos


Description
---

Added startsWith/endsWith to support char.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 9bae71246e751e491be5a989eea8aca29c9aa751 
  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  docs/contributors.yaml 083715f23bb2d0610d94dd8995a026254125 
  docs/home.md 2d728333a3c1421b310d861b71c92691fd41a482 
  docs/networking.md f6652f58b02edee08e0b2410c23b2beb4d25e83b 
  docs/port-mapping-isolator.md 6a6bab9df3f6f3960ceeab38b6302823dceae9c2 
  docs/submitting-a-patch.md ffc6e561b8721b8849ef6025c15936ea712d3bfa 
  src/Makefile.am 61789a7603dac08a5f8ac4fe3d63b43e123ed98a 
  src/common/http.hpp 55bd0ac81af80c656a4a80766a3e4b21db9cf0cf 
  src/common/http.cpp 5cc9e86bc7966051507ce6620651d1f5d0914994 
  src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
  src/master/http.cpp ad3f723ec21dd0e9f3a3f015d4de0702625cd603 
  src/master/master.cpp d89c049326358bad509bb1455c81eb11610eeeb8 
  src/slave/containerizer/mesos/isolators/gpu/allocator.hpp 
b2eabfebef99ccebef427d144bb816adc0175ecf 
  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
769dfa242ed5322a72b65fbb422894e70af2ad0c 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
7678a7c81c3cdb27410c1f066021eb34bd02a83f 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
f9056c90f1075cb19c4f554e7d5b629561d06035 
  src/slave/http.cpp c038bf0c9680ec86f77f1a27efeb7354a9e67627 
  src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
  src/tests/containerizer/cni_isolator_tests.cpp 
991823b96f8eac7539625ef0f1e045e6a10464ac 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
28ec3f9954576d78153e9d0f57e22a240e950639 
  src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
  src/tests/mesos.hpp bcd060ec07da27db8738950bd81ead37da11ee70 
  src/tests/mesos.cpp 6918343b13a735aec243b54a9fcbced056894f53 
  src/tests/persistent_volume_endpoints_tests.cpp 
6c85e19eeaa69bf3a4e3077261331191db6eec06 
  src/tests/reservation_endpoints_tests.cpp 
3ee59d5db0089dd59acfe48a77910d069ffc377b 
  src/tests/slave_authorization_tests.cpp 
18bcb0e499a9d2d84113b5b9e609e5e40913ebcc 
  support/docker_build.sh 8ae1aadbc12b12e44984d34ccfbcb8a97bf05bcf 

Diff: https://reviews.apache.org/r/49140/diff/


Testing
---

make


Thanks,

Klaus Ma

Review Request 49223: WIP: enhance value parsing.

[Klaus Ma]

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

Review request for mesos.


Repository: mesos


Description
---

WIP: enhance value parsing.


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 

Diff: https://reviews.apache.org/r/49223/diff/


Testing
---

make


Thanks,

Klaus Ma

Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Added startsWith/endsWith to support char.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/49140/diff/


Testing
---

make


Thanks,

Klaus Ma

Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.

[Klaus Ma]

> On June 19, 2016, 10:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1277-1279
> > 
> >
> > This logic seems also allow agents without gpu resources offered to the 
> > framework with gpuCapble, is it expected behavior?
> 
> Kevin Klues wrote:
> Yes, that was my intended behaviour as I wrote it. If we think there is 
> some problem with this, we should discuss it further.
> 
> Guangya Liu wrote:
> So what is the use of the agent without GPU for the framework which 
> request gpu resources?

As far as I known, some GPU application can also run on CPU.


- Klaus


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


On June 19, 2016, 6:07 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48914/
> ---
> 
> (Updated June 19, 2016, 6:07 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5634
> https://issues.apache.org/jira/browse/MESOS-5634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the scarce resource problem described in MESOS-5377, we are
> introducing a GPU_RESOURCES Framework capability. This capability
> allows the Mesos allocator to make better decisions about which
> frameworks should receive resources from GPU capable machines. In
> essence, the allocator ONLY allocate resources from GPU capable
> machines to frameworks that have this capability. This is necessary to
> prevent non-GPU workloads from filling up the GPU machines and
> preventing GPU workloads to run.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e4c5bd31cf035707036eb509336fe051119b4e78 
>   include/mesos/v1/mesos.proto 9be22f02861f1eb89ab547d88530faf90ebee7ab 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9c6b23abe2b0cb16412f1ed90165f8d0c14552fa 
>   src/master/allocator/mesos/hierarchical.cpp 
> 8b7b3afb5770c617918ec4864faaf8d8a7a81ef2 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> e06d107f2dcdb9b470e330c8ceee66a54220d41b 
> 
> Diff: https://reviews.apache.org/r/48914/diff/
> 
> 
> Testing
> ---
> 
> $ make -j check; sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

[Klaus Ma]

> On June 13, 2016, 11:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > <https://reviews.apache.org/r/48593/diff/2/?file=1415977#file1415977line411>
> >
> > Any data on performance improvement?
> 
> Yanyan Hu wrote:
> Hi, Klaus, performance data is listed here: 
> https://issues.apache.org/jira/browse/MESOS-5425# thanks.
> 
> Klaus Ma wrote:
> Thanks; according to the data. It seems worse performance when # < 200, 
> but better performance when # > 200. I'm not sure which is general case in 
> production :).
> 
> Yanyan Hu wrote:
> Hi, if you refer to the latest result, the performance is always better 
> than current implemenation :)
> 
> https://issues.apache.org/jira/browse/MESOS-5425?focusedCommentId=15303641=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15303641

Great! would you add some benchmark test for this patch? I think current test 
cases has covered the general feature, but not benchmark for it :).


- Klaus


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


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

[Klaus Ma]

> On June 13, 2016, 11:40 a.m., Klaus Ma wrote:
> > src/common/values.cpp, lines 411-419
> > <https://reviews.apache.org/r/48593/diff/2/?file=1415977#file1415977line411>
> >
> > Any data on performance improvement?
> 
> Yanyan Hu wrote:
> Hi, Klaus, performance data is listed here: 
> https://issues.apache.org/jira/browse/MESOS-5425# thanks.

Thanks; according to the data. It seems worse performance when # < 200, but 
better performance when # > 200. I'm not sure which is general case in 
production :).


- Klaus


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


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 48593: Refactor Ranges Subtraction.

[Klaus Ma]

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




src/common/values.cpp (lines 401 - 409)
<https://reviews.apache.org/r/48593/#comment202389>

Any data on performance improvement?


- Klaus Ma


On June 12, 2016, 4:03 p.m., Yanyan Hu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48593/
> ---
> 
> (Updated June 12, 2016, 4:03 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-5425
> https://issues.apache.org/jira/browse/MESOS-5425
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch reimplement Ranges subtraction using
> IntervalSet data type: Ranges values will be
> converted to IntervalSet values for subtraction
> and the result will be converted back to Ranges
> after subtraction is done. This change is for
> fixing jira MESOS-5425.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
> 
> Diff: https://reviews.apache.org/r/48593/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yanyan Hu
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, lines 612-627
> > <https://reviews.apache.org/r/43561/diff/3/?file=1273811#file1273811line612>
> >
> > Should we add a benchmark here to understand the effect of now having 3 
> > levels of `tokenize`?
> > Is this code in any critical paths?
> 
> Klaus Ma wrote:
> This function is only uesd when parsing resources from string; it only 
> impacts the duration of `make check`. I'll try it and let you known the 
> impact.

Run `time build/src/mesos-test`, the result is almost the same :).


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46375: Updated MACHINE_UP_HELP's comments.

[Klaus Ma]

> On April 20, 2016, 4:25 a.m., Joseph Wu wrote:
> > Ship It!

@Joseph, would you help to commit it?


- Klaus


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


On April 19, 2016, 3:51 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46375/
> ---
> 
> (Updated April 19, 2016, 3:51 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated MACHINE_UP_HELP's comments.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
> 
> Diff: https://reviews.apache.org/r/46375/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/tests/values_tests.cpp, lines 39-72
> > <https://reviews.apache.org/r/43561/diff/5/?file=1310343#file1310343line39>
> >
> > It would be great to split apart this test into the respective value 
> > types, e.g.
> > 
> > ```
> > ValuesTest.ParseScalar
> > ValuesTest.ParseRanges
> > ValuesTest.ParseSet
> > ```
> > 
> > With more test cases that show what is currently accepted and what is 
> > not. There are a lot of other formats currently accepted from what I can 
> > tell (see my comment below).

Logged MESOS-5603 to trace those test refactor; I'll continue to work on this.

https://issues.apache.org/jira/browse/MESOS-5603


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/tests/values_tests.cpp, line 210
> > <https://reviews.apache.org/r/43561/diff/5/?file=1310343#file1310343line210>
> >
> > If I understand correctly, the following are currently accepted way to 
> > specify 1-4:
> > 
> > `[[1-4]]`
> > `[1-2]\n[3-4]`
> > `[1-2],[3-4]`
> > 
> > I'm ok with rejecting these in favor of the following canonical formats:
> > 
> > `[1-2,3-4]`
> > 
> > Although it doesn't have to be coalesced. However, we should notify the 
> > user mailing list that we're moving towards more strict parsing of ranges 
> > if you're going to change what we accept.
> > 
> > Ideally, we could pull in a regex or parsing expression grammar library 
> > to more formally define our formats. The current parsing code is a mess.

Logged MESOS-5602 to trace "expression grammar library", 
https://issues.apache.org/jira/browse/MESOS-5602

And i'll send an email to dev@ and user@ list for this notification.


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

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

(Updated June 11, 2016, 10:29 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
Remoortere.


Changes
---

rebase


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


Repository: mesos


Description
---

Improve Ranges parsing to handle single values.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

Diff: https://reviews.apache.org/r/43561/diff/


Testing
---

make
make check GTEST_FILTER=~"*"
./src/mesos-test


Thanks,

Klaus Ma

Re: Review Request 48581: Removed duplicated code in 'strings::tokenize()'.

[Klaus Ma]

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

(Updated June 11, 2016, 7 p.m.)


Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Removed duplicated code in 'strings::tokenize()'.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/48581/diff/


Testing (updated)
---

make && make check


Thanks,

Klaus Ma

Review Request 48581: Removed duplicated code in 'strings::tokenize()'.

[Klaus Ma]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Removed duplicated code in 'strings::tokenize()'.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

Diff: https://reviews.apache.org/r/48581/diff/


Testing
---


Thanks,

Klaus Ma

Re: Review Request 46425: Add helper function to simplify tokenize handling.

[Klaus Ma]

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

(Updated June 11, 2016, 5:21 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Add helper function to simplify tokenize handling.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  3rdparty/stout/tests/strings_tests.cpp 
b54a9dbf162403310b8bba687442e184a473f5a6 

Diff: https://reviews.apache.org/r/46425/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 48579: Reset "dirty" to false in sort().

[Klaus Ma]

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




src/master/allocator/sorter/drf/sorter.cpp (lines 137 - 142)
<https://reviews.apache.org/r/48579/#comment202259>

Dup the comments from JIRA:

In {{DRFSorter::allocated()}}, the {{allocations}} of client was updated so 
{{dirty}} should be true to trigger {{sort()}} and {{update(name)}} seems not 
necessary as the {{share}} will be re-calculated in {{sort()}}.
 
Further more, I'm thinking how many performance contribution will {{dirty}} 
help? In each allocator loop, the allocation maybe changed so the sorter should 
re-calculate the order.

To improve the performance, I think we can only update the single client in 
{{allocated}} & {{unallocated}}; and only return clients list in {{sort()}} 
(the client was sorted when inserted in {{allocated}} & {{unallocated}}).


- Klaus Ma


On June 11, 2016, 11:42 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48579/
> ---
> 
> (Updated June 11, 2016, 11:42 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Bugs: MESOS-5600
> https://issues.apache.org/jira/browse/MESOS-5600
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "dirty" was set to "true" when the total resources in cluster was
> updated. But in sorter, the "dirty" was never set back as "false" after
> re-calculate share for each clients.
> 
> This patch reset the "dirty" to "false" in sort(), this can make sure
> only one client share was updated when there are allocation changes but
> not all clients in the cluster.
> 
> This can improve the performance of sorter.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 65d473a5da0d846214c930c14d333040b2085b13 
>   src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 
> 
> Diff: https://reviews.apache.org/r/48579/diff/
> 
> 
> Testing
> ---
> 
> [==] Running 9 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 9 tests from SorterTest
> [ RUN  ] SorterTest.DRFSorter
> [   OK ] SorterTest.DRFSorter (1 ms)
> [ RUN  ] SorterTest.WDRFSorter
> [   OK ] SorterTest.WDRFSorter (1 ms)
> [ RUN  ] SorterTest.SplitResourceShares
> [   OK ] SorterTest.SplitResourceShares (0 ms)
> [ RUN  ] SorterTest.UpdateAllocation
> [   OK ] SorterTest.UpdateAllocation (0 ms)
> [ RUN  ] SorterTest.MultipleSlaves
> [   OK ] SorterTest.MultipleSlaves (0 ms)
> [ RUN  ] SorterTest.MultipleSlavesUpdateAllocation
> [   OK ] SorterTest.MultipleSlavesUpdateAllocation (1 ms)
> [ RUN  ] SorterTest.UpdateTotal
> [   OK ] SorterTest.UpdateTotal (0 ms)
> [ RUN  ] SorterTest.MultipleSlavesUpdateTotal
> [   OK ] SorterTest.MultipleSlavesUpdateTotal (0 ms)
> [ RUN  ] SorterTest.RevocableResources
> [   OK ] SorterTest.RevocableResources (0 ms)
> [--] 9 tests from SorterTest (37 ms total)
> 
> [--] Global test environment tear-down
> [==] 9 tests from 1 test case ran. (48 ms total)
> [  PASSED  ] 9 tests.
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/common/values.cpp, lines 654-657
> > <https://reviews.apache.org/r/43561/diff/5/?file=1310341#file1310341line654>
> >
> > Can you pull this cleanup into a separate patch? Note that you don't 
> > need the variable:
> > 
> > ```
> >   foreach (const string& token, sstrings::tokenize(temp, "{},\n")) {
> > set->add_item(token);
> >   }
> > ```

Sorry for un-based code, this's handled in r/45813.


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > Hi Klaus sorry for the delay.
> > 
> > This piece of code is very old in Mesos and has some significant technical 
> > debt. We should have used a parsing library here but at the time we didn't 
> > have  because we hadn't moved to C++11 yet. From what I can tell, 
> > regex needs GCC 4.9+ to work correctly in the std:: namespace. There 
> > appears to be a tr1 regex library. Can you do some investigation on whether 
> > we can pull in a regex library? I would suggest mailing the dev@ list to 
> > see if anyone has looked into this before.
> > 
> > Let me know what you find, I'm hesitant to make the parsing even more 
> > complicated and hacky here.
> > 
> > Until then, I would suggest splitting apart the tests here for each value 
> > type, and show all accepted formats (there are a lot that we'll accept 
> > currently that we don't show in the tests!).

Sure, I'll do some investigation on regex library :).


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46425: Add helper function to simplify tokenize handling.

[Klaus Ma]

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



ping @joris :).

- Klaus Ma


On April 20, 2016, 1:48 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46425/
> ---
> 
> (Updated April 20, 2016, 1:48 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Joris Van Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5234
> https://issues.apache.org/jira/browse/MESOS-5234
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add helper function to simplify tokenize handling.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
> 7eed0f3d08cd52a07c46b6ad194496186ac205b7 
> 
> Diff: https://reviews.apache.org/r/46425/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Klaus Ma]

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



ping :).

- Klaus Ma


On Feb. 23, 2016, 7:55 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 23, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 613
> > <https://reviews.apache.org/r/43561/diff/3/?file=1273811#file1273811line613>
> >
> > @benm I wish we had support for iterating over these splicers eg:
> > `foreachtoken(temp, ",\n", [](const string& token) { ... });`
> > 
> > These helpers become rather heavy (at 3 nested) considering their 
> > utility.
> 
> Klaus Ma wrote:
> @joris, I think `foreachtoken` is also nested loop. When doing the 
> patches, there're three options to me: FSM, regex and nested loop; FSM & 
> regex seems hard to maintain, so I used nested loop.
> 
> Klaus Ma wrote:
> `foreachtoken(temp, ",\n", [](const string& token) { ... });` is a good 
> helper function; but no performance improvement. Any comments?
> 
> Klaus Ma wrote:
> @joris, I logged MESOS-5234 to trace this; I'll update patches 
> accordingly.

@joris, had a patch for `foreachtoken`; would you help to review it?

https://reviews.apache.org/r/46425/


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46425: Add helper function to simplify tokenize handling.

[Klaus Ma]

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

(Updated April 20, 2016, 1:48 p.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, and Michael Park.


Changes
---

Correct typo.


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


Repository: mesos


Description
---

Add helper function to simplify tokenize handling.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7eed0f3d08cd52a07c46b6ad194496186ac205b7 

Diff: https://reviews.apache.org/r/46425/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 46425: Add helper function to simplify tokenize handling.

[Klaus Ma]

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

Review request for mesos, Ben Mahler, Joris Van Remoortere, and Michael Park.


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


Repository: mesos


Description
---

Add helper function to simplify tokenize handling.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7eed0f3d08cd52a07c46b6ad194496186ac205b7 

Diff: https://reviews.apache.org/r/46425/diff/


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 613
> > <https://reviews.apache.org/r/43561/diff/3/?file=1273811#file1273811line613>
> >
> > @benm I wish we had support for iterating over these splicers eg:
> > `foreachtoken(temp, ",\n", [](const string& token) { ... });`
> > 
> > These helpers become rather heavy (at 3 nested) considering their 
> > utility.
> 
> Klaus Ma wrote:
> @joris, I think `foreachtoken` is also nested loop. When doing the 
> patches, there're three options to me: FSM, regex and nested loop; FSM & 
> regex seems hard to maintain, so I used nested loop.
> 
> Klaus Ma wrote:
> `foreachtoken(temp, ",\n", [](const string& token) { ... });` is a good 
> helper function; but no performance improvement. Any comments?

@joris, I logged MESOS-5234 to trace this; I'll update patches accordingly.


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, lines 612-627
> > <https://reviews.apache.org/r/43561/diff/3/?file=1273811#file1273811line612>
> >
> > Should we add a benchmark here to understand the effect of now having 3 
> > levels of `tokenize`?
> > Is this code in any critical paths?

This function is only uesd when parsing resources from string; it only impacts 
the duration of `make check`. I'll try it and let you known the impact.


> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 613
> > <https://reviews.apache.org/r/43561/diff/3/?file=1273811#file1273811line613>
> >
> > @benm I wish we had support for iterating over these splicers eg:
> > `foreachtoken(temp, ",\n", [](const string& token) { ... });`
> >     
> > These helpers become rather heavy (at 3 nested) considering their 
> > utility.
> 
> Klaus Ma wrote:
> @joris, I think `foreachtoken` is also nested loop. When doing the 
> patches, there're three options to me: FSM, regex and nested loop; FSM & 
> regex seems hard to maintain, so I used nested loop.

`foreachtoken(temp, ",\n", [](const string& token) { ... });` is a good helper 
function; but no performance improvement. Any comments?


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 46375: Updated MACHINE_UP_HELP's comments.

[Klaus Ma]

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

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


Repository: mesos


Description
---

Updated MACHINE_UP_HELP's comments.


Diffs
-

  src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 

Diff: https://reviews.apache.org/r/46375/diff/


Testing
---

make


Thanks,

Klaus Ma

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

> On April 18, 2016, 5:47 p.m., Alexander Rukletsov wrote:
> > LGTM.
> > 
> > Do you want to document somewhere that if docker task will be run on the 
> > agent, this path should only contain symbols allowed for docker volumes 
> > (that's because we always create a docker volume for task's sandbox). Or do 
> > you want to do it in a separate patch?
> 
> Klaus Ma wrote:
> I'll submit another patch for that :).

@AlexR, I updated usage out for this in this patch.


- Klaus


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


On April 19, 2016, 3:31 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 19, 2016, 3:31 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
>   src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

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

(Updated April 19, 2016, 3:31 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


Changes
---

Address AlexR's comments.


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs (updated)
-

  src/slave/flags.cpp 10d2974bd2b6e79255fc894979607f0d2d00c315 
  src/slave/main.cpp 38bd00584dd9c6a872398678b2288edeed1cd2a4 

Diff: https://reviews.apache.org/r/46298/diff/


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma

Re: Review Request 46149: Speed up DynamicReservationFramework.

[Klaus Ma]

> On April 14, 2016, 7:56 p.m., Benjamin Bannier wrote:
> > This is much better, but still long. I see that now most of the time is 
> > spent in `resourceOffers` -- can you think of a way to speed these up as 
> > well?
> 
> Klaus Ma wrote:
> @Bannier, how did you check the time in `resourceOffers`?
> 
> Benjamin Bannier wrote:
> I looked at the verbose log -- and obviously cannot convert microseconds 
> to seconds (duh). The time seems to be spent elsewhere.
> 
> Klaus Ma wrote:
> There're ~3s in allocator: we'll launch 5 tasks on 3 agents, one task per 
> agent once on reserved resources; so scheduler need `RESERVE`, `LAUNCH` and 
> `UNRESERVE` to interact with master. I think we can reduce allocator interval.

@bbannier/@mcypark :).


- Klaus


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


On April 14, 2016, 8:59 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46149/
> ---
> 
> (Updated April 14, 2016, 8:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-5166
> https://issues.apache.org/jira/browse/MESOS-5166
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> **Phenomenon**: The runtime of "ExamplesTest.DynamicReservationFramework" is 
> ~13s
> **Root Cause**: In dynamic reservation example, the framework will accept 
> offer when reserving resources, launching tasks and unreserving tasks. The 
> un-used resources will return to allocator with 5s RefusedFilters. It impact 
> the performance of this example.
> **How to Fix**: Set filters.refused_timeout to zero when launching task, 
> reserving/unreserving resources.
> 
> 
> Diffs
> -
> 
>   src/examples/dynamic_reservation_framework.cpp 
> 8f00bcf50c25cf46c3dc32e3e77370b39fbd46bc 
> 
> Diff: https://reviews.apache.org/r/46149/diff/
> 
> 
> Testing
> ---
> 
> make && make check GTEST_FILTER="ExamplesTest.*"
> 
> [==] Running 5 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 5 tests from ExamplesTest
> [ RUN  ] ExamplesTest.TestFramework
> [   OK ] ExamplesTest.TestFramework (5252 ms)
> [ RUN  ] ExamplesTest.NoExecutorFramework
> [   OK ] ExamplesTest.NoExecutorFramework (5407 ms)
> [ RUN  ] ExamplesTest.TestHTTPFramework
> [   OK ] ExamplesTest.TestHTTPFramework (1265 ms)
> [ RUN  ] ExamplesTest.PersistentVolumeFramework
> [   OK ] ExamplesTest.PersistentVolumeFramework (3379 ms)
> [ RUN  ] ExamplesTest.DynamicReservationFramework
> [   OK ] ExamplesTest.DynamicReservationFramework (5127 ms)
> [--] 5 tests from ExamplesTest (20433 ms total)
> 
> [--] Global test environment tear-down
> [==] 5 tests from 1 test case ran. (20443 ms total)
> [  PASSED  ] 5 tests.
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On April 19, 2016, 11:28 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46364/
> ---
> 
> (Updated April 19, 2016, 11:28 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes from `default` statement to explicit case
> statement (`UNKNOWN`) so that complier could help catching all
> switches when new enums are introduced in the future.
> 
> This patch is related to:
> https://reviews.apache.org/r/45317/ (MESOS-5014)
> https://reviews.apache.org/r/45304/ (MESOS-5015)
> https://reviews.apache.org/r/45342/ (MESOS-5031)
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> c744a16041c2466d3839a37fbee2bf86887bf4e1 
>   src/examples/test_http_executor.cpp 
> ceb489d43f35d24c8a7f5fbb0148529745ee357a 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46364/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44379: Use tokens size to parse perf stat format.

[Klaus Ma]

> On April 11, 2016, 10:09 p.m., Klaus Ma wrote:
> > Ship It!
> 
> Klaus Ma wrote:
> LGTM. Just wonder how to handle the input that did not follow our 
> expectation:
> 
> // value,event,cgroup
> // value,unit,event,cgroup
> // value,unit,event,cgroup,running,ratio
> 
> fan du wrote:
> Any format other than above is invalid :)

No special case for now; but if there's any exception, it's hard for us to find 
it. Anyway, current patch is OK to me :).


- Klaus


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


On April 18, 2016, 1:41 p.m., fan du wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44379/
> ---
> 
> (Updated April 18, 2016, 1:41 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4705
> https://issues.apache.org/jira/browse/MESOS-4705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Co-authored with haosdent.
> 
> 
> Diffs
> -
> 
>   src/linux/perf.cpp 749e676aaf2ce639dd976f2b23e323300c6114c5 
> 
> Diff: https://reviews.apache.org/r/44379/diff/
> 
> 
> Testing
> ---
> 
> 1. {Found and Test} with Serenity, ema filter could get perf event statistics 
> correctly as expected.
> 2. ./bin/mesos-tests.sh --gtest_filter=PerfEventIsolatorTest* 
> --log_dir=/tmp/mesos/
> 
> 
> Thanks,
> 
> fan du
> 
>

Re: Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

> On April 18, 2016, 5:47 p.m., Alexander Rukletsov wrote:
> > LGTM.
> > 
> > Do you want to document somewhere that if docker task will be run on the 
> > agent, this path should only contain symbols allowed for docker volumes 
> > (that's because we always create a docker volume for task's sandbox). Or do 
> > you want to do it in a separate patch?

I'll submit another patch for that :).


- Klaus


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


On April 16, 2016, 4:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46298/
> ---
> 
> (Updated April 16, 2016, 4:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Jie Yu.
> 
> 
> Bugs: MESOS-5123
> https://issues.apache.org/jira/browse/MESOS-5123
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rejected relative path agent work_dir.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
>   src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 
> 
> Diff: https://reviews.apache.org/r/46298/diff/
> 
> 
> Testing
> ---
> 
> 1. make && make check
> 2. e2e test: 
>  
> ```
> $ ./src/mesos-slave --work_dir=aa --master=aa
> The required option `--work_dir` must be absolute path.
> ```
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Review Request 46298: Rejected relative path agent work_dir.

[Klaus Ma]

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

Review request for mesos, Alexander Rukletsov and Jie Yu.


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


Repository: mesos


Description
---

Rejected relative path agent work_dir.


Diffs
-

  src/slave/flags.cpp 316feec26e52a8cb1f0fd0739176b3f502e1ed86 
  src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 

Diff: https://reviews.apache.org/r/46298/diff/


Testing
---

1. make && make check
2. e2e test: 
 
```
$ ./src/mesos-slave --work_dir=aa --master=aa
The required option `--work_dir` must be absolute path.
```


Thanks,

Klaus Ma

Re: Review Request 46149: Speed up DynamicReservationFramework.

[Klaus Ma]

> On April 14, 2016, 7:56 p.m., Benjamin Bannier wrote:
> > This is much better, but still long. I see that now most of the time is 
> > spent in `resourceOffers` -- can you think of a way to speed these up as 
> > well?
> 
> Klaus Ma wrote:
> @Bannier, how did you check the time in `resourceOffers`?
> 
> Benjamin Bannier wrote:
> I looked at the verbose log -- and obviously cannot convert microseconds 
> to seconds (duh). The time seems to be spent elsewhere.

There're ~3s in allocator: we'll launch 5 tasks on 3 agents, one task per agent 
once on reserved resources; so scheduler need `RESERVE`, `LAUNCH` and 
`UNRESERVE` to interact with master. I think we can reduce allocator interval.


- Klaus


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


On April 14, 2016, 8:59 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46149/
> ---
> 
> (Updated April 14, 2016, 8:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Michael Park.
> 
> 
> Bugs: MESOS-5166
> https://issues.apache.org/jira/browse/MESOS-5166
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> **Phenomenon**: The runtime of "ExamplesTest.DynamicReservationFramework" is 
> ~13s
> **Root Cause**: In dynamic reservation example, the framework will accept 
> offer when reserving resources, launching tasks and unreserving tasks. The 
> un-used resources will return to allocator with 5s RefusedFilters. It impact 
> the performance of this example.
> **How to Fix**: Set filters.refused_timeout to zero when launching task, 
> reserving/unreserving resources.
> 
> 
> Diffs
> -
> 
>   src/examples/dynamic_reservation_framework.cpp 
> 8f00bcf50c25cf46c3dc32e3e77370b39fbd46bc 
> 
> Diff: https://reviews.apache.org/r/46149/diff/
> 
> 
> Testing
> ---
> 
> make && make check GTEST_FILTER="ExamplesTest.*"
> 
> [==] Running 5 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 5 tests from ExamplesTest
> [ RUN  ] ExamplesTest.TestFramework
> [   OK ] ExamplesTest.TestFramework (5252 ms)
> [ RUN  ] ExamplesTest.NoExecutorFramework
> [   OK ] ExamplesTest.NoExecutorFramework (5407 ms)
> [ RUN  ] ExamplesTest.TestHTTPFramework
> [   OK ] ExamplesTest.TestHTTPFramework (1265 ms)
> [ RUN  ] ExamplesTest.PersistentVolumeFramework
> [   OK ] ExamplesTest.PersistentVolumeFramework (3379 ms)
> [ RUN  ] ExamplesTest.DynamicReservationFramework
> [   OK ] ExamplesTest.DynamicReservationFramework (5127 ms)
> [--] 5 tests from ExamplesTest (20433 ms total)
> 
> [--] Global test environment tear-down
> [==] 5 tests from 1 test case ran. (20443 ms total)
> [  PASSED  ] 5 tests.
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>