You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2018/01/10 07:06:45 UTC

Review Request 65063: Replace `stringify()` with `jsonify()` in http serialization.

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

Review request for mesos, Benjamin Mahler and Michael Park.


Repository: mesos


Description
-------

Replace `stringify()` with `jsonify()` in http serialization.


Diffs
-----

  src/common/http.cpp 3ada1f064b4bd001cd4d7dccc186641f475011a0 


Diff: https://reviews.apache.org/r/65063/diff/1/


Testing
-------

Running on MacBook Pro with 3.3 GHz Intel Core i7, built with `--enable-optimize`:

With `stringify()`:

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

---------------------------------------------------------------------------------------

With `jsonify()`:

Test setup: 2000 agents with a total of 100000 running tasks and 100000 completed tasks.
Getstate application/json query response took 2.10034958736667mins

Test setup: 2000 agents with a total of 200000 running tasks and 0 completed tasks.
Getstate application/json query response took 3.38100704925mins

Test setup: 20000 agents with a total of 100000 running tasks and 0 completed tasks.
Getstate application/json query response took 2.2140585963mins


Thanks,

Meng Zhu


Re: Review Request 65063: Replace `stringify()` with `jsonify()` in http serialization.

Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65063/#review195377
-----------------------------------------------------------


Ship it!




Ship It!

- Michael Park


On Jan. 9, 2018, 11:06 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65063/
> -----------------------------------------------------------
> 
> (Updated Jan. 9, 2018, 11:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replace `stringify()` with `jsonify()` in http serialization.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp 3ada1f064b4bd001cd4d7dccc186641f475011a0 
> 
> 
> Diff: https://reviews.apache.org/r/65063/diff/1/
> 
> 
> Testing
> -------
> 
> Running on MacBook Pro with 3.3 GHz Intel Core i7, built with `--enable-optimize`:
> 
> With `stringify()`:
> 
> 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
> 
> ---------------------------------------------------------------------------------------
> 
> With `jsonify()`:
> 
> Test setup: 2000 agents with a total of 100000 running tasks and 100000 completed tasks.
> Getstate application/json query response took 2.10034958736667mins
> 
> Test setup: 2000 agents with a total of 200000 running tasks and 0 completed tasks.
> Getstate application/json query response took 3.38100704925mins
> 
> Test setup: 20000 agents with a total of 100000 running tasks and 0 completed tasks.
> Getstate application/json query response took 2.2140585963mins
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 65063: Use `jsonify()` in http serialization for better performance.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65063/
-----------------------------------------------------------

(Updated Jan. 13, 2018, 10:04 p.m.)


Review request for mesos, Benjamin Mahler and Michael Park.


Summary (updated)
-----------------

Use `jsonify()` in http serialization for better performance.


Repository: mesos


Description (updated)
-------

This change avoids building an in-memory JSON::Object in favor
of using jsonify to directly output the serialized JSON from
the protobuf.


Diffs (updated)
-----

  src/common/http.cpp 3ada1f064b4bd001cd4d7dccc186641f475011a0 


Diff: https://reviews.apache.org/r/65063/diff/2/

Changes: https://reviews.apache.org/r/65063/diff/1-2/


Testing (updated)
-------

Please see performance test result in #64969


Thanks,

Meng Zhu


Re: Review Request 65063: Replace `stringify()` with `jsonify()` in http serialization.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65063/#review195122
-----------------------------------------------------------



FAIL: Some Mesos tests failed.

Reviews applied: `['65063']`

Failed command: `D:\DCOS\mesos\src\mesos-tests.exe --verbose`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65063

Relevant logs:

- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65063/logs/mesos-tests-stdout.log):

