mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yongqiao Wang <yq...@cn.ibm.com>
Subject Re: Review Request 41672: Test case(s) for weights + allocation behaviour.
Date Mon, 22 Feb 2016 07:22:14 GMT


> On Feb. 19, 2016, 9:31 a.m., Alexander Rukletsov wrote:
> >
> 
> Adam B wrote:
>     Yongqiao, since Alex didn't get his review in before I committed the patch, could
you create a new patch addressing his feedback and link to it from the comments here? Thanks.

Thanks Adam and Alex, I have posted patch https://reviews.apache.org/r/43824/ to address the
comments as below.


> On Feb. 19, 2016, 9:31 a.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2338
> > <https://reviews.apache.org/r/41672/diff/12/?file=1203046#file1203046line2338>
> >
> >     Backticks instead of single quotes, please! Here and everywhere.

I find single quote?apostrophe and backticks are all used in our comments, You can check this
file and others like `master_quota_tests.cpp` `master_maintenance_tests.cpp`, etc. Cloud you
tell me what is the rule for using them in comments, I will follow up later. Thanks.


> On Feb. 19, 2016, 9:31 a.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 2423-2433
> > <https://reviews.apache.org/r/41672/diff/12/?file=1203046#file1203046line2423>
> >
> >     Will it be cleaner to put resources from the allocation into a hashmap by framework
id? I think this way you can even get rid of the loop.
> >     
> >     Same suggestions for the section below.

Personallly, the current implementation is cleaner, if we using a loop for this, we also need
to check the framework Id in the loop due to there allocation size and resources are different.


> On Feb. 19, 2016, 9:31 a.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 2373
> > <https://reviews.apache.org/r/41672/diff/12/?file=1203046#file1203046line2373>
> >
> >     You can put framework id's into a `std::set` instead. Given there are two allocations,
there is a guarantee, that there will be no attempts to put the same framework into the set
twice.

In the current implementation, we only need to check the allocation size for each framework,
it does not care the size of whole contianer (currentt is hashmap), so I think use hashmap
can meet the current requirement.


> On Feb. 19, 2016, 9:31 a.m., Alexander Rukletsov wrote:
> > src/tests/hierarchical_allocator_tests.cpp, lines 2402-2403
> > <https://reviews.apache.org/r/41672/diff/12/?file=1203046#file1203046line2402>
> >
> >     If you use `set` as proposed above, you can check  set against set here.

Sorry to do not understand your suggestion here, current using hashmap and use frameworkid
as it's key, here to check value for allocation number for each framework.


- Yongqiao


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


On Jan. 20, 2016, 8:36 a.m., Yongqiao Wang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41672/
> -----------------------------------------------------------
> 
> (Updated Jan. 20, 2016, 8:36 a.m.)
> 
> 
> Review request for mesos, Adam B, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4200
>     https://issues.apache.org/jira/browse/MESOS-4200
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Test case(s) for weights + allocation behaviour.
> 
> 
> Diffs
> -----
> 
>   src/tests/hierarchical_allocator_tests.cpp 953712149bd951789beb29c72779c4ac65aa48dc

> 
> Diff: https://reviews.apache.org/r/41672/diff/
> 
> 
> Testing
> -------
> 
> Make check done:
> $ ./src/mesos-tests --gtest_filter=HierarchicalAllocatorTest.UpdateWeight
> Source directory: /Users/yqwyq/Desktop/mesos
> Build directory: /Users/yqwyq/Desktop/mesos/build
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from HierarchicalAllocatorTest
> [ RUN      ] HierarchicalAllocatorTest.UpdateWeight
> [       OK ] HierarchicalAllocatorTest.UpdateWeight (87 ms)
> [----------] 1 test from HierarchicalAllocatorTest (87 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (176 ms total)
> [  PASSED  ] 1 test.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>


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