You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Gaston Kleiman <ga...@mesosphere.io> on 2018/01/12 23:36:29 UTC
Review Request 65137: Added some deduplication logic to
`Master::updateOperation`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65137/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier and Greg Mann.
Repository: mesos
Description
-------
This patch adds some logic to deduplicate the status updates that are
added to `Operation::statuses`. The logic is consistent with the
handling of task status updates, but we should revisit it in MESOS-8441.
Diffs
-----
src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b
Diff: https://reviews.apache.org/r/65137/diff/1/
Testing
-------
`sudo bin/mesos-tests.sh` on GNU/Linux.
Thanks,
Gaston Kleiman
Re: Review Request 65137: Added some deduplication logic to
`Master::updateOperation`.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65137/#review195384
-----------------------------------------------------------
PASS: Mesos patch 65137 was successfully built and tested.
Reviews applied: `['65137']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65137
- Mesos Reviewbot Windows
On Jan. 13, 2018, 2:29 a.m., Gaston Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65137/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2018, 2:29 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Greg Mann.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds some logic to deduplicate the status updates that are
> added to `Operation::statuses`. The logic is consistent with the
> handling of task status updates, but we should revisit it in MESOS-8441.
>
>
> Diffs
> -----
>
> src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b
>
>
> Diff: https://reviews.apache.org/r/65137/diff/4/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh` on GNU/Linux.
>
>
> Thanks,
>
> Gaston Kleiman
>
>
Re: Review Request 65137: Added some deduplication logic to
`Master::updateOperation`.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65137/
-----------------------------------------------------------
(Updated Jan. 12, 2018, 6:29 p.m.)
Review request for mesos, Benjamin Bannier and Greg Mann.
Changes
-------
Addressed comment.
Repository: mesos
Description
-------
This patch adds some logic to deduplicate the status updates that are
added to `Operation::statuses`. The logic is consistent with the
handling of task status updates, but we should revisit it in MESOS-8441.
Diffs (updated)
-----
src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b
Diff: https://reviews.apache.org/r/65137/diff/4/
Changes: https://reviews.apache.org/r/65137/diff/3-4/
Testing
-------
`sudo bin/mesos-tests.sh` on GNU/Linux.
Thanks,
Gaston Kleiman
Re: Review Request 65137: Added some deduplication logic to
`Master::updateOperation`.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65137/#review195379
-----------------------------------------------------------
src/master/master.cpp
Lines 10358-10359 (patched)
<https://reviews.apache.org/r/65137/#comment274550>
Whoops! Looks like we won't actually push the first status onto the list as written. Should probably be:
```
if (operation->statuses().empty() ||
*(operation->statuses().rbegin()) != status) {
```
- Greg Mann
On Jan. 13, 2018, 12:28 a.m., Gaston Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65137/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2018, 12:28 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Greg Mann.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds some logic to deduplicate the status updates that are
> added to `Operation::statuses`. The logic is consistent with the
> handling of task status updates, but we should revisit it in MESOS-8441.
>
>
> Diffs
> -----
>
> src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b
>
>
> Diff: https://reviews.apache.org/r/65137/diff/3/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh` on GNU/Linux.
>
>
> Thanks,
>
> Gaston Kleiman
>
>
Re: Review Request 65137: Added some deduplication logic to
`Master::updateOperation`.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65137/#review195376
-----------------------------------------------------------
FAIL: Some Mesos tests failed.
Reviews applied: `['65093', '65095', '65096', '65072', '65137']`
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/65137
Relevant logs:
- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65137/logs/mesos-tests-stdout.log):
```
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/0 (4 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/1
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/1 (4 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/0
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/0 (10 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/1
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/1 (14 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/0
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/0 (8 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/1
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/1 (11 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/0
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/0 (16 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/1
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/1 (15 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/0
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/0 (11 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/1
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/1 (16 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/0
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/0 (9 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/1
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/1 (13 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/0
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/0 (152 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/1
[ OK ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/1 (153 ms)
[ RUN ] ContentType/ResourceProviderManagerHttpApiTest.ConvertResources/0
D:\DCOS\mesos\mesos\src\tests\resource_provider_manager_tests.cpp(1041): error: Failed to wait 15secs for offers2
```
- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65137/logs/mesos-tests-stderr.log):
```
@ 00007FF649D1CC31 google::LogMessageFatal::~LogMessageFatal
@ 00007FF6474E829B mesos::internal::master::Master::updateOperationStatus
@ 00007FF6476501DF ProtobufProcess<mesos::internal::master::Master>::_handlerM<mesos::internal::UpdateOperationStatusMessage>
@ 00007FF6475DAD59 __cdecl*& __ptr64)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<ch
@ 00007FF6476A9830 __cdecl*& __ptr64)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<ch
@ 00007FF6475DAC80 __cdecl*& __ptr64)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<ch
@ 00007FF6475F3F03 __cdecl*)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<char>,std::
@ 00007FF6475B721B __cdecl*)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<char>,std::
@ 00007FF6475E3928 __cdecl*)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<char>,std::
@ 00007FF6476AE028 __cdecl*)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<char>,std::
@ 00007FF6475E3898 __cdecl*)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<char>,std::
@ 00007FF6477D43DA __cdecl*)(mesos::internal::master::Master * __ptr64,void (__cdecl mesos::internal::master::Master::*)(mesos::internal::UpdateOperationStatusMessage const & __ptr64) __ptr64,process::UPID const & __ptr64,std::basic_string<char,std::char_traits<char>,std::
@ 00007FF645F19C9B ??
@ 00007FF64785D072 ProtobufProcess<mesos::internal::master::Master>::consume
@ 00007FF6474FE5BC mesos::internal::master::Master::_consume
@ 00007FF6474FC47D mesos::internal::master::Master::consume
@ 00007FF64785D47A process::MessageEvent::consume
@ 00007FF645C39497 process::ProcessBase::serve
@ 00007FF6496D87E0 process::ProcessManager::resume
@ 00007FF6497E1FE1 ??
@ 00007FF649720BE0 std::_Invoker_functor::_Call<<lambda_124422ac022fa041208b80c1460630d7> >
@ 00007FF649776580 std::invoke<<lambda_124422ac022fa041208b80c1460630d7> >
@ 00007FF64972FADC std::_LaunchPad<std::unique_ptr<std::tuple<<lambda_124422ac022fa041208b80c1460630d7> >,std::default_delete<std::tuple<<lambda_124422ac022fa041208b80c1460630d7> > > > >::_Execute<0>
@ 00007FF64982DB7A std::_LaunchPad<std::unique_ptr<std::tuple<<lambda_124422ac022fa041208b80c1460630d7> >,std::default_delete<std::tuple<<lambda_124422ac022fa041208b80c1460630d7> > > > >::_Run
@ 00007FF64981A608 std::_LaunchPad<std::unique_ptr<std::tuple<<lambda_124422ac022fa041208b80c1460630d7> >,std::default_delete<std::tuple<<lambda_124422ac022fa041208b80c1460630d7> > > > >::_Go
@ 00007FF6498022ED std::_Pad::_Call_func
@ 00007FF64A9CD038 invoke_thread_procedure
@ 00007FF64A9CCAE1 __cdecl*)(void * __ptr64)
@ 00007FFD31661FE4 BaseThreadInitThunk
```
- Mesos Reviewbot Windows
On Jan. 12, 2018, 4:28 p.m., Gaston Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65137/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2018, 4:28 p.m.)
>
>
> Review request for mesos, Benjamin Bannier and Greg Mann.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds some logic to deduplicate the status updates that are
> added to `Operation::statuses`. The logic is consistent with the
> handling of task status updates, but we should revisit it in MESOS-8441.
>
>
> Diffs
> -----
>
> src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b
>
>
> Diff: https://reviews.apache.org/r/65137/diff/3/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh` on GNU/Linux.
>
>
> Thanks,
>
> Gaston Kleiman
>
>
Re: Review Request 65137: Added some deduplication logic to
`Master::updateOperation`.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65137/#review195371
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On Jan. 13, 2018, 12:28 a.m., Gaston Kleiman wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65137/
> -----------------------------------------------------------
>
> (Updated Jan. 13, 2018, 12:28 a.m.)
>
>
> Review request for mesos, Benjamin Bannier and Greg Mann.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds some logic to deduplicate the status updates that are
> added to `Operation::statuses`. The logic is consistent with the
> handling of task status updates, but we should revisit it in MESOS-8441.
>
>
> Diffs
> -----
>
> src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b
>
>
> Diff: https://reviews.apache.org/r/65137/diff/3/
>
>
> Testing
> -------
>
> `sudo bin/mesos-tests.sh` on GNU/Linux.
>
>
> Thanks,
>
> Gaston Kleiman
>
>
Re: Review Request 65137: Added some deduplication logic to
`Master::updateOperation`.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65137/
-----------------------------------------------------------
(Updated Jan. 12, 2018, 4:28 p.m.)
Review request for mesos, Benjamin Bannier and Greg Mann.
Changes
-------
Use `OperationStatus::operator==` to deduplicate.
Repository: mesos
Description
-------
This patch adds some logic to deduplicate the status updates that are
added to `Operation::statuses`. The logic is consistent with the
handling of task status updates, but we should revisit it in MESOS-8441.
Diffs (updated)
-----
src/master/master.cpp c96cd7090875fbf1b11b1708390e88182c77655b
Diff: https://reviews.apache.org/r/65137/diff/2/
Changes: https://reviews.apache.org/r/65137/diff/1-2/
Testing
-------
`sudo bin/mesos-tests.sh` on GNU/Linux.
Thanks,
Gaston Kleiman