```
[       OK ] GroupTest.GroupPathWithRestrictivePerms (13415 ms)
[ RUN      ] GroupTest.RetryableErrors
[       OK ] GroupTest.RetryableErrors (16829 ms)
[ RUN      ] GroupTest.LabelledGroup
[       OK ] GroupTest.LabelledGroup (70 ms)
[ RUN      ] GroupTest.ConnectTimer
[       OK ] GroupTest.ConnectTimer (26 ms)
[ RUN      ] GroupTest.TimerCleanup
[       OK ] GroupTest.TimerCleanup (13 ms)
[----------] 12 tests from GroupTest (31273 ms total)

[----------] 1 test from RegistrarZooKeeperTest
[ RUN      ] RegistrarZooKeeperTest.TaskRunning
[       OK ] RegistrarZooKeeperTest.TaskRunning (277 ms)
[----------] 1 test from RegistrarZooKeeperTest (294 ms total)

[----------] 7 tests from ZooKeeperTest
[ RUN      ] ZooKeeperTest.Auth
[       OK ] ZooKeeperTest.Auth (6742 ms)
[ RUN      ] ZooKeeperTest.SessionTimeoutNegotiation
[       OK ] ZooKeeperTest.SessionTimeoutNegotiation (62 ms)
[ RUN      ] ZooKeeperTest.Create
[       OK ] ZooKeeperTest.Create (6731 ms)
[ RUN      ] ZooKeeperTest.LeaderDetector
[       OK ] ZooKeeperTest.LeaderDetector (87 ms)
[ RUN      ] ZooKeeperTest.LeaderDetectorTimeoutHandling
[       OK ] ZooKeeperTest.LeaderDetectorTimeoutHandling (71 ms)
[ RUN      ] ZooKeeperTest.LeaderDetectorCancellationHandling
D:\DCOS\mesos\mesos\src\tests\zookeeper_tests.cpp(240): error: Failed to wait 15secs for group.join("member 1")
```

- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65063/logs/mesos-tests-stderr.log):

```
2018-01-10 08:00:42,656:5124(0x22e0):ZOO_INFO@log_env@753: Client environment:zookeeper.version=zookeeper C client 3.4.8
2018-01-10 08:00:42,657:5124(0x22e0):ZOO_INFO@log_env@759: Client environment:host.name=<not implemented>
2018-01-10 08:00:42,657:5124(0x22e0):ZOO_INFO@log_env@768: Client environment:os.name=<not implemented>
2018-01-10 08:00:42,657:5124(0x22e0):ZOO_INFO@log_env@769: Client environment:os.arch=<not implemented>
2018-01-10 08:00:42,658:5124(0x22e0):ZOO_INFO@log_env@770: Client environment:os.version=<not implemented>
2018-01-10 08:00:42,658:5124(0x22e0):ZOO_INFO@log_env@776: Client environment:user.name=<not implemented>
2018-01-10 08:00:42,659:5124(0x22e0):ZOO_INFO@log_env@787: Client environment:user.home=<not implemented>
2018-01-10 08:00:42,659:5124(0x22e0):ZOO_INFO@log_env@794: Client environment:user.dir=C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build
2018-01-10 08:00:42,659:5124(0x22e0):ZOO_INFO@zookeeper_init@827: Initiating client connection, host=127.0.0.1:56548 sessionTimeout=10000 watcher=00007FF659F613E7 sessionId=0 sessionPasswd=<null> context=000001D328205360 flags=0
2018-01-10 08:00:42,661:5124(0x211c):ZOO_INFO@check_events@1764: initiated connection to server [127.0.0.1:56548]
2018-01-10 08:00:49,326:5124(0x211c):ZOO_ERROR@handle_socket_error_msg@1702: Socket [127.0.0.1:56548] zk retcode=-7, errno=10060(Unknown error): connection to 127.0.0.1:56548 timed out (exceeded timeout by 0ms)
2018-01-10 08:00:52,659:5124(0x211c):ZOO_WARN@zookeeper_interest@1597: Exceeded deadline by 3333ms
W0110 08:00:52.662047  9304 group.cpp:495] Timed out waiting to connect to ZooKeeper. Forcing ZooKeeper session (sessionId=0) expiration
I0110 08:00:52.663086  9304 group.cpp:511] ZooKeeper session expired
2018-01-10 08:00:52,670:5124(0x211c):ZOO_INFO@check_events@1764: initiated connection to server [127.0.0.1:56548]
2018-01-10 08:00:52,670:5124(0x2458):ZOO_INFO@zookeeper_close@2579: Freeing zookeeper resources for sessionId=0

2018-01-10 08:00:52,671:5124(0x1c18):ZOO_INFO@log_env@753: Client environment:zookeeper.version=zookeeper C client 3.4.8
2018-01-10 08:00:52,672:5124(0x1c18):ZOO_INFO@log_env@759: Client environment:host.name=<not implemented>
2018-01-10 08:00:52,672:5124(0x1c18):ZOO_INFO@log_env@768: Client environment:os.name=<not implemented>
2018-01-10 08:00:52,673:5124(0x1c18):ZOO_INFO@log_env@769: Client environment:os.arch=<not implemented>
2018-01-10 08:00:52,673:5124(0x1c18):ZOO_INFO@log_env@770: Client environment:os.version=<not implemented>
2018-01-10 08:00:52,673:5124(0x1c18):ZOO_INFO@log_env@776: Client environment:user.name=<not implemented>
2018-01-10 08:00:52,674:5124(0x1c18):ZOO_INFO@log_env@787: Client environment:user.home=<not implemented>
2018-01-10 08:00:52,674:5124(0x1c18):ZOO_INFO@log_env@794: Client environment:user.dir=C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build
2018-01-10 08:00:52,674:5124(0x1c18):ZOO_INFO@zookeeper_init@827: Initiating client connection, host=127.0.0.1:56548 sessionTimeout=10000 watcher=00007FF659F613E7 sessionId=0 sessionPasswd=<null> context=000001D328204A20 flags=0
2018-01-10 08:00:52,676:5124(0x1e44):ZOO_INFO@check_events@1764: initiated connection to server [127.0.0.1:56548]
2018-01-10 08:00:57,668:5124(0x22d8):ZOO_INFO@zookeeper_close@2579: Freeing zookeeper resources for sessionId=0

```

