You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jan Schlicht <ja...@mesosphere.io> on 2017/12/06 10:16:56 UTC
Review Request 64370: Provided resource providere infos in
'UPDATE_STATE' message.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64370/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier and Jie Yu.
Repository: mesos
Description
-------
To support sending 'ResourceProviderInfo's of all known resource
providers to the master as part of a 'UPDATE_SLAVE' message, these
information has been added to the 'UPDATE_STATE' message. Also, an agent
now keeps track of all 'ResourceProviderInfo's.
Diffs
-----
src/resource_provider/manager.cpp 8d8b2f1396230d6edba590b8b7e9b4ca51366efe
src/resource_provider/message.hpp c94e9c7f0bb512cc579b5fe569831002a32256b0
src/slave/slave.hpp 06afd522ac424faa427366219cb212ee18a81c33
src/slave/slave.cpp 49270013537356c8fe9150d757b064bc3bbae3cb
src/tests/resource_provider_manager_tests.cpp a4c19ca769e66110d9aba0bae4792df9db3fed01
Diff: https://reviews.apache.org/r/64370/diff/1/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 64370: Provided resource provider infos in
'UpdateState' message.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64370/
-----------------------------------------------------------
(Updated Dec. 7, 2017, 4:52 p.m.)
Review request for mesos, Benjamin Bannier and Jie Yu.
Changes
-------
Addressed issues.
Summary (updated)
-----------------
Provided resource provider infos in 'UpdateState' message.
Repository: mesos
Description (updated)
-------
To support sending 'ResourceProviderInfo's of all known resource
providers to the master as part of 'UpdateSlaveMessage', these
information has been added to the 'UpdateState' message that is sent
from resource provider to agents. Also, an agent now keeps track of all
'ResourceProviderInfo's.
Diffs (updated)
-----
src/resource_provider/manager.cpp 8d8b2f1396230d6edba590b8b7e9b4ca51366efe
src/resource_provider/message.hpp c94e9c7f0bb512cc579b5fe569831002a32256b0
src/slave/slave.hpp fc762fb1e8aeb57b0d7ad551d960ec4be06e8e33
src/slave/slave.cpp dd830d93f8a08720231e3d4a6421f7a058ba6093
src/tests/resource_provider_manager_tests.cpp a4c19ca769e66110d9aba0bae4792df9db3fed01
Diff: https://reviews.apache.org/r/64370/diff/2/
Changes: https://reviews.apache.org/r/64370/diff/1-2/
Testing
-------
make check
Thanks,
Jan Schlicht
Re: Review Request 64370: Provided resource providere infos in
'UPDATE_STATE' message.
Posted by Jan Schlicht <ja...@mesosphere.io>.
> On Dec. 6, 2017, 11:27 a.m., Benjamin Bannier wrote:
> > src/resource_provider/message.hpp
> > Line 79 (original), 79 (patched)
> > <https://reviews.apache.org/r/64370/diff/1/?file=1909475#file1909475line79>
> >
> > Why don't we just output the info?
This would add a lot of verbosity in the logs, IMO the ID is enough here.
- Jan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64370/#review192973
-----------------------------------------------------------
On Dec. 6, 2017, 11:16 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64370/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:16 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support sending 'ResourceProviderInfo's of all known resource
> providers to the master as part of a 'UPDATE_SLAVE' message, these
> information has been added to the 'UPDATE_STATE' message. Also, an agent
> now keeps track of all 'ResourceProviderInfo's.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp 8d8b2f1396230d6edba590b8b7e9b4ca51366efe
> src/resource_provider/message.hpp c94e9c7f0bb512cc579b5fe569831002a32256b0
> src/slave/slave.hpp 06afd522ac424faa427366219cb212ee18a81c33
> src/slave/slave.cpp 49270013537356c8fe9150d757b064bc3bbae3cb
> src/tests/resource_provider_manager_tests.cpp a4c19ca769e66110d9aba0bae4792df9db3fed01
>
>
> Diff: https://reviews.apache.org/r/64370/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>
Re: Review Request 64370: Provided resource providere infos in
'UPDATE_STATE' message.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64370/#review192973
-----------------------------------------------------------
Fix it, then Ship it!
Could you fix the mixup between `UPDATE_STATE` and `UpdateSlaveMessage` in the description?
src/resource_provider/message.hpp
Line 79 (original), 79 (patched)
<https://reviews.apache.org/r/64370/#comment271409>
Why don't we just output the info?
src/slave/slave.cpp
Line 6865 (original), 6865 (patched)
<https://reviews.apache.org/r/64370/#comment271408>
Let's add a `CHECK(message->updateState->info.has_id()` here.
src/slave/slave.cpp
Lines 6868-6875 (patched)
<https://reviews.apache.org/r/64370/#comment271410>
This could be simplified to
resourceProviderInfos[resourceProviderId] = message->updateState->info;
src/tests/resource_provider_manager_tests.cpp
Lines 344 (patched)
<https://reviews.apache.org/r/64370/#comment271411>
Let's assert that the info has an id.
- Benjamin Bannier
On Dec. 6, 2017, 11:16 a.m., Jan Schlicht wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64370/
> -----------------------------------------------------------
>
> (Updated Dec. 6, 2017, 11:16 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Jie Yu.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> To support sending 'ResourceProviderInfo's of all known resource
> providers to the master as part of a 'UPDATE_SLAVE' message, these
> information has been added to the 'UPDATE_STATE' message. Also, an agent
> now keeps track of all 'ResourceProviderInfo's.
>
>
> Diffs
> -----
>
> src/resource_provider/manager.cpp 8d8b2f1396230d6edba590b8b7e9b4ca51366efe
> src/resource_provider/message.hpp c94e9c7f0bb512cc579b5fe569831002a32256b0
> src/slave/slave.hpp 06afd522ac424faa427366219cb212ee18a81c33
> src/slave/slave.cpp 49270013537356c8fe9150d757b064bc3bbae3cb
> src/tests/resource_provider_manager_tests.cpp a4c19ca769e66110d9aba0bae4792df9db3fed01
>
>
> Diff: https://reviews.apache.org/r/64370/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jan Schlicht
>
>