mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 69098: Added a benchmark to compare quota and nonquota allocation performance.
Date Thu, 06 Dec 2018 22:43:03 GMT


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > Thanks for updating the test comment and commit description! Much clearer now and
I think I would understand the need for a separate benchmark it if I were coming in fresh.
> > 
> > Did you happen to post perf stacks anywhere?

Will upload one in the JIRA.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 664-666 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line664>
> >
> >     No need to state the name of the variable in its comment:
> >     
> >     // Determines the the number of agents in the cluster...
> >     
> >     But if this all that it is, no need for a comment at all, it's clear from the
name what it represents.
> >     
> >     I can't quite understand the second sentence, perhaps you can clarify it? Intuitively
it's not clear why the number of agents would influence how many are needed to satisfy a quota.

Updated the comment to:

  // This determines the number of agents needed to satisfy a role's quota.
  // And total number of agents in the cluster will be
  // roleCount * agentsPerRole.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 667 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line667>
> >
> >     agentsPerRole?

Done.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 669 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line669>
> >
> >     frameworksPerRole?

Done.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 673-683 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line673>
> >
> >     Why do we need to store it as a member? Are we doing this anywhere else? I seem
to recall we just access GetParam from the test itself?

Removed.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 690 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line690>
> >
> >     This would be clearer?
> >     
> >     ```
> >     // 10 roles, 10*2 = 20 agents, 10*2 = 20 frameworks.
> >     ```
> >     
> >     etc

Done.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 690-694 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line690>
> >
> >     periods?

Done.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 766 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line766>
> >
> >     Can you pre-increment as a habit? I'm not sure if the compiler can optimize
away the post-increment copying in the case of iterators, Metric objects, etc. So as a habit
pre-incrementing seems like the better approach to adhere to

Ah, good point. Will try to remember that. Thanks for pointing that out!


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 779-780 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line779>
> >
> >     It's certainly not pristine from the perspective of the drf sorter's allocation
count and metrics for example. So we probably don't want to say pristine here.
> >     
> >     While it's a bit brittle of an assumption, it seems ok to me for now with a
note about this potentially starting from a dirty state if we change things.
> >     
> >     Did you consider using the param for this?

Yeah, should have used the param. Updated.


> On Dec. 5, 2018, 3:39 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Lines 793 (patched)
> > <https://reviews.apache.org/r/69098/diff/2/?file=2112229#file2112229line793>
> >
> >     Do we need these two start outputs? It seems like they clutter the output to
me.

Removed.


- Meng


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


On Dec. 6, 2018, 2:42 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69098/
> -----------------------------------------------------------
> 
> (Updated Dec. 6, 2018, 2:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-6630
>     https://issues.apache.org/jira/browse/MESOS-6630
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This benchmark evaluates the performance difference between nonquota
> and quota settings. In both settings, the same allocations are made
> for fair comparison. In particular, since the agent will always be
> allocated as a whole in nonquota settings, we should also avoid
> agent chopping in quota setting as well. Thus in this benchmark,
> quotas are only set to be multiples of whole agent resources.
> This is also why we have this dedicated benchmark for comparison
> rather than extending the existing quota benchmarks (which involves
> agent chopping).
> 
> 
> Diffs
> -----
> 
>   src/tests/hierarchical_allocator_benchmarks.cpp 6c6330e8cdbc705be322a7e2445b295c35ee6952

> 
> 
> Diff: https://reviews.apache.org/r/69098/diff/3/
> 
> 
> Testing
> -------
> 
> Nonquota run setup: 20 agents, 10 roles, 20 frameworks
> Made 20 allocations in 2.687627ms
> Made 0 allocation in 1.575616ms
> 
> Quota run setup: 20 agents, 10 roles, 20 frameworks
> Made 20 allocations in 6.190203ms
> Made 0 allocation in 3.978099ms
> 
> Nonquota run setup: 200 agents, 100 roles, 200 frameworks
> Made 200 allocations in 45.137172ms
> Made 0 allocation in 31.409069ms
> 
> Quota run setup: 200 agents, 100 roles, 200 frameworks
> Made 200 allocations in 356.202073ms
> Made 0 allocation in 246.514222ms
> 
> Nonquota run setup: 2000 agents, 1000 roles, 2000 frameworks
> Made 2000 allocations in 3.915063634secs
> Made 0 allocation in 3.642528426secs
> 
> Quota run setup: 2000 agents, 1000 roles, 2000 frameworks
> Made 2000 allocations in 36.141305833secs
> Made 0 allocation in 26.49620811secs
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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