- Mesos Reviewbot Windows


On Jan. 10, 2018, 7:06 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65063/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2018, 7:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replace `stringify()` with `jsonify()` in http serialization.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp 3ada1f064b4bd001cd4d7dccc186641f475011a0 
> 
> 
> Diff: https://reviews.apache.org/r/65063/diff/1/
> 
> 
> Testing
> -------
> 
> Running on MacBook Pro with 3.3 GHz Intel Core i7, built with `--enable-optimize`:
> 
> With `stringify()`:
> 
> 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
> 
> ---------------------------------------------------------------------------------------
> 
> With `jsonify()`:
> 
> Test setup: 2000 agents with a total of 100000 running tasks and 100000 completed tasks.
> Getstate application/json query response took 2.10034958736667mins
> 
> Test setup: 2000 agents with a total of 200000 running tasks and 0 completed tasks.
> Getstate application/json query response took 3.38100704925mins
> 
> Test setup: 20000 agents with a total of 100000 running tasks and 0 completed tasks.
> Getstate application/json query response took 2.2140585963mins
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 65063: Replace `stringify()` with `jsonify()` in http serialization.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65063/#review195283
-----------------------------------------------------------


Ship it!




In terms of the description, this change avoids building an in-memory JSON::Object in favor of using jsonify to directly output the serialized JSON from the protobuf. It would also be good to note that this change is done to improve performance.

Can you update the summary/description?

- Benjamin Mahler


On Jan. 10, 2018, 7:06 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65063/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2018, 7:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Replace `stringify()` with `jsonify()` in http serialization.
> 
> 
> Diffs
> -----
> 
>   src/common/http.cpp 3ada1f064b4bd001cd4d7dccc186641f475011a0 
> 
> 
> Diff: https://reviews.apache.org/r/65063/diff/1/
> 
> 
> Testing
> -------
> 
> Running on MacBook Pro with 3.3 GHz Intel Core i7, built with `--enable-optimize`:
> 
> With `stringify()`:
> 
> 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
> 
> ---------------------------------------------------------------------------------------
> 
> With `jsonify()`:
> 
> Test setup: 2000 agents with a total of 100000 running tasks and 100000 completed tasks.
> Getstate application/json query response took 2.10034958736667mins
> 
> Test setup: 2000 agents with a total of 200000 running tasks and 0 completed tasks.
> Getstate application/json query response took 3.38100704925mins
> 
> Test setup: 20000 agents with a total of 100000 running tasks and 0 completed tasks.
> Getstate application/json query response took 2.2140585963mins
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>