----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64969/#review195067 -----------------------------------------------------------
Looks pretty good! Just a few suggestions below. s/getstate/GetState/ in the review summary. Rather than making the API query part of the test parameters, why don't we just do each query we're interested in within each test setup? That would match the output I was expecting and it avoids the redundant test setups: ``` Test setup: 2000 agents with a total of 100000 running tasks and 100000 completed tasks. v0 /state response took 1.411724628secs v1 GetState application/x-protobuf response took 12.411724628secs v1 GetState application/json response took 22.411724628secs Test setup: 2000 agents with a total of 200000 running tasks and 0 completed tasks. v0 /state response took 1.411724628secs v1 GetState application/x-protobuf response took 12.411724628secs v1 GetState application/json response took 22.411724628secs ... ``` Be sure to include the v0 /state endpoint timing, since that's our baseline we'd like to acheive for v1. src/tests/master_benchmarks.cpp Lines 346 (patched) <https://reviews.apache.org/r/64969/#comment274196> Per my comment above, let's just make each query within a test setup rather than making it part of the parameters. src/tests/master_benchmarks.cpp Lines 353-355 (patched) <https://reviews.apache.org/r/64969/#comment274195> How about: 1000 agents, 10K tasks, 10K completed tasks 10000 agents, 100K tasks, 100K completed tasks 20000 agents, 200K tasks, 200K completed tasks 40000 agents, 400K tasks, 400K completed tasks We can comment out the lines that take too long to run for now. src/tests/master_benchmarks.cpp Lines 361-362 (patched) <https://reviews.apache.org/r/64969/#comment274197> Let's explain what we're doing to test the performance. i.e. we're doing the same as the failover the test to set up a lot of master state from artificial agents src/tests/master_benchmarks.cpp Lines 379 (patched) <https://reviews.apache.org/r/64969/#comment274204> Let's say we're doing this to avoid the authentication overhead, since we don't care about it in this test. src/tests/master_benchmarks.cpp Lines 381-382 (patched) <https://reviews.apache.org/r/64969/#comment274203> I don't think we care about this in this test, let's not change this. src/tests/master_benchmarks.cpp Lines 387 (patched) <https://reviews.apache.org/r/64969/#comment274198> Use a vector here? src/tests/master_benchmarks.cpp Lines 409 (patched) <https://reviews.apache.org/r/64969/#comment274199> Can you remove this? src/tests/master_benchmarks.cpp Lines 425 (patched) <https://reviews.apache.org/r/64969/#comment274200> Ditto here, no need for this src/tests/master_benchmarks.cpp Lines 448 (patched) <https://reviews.apache.org/r/64969/#comment274202> Let's not include the de-serialization time on the client side, what we're interested in is the time it takes us to get the bytes out on the wire. src/tests/master_benchmarks.cpp Lines 462 (patched) <https://reviews.apache.org/r/64969/#comment274201> you can just do v1Response.await() here - Benjamin Mahler On Jan. 8, 2018, 11:18 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64969/ > ----------------------------------------------------------- > > (Updated Jan. 8, 2018, 11:18 p.m.) > > > Review request for mesos, Benjamin Mahler, Michael Park, and Jiang Yan Xu. > > > Bugs: MESOS-8344 > https://issues.apache.org/jira/browse/MESOS-8344 > > > Repository: mesos > > > Description > ------- > > This test is based on the MasterFailover benchmark which > currently lacks things like executor. It is possible > to add these when needed. > > > Diffs > ----- > > src/tests/master_benchmarks.cpp 7d69588b3a4090ad1a755b592d2f8756ad6dff9e > > > Diff: https://reviews.apache.org/r/64969/diff/2/ > > > Testing > ------- > > Output from running on MacBook Pro with 3.3 GHz Intel Core i7, build with > `--enable-optimize`: > > Test setup: 2000 agents with a total of 100000 running tasks and 100000 > completed tasks. > Getstate application/x-protobuf query response took 12.411724628secs > > Test setup: 2000 agents with a total of 200000 running tasks and 0 completed > tasks. > Getstate application/x-protobuf query response took 24.207293467secs > > Test setup: 20000 agents with a total of 100000 running tasks and 0 completed > tasks. > Getstate application/x-protobuf query response took 16.292173637secs > > Test setup: 2000 agents with a total of 100000 running tasks and 100000 > completed tasks. > Getstate application/json query response took 2.71121739318333mins > > Test setup: 2000 agents with a total of 200000 running tasks and 0 completed > tasks. > Getstate application/json query response took 5.31970605753333mins > > Test setup: 20000 agents with a total of 100000 running tasks and 0 completed > tasks. > Getstate application/json query response took 3.18027318681667mins > > > Thanks, > > Meng Zhu > >
