You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2014/03/28 01:26:22 UTC
Review Request 19761: Added a performance test for registrar which is
disabled by default.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
Diff: https://reviews.apache.org/r/19761/diff/
Testing
-------
mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
Thanks,
Jiang Yan Xu
Re: Review Request 19761: Added a performance test for registrar which is
disabled by default.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review38997
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19761]
All tests passed.
- Mesos ReviewBot
On March 28, 2014, 12:26 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated March 28, 2014, 12:26 a.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On April 3, 2014, 3:51 p.m., Dominic Hamon wrote:
> > src/tests/registrar_tests.cpp, line 291
> > <https://reviews.apache.org/r/19761/diff/2/?file=548409#file548409line291>
> >
> > can you inherit from Registrar_Test and just change the storage?
RegistrarTest also inherits TestWithParam<T> but with a different T.
class RegistrarTest : public ::testing::TestWithParam<bool>
class Registrar_BENCHMARK_Test : public ::testing::TestWithParam<int>
Having Registrar_BENCHMARK_Test inherit from RegistrarTest confuses INSTANTIATE_TEST_CASE_P about the argument type.
Given the small amount the code duplication the fact that factoring out the common logic may not actually reduce the code size I think it's better to avoid it.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review39484
-----------------------------------------------------------
On April 3, 2014, 3:28 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 3:28 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1155
> https://issues.apache.org/jira/browse/MESOS-1155
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review39484
-----------------------------------------------------------
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment71882>
can you inherit from Registrar_Test and just change the storage?
- Dominic Hamon
On April 3, 2014, 3:28 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 3:28 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1155
> https://issues.apache.org/jira/browse/MESOS-1155
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review40301
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19761]
All tests passed.
- Mesos ReviewBot
On April 14, 2014, 6:45 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated April 14, 2014, 6:45 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-1155
> https://issues.apache.org/jira/browse/MESOS-1155
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/
-----------------------------------------------------------
(Updated April 14, 2014, 11:45 a.m.)
Review request for mesos and Ben Mahler.
Changes
-------
NNRF. Will commit it per BenM's suggestion but will follow up with new tests to measure how long an individual operation takes on a full registry.
Bugs: MESOS-1155
https://issues.apache.org/jira/browse/MESOS-1155
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
Diff: https://reviews.apache.org/r/19761/diff/
Testing
-------
mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
Thanks,
Jiang Yan Xu
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review39602
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19761]
All tests passed.
- Mesos ReviewBot
On April 4, 2014, 6:33 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated April 4, 2014, 6:33 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1155
> https://issues.apache.org/jira/browse/MESOS-1155
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review39815
-----------------------------------------------------------
Ship it!
Feel free to commit this after addressing the comments so that we can iterate on this!
As a follow up, per my comments on the other review, it would be nice to also know how long an individual operation takes on a full registry.
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72452>
Maybe a TODO that we want to use LogStorage when available to exercise what we're using in production.
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72453>
Add a comment about why you needed the second test frame?
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72454>
We should test up to 50,000 as I promised that to some folks. ;)
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72455>
Do you want to add a using clause for vector?
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72456>
Is it possible to use size_t instead of int for the parameterization?
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72457>
Can you put result right above the loop, below the watch starting?
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72458>
Probably we can lower the Hours(1) now, right? ;)
Ditto below.
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72459>
Comment as to why we chose to shuffle?
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment72460>
Ditto.
- Ben Mahler
On April 4, 2014, 6:33 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated April 4, 2014, 6:33 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1155
> https://issues.apache.org/jira/browse/MESOS-1155
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/
-----------------------------------------------------------
(Updated April 4, 2014, 11:33 a.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Missed a clean up line.
Bugs: MESOS-1155
https://issues.apache.org/jira/browse/MESOS-1155
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
Diff: https://reviews.apache.org/r/19761/diff/
Testing
-------
mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
Thanks,
Jiang Yan Xu
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review39563
-----------------------------------------------------------
Patch looks great!
Reviews applied: [19761]
All tests passed.
- Mesos ReviewBot
On April 3, 2014, 10:28 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 10:28 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1155
> https://issues.apache.org/jira/browse/MESOS-1155
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a benchmark test for Registrar which is
enabled by 'make bench'.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/
-----------------------------------------------------------
(Updated April 3, 2014, 3:28 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Implemented:
- TODO(bmahler): Only use LevelDBStorage or LogStorage for performance testing, otherwise just use InMemoryStorage.
- TODO(bmahler): Add a 'make bench' target for performance tests.
Summary (updated)
-----------------
Added a benchmark test for Registrar which is enabled by 'make bench'.
Bugs: MESOS-1155
https://issues.apache.org/jira/browse/MESOS-1155
Repository: mesos-git
Description
-------
See summary.
Diffs (updated)
-----
src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
Diff: https://reviews.apache.org/r/19761/diff/
Testing
-------
mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
Thanks,
Jiang Yan Xu
Re: Review Request 19761: Added a performance test for registrar which is
disabled by default.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/#review39215
-----------------------------------------------------------
src/tests/registrar_tests.cpp
<https://reviews.apache.org/r/19761/#comment71577>
Can you actually implement this TODO? Would be good starting point for us to write benchmark tests.
- Vinod Kone
On April 1, 2014, 1:13 a.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19761/
> -----------------------------------------------------------
>
> (Updated April 1, 2014, 1:13 a.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
>
> Diff: https://reviews.apache.org/r/19761/diff/
>
>
> Testing
> -------
>
> mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 19761: Added a performance test for registrar which is
disabled by default.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19761/
-----------------------------------------------------------
(Updated March 31, 2014, 6:13 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Repository: mesos-git
Description
-------
See summary.
Diffs
-----
src/tests/registrar_tests.cpp 67c26aad8185f1328a7eb05c993ed14d47dc0664
Diff: https://reviews.apache.org/r/19761/diff/
Testing
-------
mesos-tests.sh –verbose –gtest_filter=“RegistrarTest*” –gtest_also_run_disabled_tests
Thanks,
Jiang Yan Xu