You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2019/11/26 21:30:58 UTC
Review Request 71825: Updated mesos code to work against recordio
encoder/decoder changes.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71825/
-----------------------------------------------------------
Review request for mesos, Andrei Sekretenko and Greg Mann.
Bugs: MESOS-10026
https://issues.apache.org/jira/browse/MESOS-10026
Repository: mesos
Description
-------
The recordio encoder and decoder were updated to operate on records
as bytes instead of typed T records.
Diffs
-----
src/checks/checker_process.cpp c214bd1af24051b504e86948180c947c550ff3fd
src/common/http.hpp b9ab561e11d52b776a283687b3a19aa1b7942522
src/common/recordio.hpp 8cb2e73f16c753a85074fbf2ae9fd92ae0d2aeea
src/executor/executor.cpp b4126037ed3e14a93d4acb9d02b62115ba4ef690
src/resource_provider/http_connection.hpp 05863aa9871d73c34993901017f53eb0f22e7886
src/resource_provider/manager.cpp 427ce70a1fd746c77ef2093e82b39df9d4caff70
src/scheduler/scheduler.cpp 674483aa80bc74b654343c97892a96f49d5c7ed4
src/slave/containerizer/mesos/io/switchboard.cpp 8e02e511db6ee049dde80d9f28fdcaeaafb21db8
src/slave/http.cpp 4d68ce746a5ddb349c2cd0830f172a536ce4e551
src/tests/api_tests.cpp 393f9a27afb4d329505c9e583913d2988500b195
src/tests/common/recordio_tests.cpp 5dd68802ed385c3f7df39e2060bd1f087dbe8ef6
src/tests/containerizer/io_switchboard_tests.cpp e4431450d7d97e551e0a0a757c5250816a9f9f0c
src/tests/executor_http_api_tests.cpp 99bcafb65b5056ce6e57fb9e69aaff5dab14584d
src/tests/master/mock_master_api_subscriber.cpp a0808e85ac51da360c236e86646c661c26eeb270
src/tests/resource_provider_manager_tests.cpp 84ec70ba76ec869f69dbb1349ad83b655fe4616e
src/tests/scheduler_http_api_tests.cpp d5b5eec0df0c01c4706c9bb4438c9daa305bd376
Diff: https://reviews.apache.org/r/71825/diff/1/
Testing
-------
make check
Thanks,
Benjamin Mahler
Re: Review Request 71825: Updated mesos code to work against recordio
encoder/decoder changes.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71825/#review218889
-----------------------------------------------------------
Ship it!
Ship It!
- Andrei Sekretenko
On Dec. 3, 2019, 5:26 a.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71825/
> -----------------------------------------------------------
>
> (Updated Dec. 3, 2019, 5:26 a.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Greg Mann.
>
>
> Bugs: MESOS-10026
> https://issues.apache.org/jira/browse/MESOS-10026
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The recordio encoder and decoder were updated to operate on records
> as bytes instead of typed T records.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp c214bd1af24051b504e86948180c947c550ff3fd
> src/common/http.hpp b9ab561e11d52b776a283687b3a19aa1b7942522
> src/common/recordio.hpp 8cb2e73f16c753a85074fbf2ae9fd92ae0d2aeea
> src/executor/executor.cpp b4126037ed3e14a93d4acb9d02b62115ba4ef690
> src/resource_provider/http_connection.hpp 05863aa9871d73c34993901017f53eb0f22e7886
> src/resource_provider/manager.cpp 427ce70a1fd746c77ef2093e82b39df9d4caff70
> src/scheduler/scheduler.cpp 674483aa80bc74b654343c97892a96f49d5c7ed4
> src/slave/containerizer/mesos/io/switchboard.cpp 8e02e511db6ee049dde80d9f28fdcaeaafb21db8
> src/slave/http.cpp 4d68ce746a5ddb349c2cd0830f172a536ce4e551
> src/tests/api_tests.cpp 393f9a27afb4d329505c9e583913d2988500b195
> src/tests/common/recordio_tests.cpp 5dd68802ed385c3f7df39e2060bd1f087dbe8ef6
> src/tests/containerizer/io_switchboard_tests.cpp e4431450d7d97e551e0a0a757c5250816a9f9f0c
> src/tests/executor_http_api_tests.cpp 99bcafb65b5056ce6e57fb9e69aaff5dab14584d
> src/tests/master/mock_master_api_subscriber.cpp a0808e85ac51da360c236e86646c661c26eeb270
> src/tests/resource_provider_manager_tests.cpp 84ec70ba76ec869f69dbb1349ad83b655fe4616e
> src/tests/scheduler_http_api_tests.cpp d5b5eec0df0c01c4706c9bb4438c9daa305bd376
>
>
> Diff: https://reviews.apache.org/r/71825/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 71825: Updated mesos code to work against recordio
encoder/decoder changes.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71825/
-----------------------------------------------------------
(Updated Dec. 3, 2019, 5:26 a.m.)
Review request for mesos, Andrei Sekretenko and Greg Mann.
Changes
-------
* Adjustments for stateless encode function.
* Avoid an unnecessary temporary `Result<T>`.
Bugs: MESOS-10026
https://issues.apache.org/jira/browse/MESOS-10026
Repository: mesos
Description
-------
The recordio encoder and decoder were updated to operate on records
as bytes instead of typed T records.
Diffs (updated)
-----
src/checks/checker_process.cpp c214bd1af24051b504e86948180c947c550ff3fd
src/common/http.hpp b9ab561e11d52b776a283687b3a19aa1b7942522
src/common/recordio.hpp 8cb2e73f16c753a85074fbf2ae9fd92ae0d2aeea
src/executor/executor.cpp b4126037ed3e14a93d4acb9d02b62115ba4ef690
src/resource_provider/http_connection.hpp 05863aa9871d73c34993901017f53eb0f22e7886
src/resource_provider/manager.cpp 427ce70a1fd746c77ef2093e82b39df9d4caff70
src/scheduler/scheduler.cpp 674483aa80bc74b654343c97892a96f49d5c7ed4
src/slave/containerizer/mesos/io/switchboard.cpp 8e02e511db6ee049dde80d9f28fdcaeaafb21db8
src/slave/http.cpp 4d68ce746a5ddb349c2cd0830f172a536ce4e551
src/tests/api_tests.cpp 393f9a27afb4d329505c9e583913d2988500b195
src/tests/common/recordio_tests.cpp 5dd68802ed385c3f7df39e2060bd1f087dbe8ef6
src/tests/containerizer/io_switchboard_tests.cpp e4431450d7d97e551e0a0a757c5250816a9f9f0c
src/tests/executor_http_api_tests.cpp 99bcafb65b5056ce6e57fb9e69aaff5dab14584d
src/tests/master/mock_master_api_subscriber.cpp a0808e85ac51da360c236e86646c661c26eeb270
src/tests/resource_provider_manager_tests.cpp 84ec70ba76ec869f69dbb1349ad83b655fe4616e
src/tests/scheduler_http_api_tests.cpp d5b5eec0df0c01c4706c9bb4438c9daa305bd376
Diff: https://reviews.apache.org/r/71825/diff/2/
Changes: https://reviews.apache.org/r/71825/diff/1-2/
Testing
-------
make check
Thanks,
Benjamin Mahler
Re: Review Request 71825: Updated mesos code to work against recordio
encoder/decoder changes.
Posted by Benjamin Mahler <bm...@apache.org>.
> On Dec. 2, 2019, 2:13 p.m., Andrei Sekretenko wrote:
> > Overall makes sense, even without the depending patches. I would say that the need to feed deserializing functor into RecordIO decoder surprised me before, to some degree.
> >
> > Might require adjustment if you decide to act on my comment (re the Encoder having no state) in the previous patch (r71824).
> I would say that the need to feed deserializing functor into RecordIO decoder surprised me before, to some degree.
Good to know!
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71825/#review218879
-----------------------------------------------------------
On Nov. 26, 2019, 9:30 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71825/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2019, 9:30 p.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Greg Mann.
>
>
> Bugs: MESOS-10026
> https://issues.apache.org/jira/browse/MESOS-10026
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The recordio encoder and decoder were updated to operate on records
> as bytes instead of typed T records.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp c214bd1af24051b504e86948180c947c550ff3fd
> src/common/http.hpp b9ab561e11d52b776a283687b3a19aa1b7942522
> src/common/recordio.hpp 8cb2e73f16c753a85074fbf2ae9fd92ae0d2aeea
> src/executor/executor.cpp b4126037ed3e14a93d4acb9d02b62115ba4ef690
> src/resource_provider/http_connection.hpp 05863aa9871d73c34993901017f53eb0f22e7886
> src/resource_provider/manager.cpp 427ce70a1fd746c77ef2093e82b39df9d4caff70
> src/scheduler/scheduler.cpp 674483aa80bc74b654343c97892a96f49d5c7ed4
> src/slave/containerizer/mesos/io/switchboard.cpp 8e02e511db6ee049dde80d9f28fdcaeaafb21db8
> src/slave/http.cpp 4d68ce746a5ddb349c2cd0830f172a536ce4e551
> src/tests/api_tests.cpp 393f9a27afb4d329505c9e583913d2988500b195
> src/tests/common/recordio_tests.cpp 5dd68802ed385c3f7df39e2060bd1f087dbe8ef6
> src/tests/containerizer/io_switchboard_tests.cpp e4431450d7d97e551e0a0a757c5250816a9f9f0c
> src/tests/executor_http_api_tests.cpp 99bcafb65b5056ce6e57fb9e69aaff5dab14584d
> src/tests/master/mock_master_api_subscriber.cpp a0808e85ac51da360c236e86646c661c26eeb270
> src/tests/resource_provider_manager_tests.cpp 84ec70ba76ec869f69dbb1349ad83b655fe4616e
> src/tests/scheduler_http_api_tests.cpp d5b5eec0df0c01c4706c9bb4438c9daa305bd376
>
>
> Diff: https://reviews.apache.org/r/71825/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>
Re: Review Request 71825: Updated mesos code to work against recordio
encoder/decoder changes.
Posted by Andrei Sekretenko <as...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71825/#review218879
-----------------------------------------------------------
Overall makes sense, even without the depending patches. I would say that the need to feed deserializing functor into RecordIO decoder surprised me before, to some degree.
Might require adjustment if you decide to act on my comment (re the Encoder having no state) in the previous patch (r71824).
src/common/recordio.hpp
Line 247 (original), 249 (patched)
<https://reviews.apache.org/r/71825/#comment306791>
Is move-constructing intermediate `Result<T>` still needed?
- Andrei Sekretenko
On Nov. 26, 2019, 9:30 p.m., Benjamin Mahler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71825/
> -----------------------------------------------------------
>
> (Updated Nov. 26, 2019, 9:30 p.m.)
>
>
> Review request for mesos, Andrei Sekretenko and Greg Mann.
>
>
> Bugs: MESOS-10026
> https://issues.apache.org/jira/browse/MESOS-10026
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The recordio encoder and decoder were updated to operate on records
> as bytes instead of typed T records.
>
>
> Diffs
> -----
>
> src/checks/checker_process.cpp c214bd1af24051b504e86948180c947c550ff3fd
> src/common/http.hpp b9ab561e11d52b776a283687b3a19aa1b7942522
> src/common/recordio.hpp 8cb2e73f16c753a85074fbf2ae9fd92ae0d2aeea
> src/executor/executor.cpp b4126037ed3e14a93d4acb9d02b62115ba4ef690
> src/resource_provider/http_connection.hpp 05863aa9871d73c34993901017f53eb0f22e7886
> src/resource_provider/manager.cpp 427ce70a1fd746c77ef2093e82b39df9d4caff70
> src/scheduler/scheduler.cpp 674483aa80bc74b654343c97892a96f49d5c7ed4
> src/slave/containerizer/mesos/io/switchboard.cpp 8e02e511db6ee049dde80d9f28fdcaeaafb21db8
> src/slave/http.cpp 4d68ce746a5ddb349c2cd0830f172a536ce4e551
> src/tests/api_tests.cpp 393f9a27afb4d329505c9e583913d2988500b195
> src/tests/common/recordio_tests.cpp 5dd68802ed385c3f7df39e2060bd1f087dbe8ef6
> src/tests/containerizer/io_switchboard_tests.cpp e4431450d7d97e551e0a0a757c5250816a9f9f0c
> src/tests/executor_http_api_tests.cpp 99bcafb65b5056ce6e57fb9e69aaff5dab14584d
> src/tests/master/mock_master_api_subscriber.cpp a0808e85ac51da360c236e86646c661c26eeb270
> src/tests/resource_provider_manager_tests.cpp 84ec70ba76ec869f69dbb1349ad83b655fe4616e
> src/tests/scheduler_http_api_tests.cpp d5b5eec0df0c01c4706c9bb4438c9daa305bd376
>
>
> Diff: https://reviews.apache.org/r/71825/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Benjamin Mahler
>
>