You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Alexander Rukletsov <ru...@gmail.com> on 2018/08/06 10:30:28 UTC
Re: Review Request 68131: Added MasterPooledStateQuery_BENCHMARK_Test.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/
-----------------------------------------------------------
(Updated Aug. 6, 2018, 10:30 a.m.)
Review request for mesos, Benno Evers and Benjamin Mahler.
Bugs: MESOS-8975
https://issues.apache.org/jira/browse/MESOS-8975
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
Diff: https://reviews.apache.org/r/68131/diff/2/
Changes: https://reviews.apache.org/r/68131/diff/1-2/
Testing
-------
See https://reviews.apache.org/r/68132/
Thanks,
Alexander Rukletsov
Re: Review Request 68131: Added MasterPooledStateQuery_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Aug. 8, 2018, 1:19 a.m., Benjamin Mahler wrote:
> > Just a high level comment, it's unfortunate that this test depends so much on timing w.r.t. to the 200ms polling interval. For example, if we actually serve the first request in less than 200ms then we wouldn't be measuring the benefits of batching.
> >
> > An idea for how to fix this:
> >
> > * Issue batches, e.g. N requests at once, when they all finish, another N requests, only need to repeat a few times. This means that the benchmark will always show how batching can help high load, and we're not assuming requests take longer than e.g. 200ms to get processed on the master. We don't need an interval here because we can just proceed with the next poll as soon as everything finished from the first one.
I think you suggestion is biased towards the batching approach: if we send request in batches then obviously processing them together is the best strategy. However, if no two '/state' requests sit in the master mailbox at any time, then batching is strictly worse. The reason why we decided to do batching is because:
1. The negative effect of batching in the second scenario is much smaller than the positive effect in the first scenario. The second scenario implies that the master mailbox is relatively empty hence an extra trip is negligible.
2. The main goal is to isolate the master workload from state-related computations and **not** speed up '/state' responses. Hence the important metric here is '/flags' response time.
I would like to keep the test not opinionated and have it mimic the request rate we saw in real world clusters.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review206962
-----------------------------------------------------------
On Aug. 6, 2018, 10:30 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 10:30 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/2/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Aug. 8, 2018, 1:19 a.m., Benjamin Mahler wrote:
> > src/tests/master_benchmarks.cpp
> > Lines 512 (patched)
> > <https://reviews.apache.org/r/68131/diff/2/?file=2068403#file2068403line512>
> >
> > The review summary needs to updated to reflect the renaming?
Good catch!
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review206962
-----------------------------------------------------------
On Aug. 9, 2018, 2:21 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2018, 2:21 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/3/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added MasterPooledStateQuery_BENCHMARK_Test.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review206962
-----------------------------------------------------------
Just a high level comment, it's unfortunate that this test depends so much on timing w.r.t. to the 200ms polling interval. For example, if we actually serve the first request in less than 200ms then we wouldn't be measuring the benefits of batching.
An idea for how to fix this:
* Issue batches, e.g. N requests at once, when they all finish, another N requests, only need to repeat a few times. This means that the benchmark will always show how batching can help high load, and we're not assuming requests take longer than e.g. 200ms to get processed on the master. We don't need an interval here because we can just proceed with the next poll as soon as everything finished from the first one.
src/tests/master_benchmarks.cpp
Lines 512 (patched)
<https://reviews.apache.org/r/68131/#comment290128>
The review summary needs to updated to reflect the renaming?
- Benjamin Mahler
On Aug. 6, 2018, 10:30 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 10:30 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/2/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review207114
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68224, 68225, 68131]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Aug. 9, 2018, 2:21 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2018, 2:21 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/3/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210314
-----------------------------------------------------------
PASS: Mesos patch 68131 was successfully built and tested.
Reviews applied: `['68131']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2571/mesos-review-68131
- Mesos Reviewbot Windows
On Nov. 4, 2018, 4:31 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2018, 4:31 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/4/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Nov. 6, 2018, 12:19 p.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Lines 616 (patched)
> > <https://reviews.apache.org/r/68131/diff/3-4/?file=2070789#file2070789line620>
> >
> > Last time we tried running this benchmark, we discovered a dead-lock caused by the interaction between the use of libprocess worker threads in both our test code and in the mesos code.
> >
> > Ideally we wouldn't use libprocess at all here, but to avoid another big change late in the review cycle, adding a check like this should be sufficient:
> > ```
> > if (numClients >= LIBPROCESS_NUM_WORKER_THREADS - 3) {
> > cerr << "Not enough worker threads for the desired number of clients.";
> > exit(1);
> > }
> > ```
Though I'm in favour of the idea, I'm not sure we can reliably get `LIBPROCESS_NUM_WORKER_THREADS` in tests. Do you think we should try find some substitute or punt on this and leave a comment?
> On Nov. 6, 2018, 12:19 p.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Line 646 (original), 702 (patched)
> > <https://reviews.apache.org/r/68131/diff/3-4/?file=2070789#file2070789line707>
> >
> > Given that we're capturing and printing baseline statistics anyways, would it make sense to print the relative slowdown between baseline performance and performance under load here, to get a metric that is comparable across Mesos versions?
I'm not sure what a formual for a standard relative slowdown would be : ). Do you have a suggestion? I'm inclining to leave it as is.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210347
-----------------------------------------------------------
On Nov. 4, 2018, 4:31 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2018, 4:31 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/4/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Nov. 6, 2018, 12:19 p.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Line 640 (original), 674-677 (patched)
> > <https://reviews.apache.org/r/68131/diff/3-4/?file=2070789#file2070789line679>
> >
> > I'm a bit confused by the intention behind the stop condition:
> >
> > My best guess is that you want to ensure that you have constant load during the whole duration of the benchmark. However, in that case it seems like the requests to `/state` should not be limited to a specific number but continue indefinitely, and `stop` should be set to true after the required number of requests to the indicator endpoint have happened.
> >
> > On the other hand, if the current implementation is as intended, I think the message should read `launching *up to* {numRequests} requests`, because there's no guarantee that the loop with requests for the indicator endpoint will be the one finishing first. Also, in this case I think the comments could be a bit more specific about the intention.
I've swapped `indicator` and the `state` endpoints in regard to stopping condition. I've also adjusted the comment. Marking as read; let me know if you think something else should be done additionally.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210347
-----------------------------------------------------------
On Nov. 18, 2018, 8:10 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2018, 8:10 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp fbfffb69930c30b038f74e0b831fc0ae41c820f0
>
>
> Diff: https://reviews.apache.org/r/68131/diff/5/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Benno Evers <be...@mesosphere.com>.
> On Nov. 6, 2018, 12:19 p.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Lines 616 (patched)
> > <https://reviews.apache.org/r/68131/diff/3-4/?file=2070789#file2070789line620>
> >
> > Last time we tried running this benchmark, we discovered a dead-lock caused by the interaction between the use of libprocess worker threads in both our test code and in the mesos code.
> >
> > Ideally we wouldn't use libprocess at all here, but to avoid another big change late in the review cycle, adding a check like this should be sufficient:
> > ```
> > if (numClients >= LIBPROCESS_NUM_WORKER_THREADS - 3) {
> > cerr << "Not enough worker threads for the desired number of clients.";
> > exit(1);
> > }
> > ```
>
> Alexander Rukletsov wrote:
> Though I'm in favour of the idea, I'm not sure we can reliably get `LIBPROCESS_NUM_WORKER_THREADS` in tests. Do you think we should try find some substitute or punt on this and leave a comment?
I think it's fine to punt for now, assuming that the comment WILL BE VERY NOTICABLE. I've created https://issues.apache.org/jira/browse/MESOS-9400 for this purpose.
> On Nov. 6, 2018, 12:19 p.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Line 646 (original), 702 (patched)
> > <https://reviews.apache.org/r/68131/diff/3-4/?file=2070789#file2070789line707>
> >
> > Given that we're capturing and printing baseline statistics anyways, would it make sense to print the relative slowdown between baseline performance and performance under load here, to get a metric that is comparable across Mesos versions?
>
> Alexander Rukletsov wrote:
> I'm not sure what a formual for a standard relative slowdown would be : ). Do you have a suggestion? I'm inclining to leave it as is.
I'm itching to sit down with a statistics textbook to figure this out, but for efficiency's sake let's leave it as is for now :)
- Benno
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210347
-----------------------------------------------------------
On Nov. 18, 2018, 8:10 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2018, 8:10 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp fbfffb69930c30b038f74e0b831fc0ae41c820f0
>
>
> Diff: https://reviews.apache.org/r/68131/diff/5/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210347
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/master_benchmarks.cpp
Line 493 (original), 503 (patched)
<https://reviews.apache.org/r/68131/#comment295016>
Do we still need the `Multiclient` suffix if we're not going to commit the `Delay` version?
src/tests/master_benchmarks.cpp
Line 598 (original), 605 (patched)
<https://reviews.apache.org/r/68131/#comment295007>
As far as I can tell, we only use `ATOMIC_VAR_INIT` in libprocess, in Mesos the usual constructor is used. (see e.g. the atomic `offerCallbacks` in `hierarchical_allocator_tests.cpp`)
Also, per our style guide `std::atomic_bool` is preferred over `std::atomic<bool>`.
src/tests/master_benchmarks.cpp
Line 600 (original), 607 (patched)
<https://reviews.apache.org/r/68131/#comment295008>
s/exist/exit/
src/tests/master_benchmarks.cpp
Line 604 (original), 611 (patched)
<https://reviews.apache.org/r/68131/#comment295010>
%s/repeateRequests/repeatedRequests/
src/tests/master_benchmarks.cpp
Lines 616 (patched)
<https://reviews.apache.org/r/68131/#comment295013>
Last time we tried running this benchmark, we discovered a dead-lock caused by the interaction between the use of libprocess worker threads in both our test code and in the mesos code.
Ideally we wouldn't use libprocess at all here, but to avoid another big change late in the review cycle, adding a check like this should be sufficient:
```
if (numClients >= LIBPROCESS_NUM_WORKER_THREADS - 3) {
cerr << "Not enough worker threads for the desired number of clients.";
exit(1);
}
```
src/tests/master_benchmarks.cpp
Lines 663 (patched)
<https://reviews.apache.org/r/68131/#comment295009>
You should probably check that the future wasn't failed before calling `.get()`.
src/tests/master_benchmarks.cpp
Line 640 (original), 674-677 (patched)
<https://reviews.apache.org/r/68131/#comment295014>
I'm a bit confused by the intention behind the stop condition:
My best guess is that you want to ensure that you have constant load during the whole duration of the benchmark. However, in that case it seems like the requests to `/state` should not be limited to a specific number but continue indefinitely, and `stop` should be set to true after the required number of requests to the indicator endpoint have happened.
On the other hand, if the current implementation is as intended, I think the message should read `launching *up to* {numRequests} requests`, because there's no guarantee that the loop with requests for the indicator endpoint will be the one finishing first. Also, in this case I think the comments could be a bit more specific about the intention.
src/tests/master_benchmarks.cpp
Lines 693 (patched)
<https://reviews.apache.org/r/68131/#comment295012>
I would advise against using `auto` here, since figuring out the actual type is not trivial. (one technically has to go the declaration of `process::collect()` in a different file)
src/tests/master_benchmarks.cpp
Line 646 (original), 702 (patched)
<https://reviews.apache.org/r/68131/#comment295015>
Given that we're capturing and printing baseline statistics anyways, would it make sense to print the relative slowdown between baseline performance and performance under load here, to get a metric that is comparable across Mesos versions?
- Benno Evers
On Nov. 4, 2018, 4:31 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2018, 4:31 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/4/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210315
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68131]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Nov. 4, 2018, 4:31 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 4, 2018, 4:31 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/4/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210677
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68131]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Nov. 18, 2018, 8:10 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2018, 8:10 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp fbfffb69930c30b038f74e0b831fc0ae41c820f0
>
>
> Diff: https://reviews.apache.org/r/68131/diff/5/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review210632
-----------------------------------------------------------
PASS: Mesos patch 68131 was successfully built and tested.
Reviews applied: `['68131']`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2615/mesos-review-68131
- Mesos Reviewbot Windows
On Nov. 18, 2018, 8:10 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Nov. 18, 2018, 8:10 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp fbfffb69930c30b038f74e0b831fc0ae41c820f0
>
>
> Diff: https://reviews.apache.org/r/68131/diff/5/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/
-----------------------------------------------------------
(Updated Nov. 18, 2018, 8:10 p.m.)
Review request for mesos, Benno Evers and Benjamin Mahler.
Bugs: MESOS-8975
https://issues.apache.org/jira/browse/MESOS-8975
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/tests/master_benchmarks.cpp fbfffb69930c30b038f74e0b831fc0ae41c820f0
Diff: https://reviews.apache.org/r/68131/diff/5/
Changes: https://reviews.apache.org/r/68131/diff/4-5/
Testing
-------
See https://reviews.apache.org/r/68132/
Thanks,
Alexander Rukletsov
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/
-----------------------------------------------------------
(Updated Nov. 4, 2018, 4:31 a.m.)
Review request for mesos, Benno Evers and Benjamin Mahler.
Bugs: MESOS-8975
https://issues.apache.org/jira/browse/MESOS-8975
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
Diff: https://reviews.apache.org/r/68131/diff/4/
Changes: https://reviews.apache.org/r/68131/diff/3-4/
Testing
-------
See https://reviews.apache.org/r/68132/
Thanks,
Alexander Rukletsov
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review208210
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68131]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Aug. 30, 2018, 8:47 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2018, 8:47 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/3/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review208152
-----------------------------------------------------------
PASS: Mesos patch 68131 was successfully built and tested.
Reviews applied: `['68131']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2266/mesos-review-68131
- Mesos Reviewbot Windows
On Aug. 30, 2018, 8:47 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 30, 2018, 8:47 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/3/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/
-----------------------------------------------------------
(Updated Aug. 30, 2018, 8:47 p.m.)
Review request for mesos, Benno Evers and Benjamin Mahler.
Bugs: MESOS-8975
https://issues.apache.org/jira/browse/MESOS-8975
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
Diff: https://reviews.apache.org/r/68131/diff/3/
Testing
-------
See https://reviews.apache.org/r/68132/
Thanks,
Alexander Rukletsov
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review207116
-----------------------------------------------------------
PASS: Mesos patch 68131 was successfully built and tested.
Reviews applied: `['68224', '68225', '68131']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2137/mesos-review-68131
- Mesos Reviewbot Windows
On Aug. 9, 2018, 7:21 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2018, 7:21 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/3/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/
-----------------------------------------------------------
(Updated Aug. 9, 2018, 2:21 p.m.)
Review request for mesos, Benno Evers and Benjamin Mahler.
Bugs: MESOS-8975
https://issues.apache.org/jira/browse/MESOS-8975
Repository: mesos
Description
-------
See summary.
Diffs (updated)
-----
src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
Diff: https://reviews.apache.org/r/68131/diff/3/
Changes: https://reviews.apache.org/r/68131/diff/2-3/
Testing
-------
See https://reviews.apache.org/r/68132/
Thanks,
Alexander Rukletsov
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/
-----------------------------------------------------------
(Updated Aug. 9, 2018, 2:15 p.m.)
Review request for mesos, Benno Evers and Benjamin Mahler.
Summary (updated)
-----------------
Added MasterActorResponsiveness_BENCHMARK_Test.
Bugs: MESOS-8975
https://issues.apache.org/jira/browse/MESOS-8975
Repository: mesos
Description
-------
See summary.
Diffs
-----
src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
Diff: https://reviews.apache.org/r/68131/diff/2/
Testing
-------
See https://reviews.apache.org/r/68132/
Thanks,
Alexander Rukletsov
Re: Review Request 68131: Added
MasterActorResponsiveness_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Aug. 6, 2018, 11:40 a.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Lines 496 (patched)
> > <https://reviews.apache.org/r/68131/diff/2/?file=2068403#file2068403line496>
> >
> > Is it possible to store the last parameter directly as `process::Milliseconds`?
Good idea.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review206877
-----------------------------------------------------------
On Aug. 9, 2018, 2:21 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 9, 2018, 2:21 p.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/3/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added MasterPooledStateQuery_BENCHMARK_Test.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Aug. 6, 2018, 11:40 a.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Lines 628 (patched)
> > <https://reviews.apache.org/r/68131/diff/2/?file=2068403#file2068403line628>
> >
> > The `async()` seems unnecessary here, we can just call the function directly.
This is true. However, it does not hurt and _looks_ more consistent, hence I'd leave it unless you have a strong opinion.
> On Aug. 6, 2018, 11:40 a.m., Benno Evers wrote:
> > src/tests/master_benchmarks.cpp
> > Lines 634 (patched)
> > <https://reviews.apache.org/r/68131/diff/2/?file=2068403#file2068403line634>
> >
> > Maybe we should add one run here where we query only `stateEndpoint` without hitting `indicatorEndpoint` at the same time?
I don't think we should. The aim of the test is to measure the responsiveness of the master actor and not the performance of the '/state' endpoint.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review206877
-----------------------------------------------------------
On Aug. 6, 2018, 10:30 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 10:30 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/2/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 68131: Added MasterPooledStateQuery_BENCHMARK_Test.
Posted by Benno Evers <be...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68131/#review206877
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/master_benchmarks.cpp
Lines 496 (patched)
<https://reviews.apache.org/r/68131/#comment289976>
Is it possible to store the last parameter directly as `process::Milliseconds`?
src/tests/master_benchmarks.cpp
Lines 628 (patched)
<https://reviews.apache.org/r/68131/#comment289977>
The `async()` seems unnecessary here, we can just call the function directly.
src/tests/master_benchmarks.cpp
Lines 634 (patched)
<https://reviews.apache.org/r/68131/#comment289978>
Maybe we should add one run here where we query only `stateEndpoint` without hitting `indicatorEndpoint` at the same time?
- Benno Evers
On Aug. 6, 2018, 10:30 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68131/
> -----------------------------------------------------------
>
> (Updated Aug. 6, 2018, 10:30 a.m.)
>
>
> Review request for mesos, Benno Evers and Benjamin Mahler.
>
>
> Bugs: MESOS-8975
> https://issues.apache.org/jira/browse/MESOS-8975
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/master_benchmarks.cpp b6d6dc7c1752491e2da854018966374b624d6682
>
>
> Diff: https://reviews.apache.org/r/68131/diff/2/
>
>
> Testing
> -------
>
> See https://reviews.apache.org/r/68132/
>
>
> Thanks,
>
> Alexander Rukletsov
>
>