Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

2018-01-12 Thread Mesos Reviewbot Windows 

---
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`.

2018-01-12 Thread Gaston Kleiman 

---
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`.

2018-01-12 Thread Greg Mann 

---
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)


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`.

2018-01-12 Thread Mesos Reviewbot Windows 

---
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):

```
@   7FF649D1CC31  google::LogMessageFatal::~LogMessageFatal
@   7FF6474E829B  mesos::internal::master::Master::updateOperationStatus
@   7FF6476501DF  
ProtobufProcess::_handlerM
@   7FF6475DAD59  __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

Re: Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

2018-01-12 Thread Greg Mann 

---
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
> 
>

Review Request 65137: Added some deduplication logic to `Master::updateOperation`.

2018-01-12 Thread Gaston Kleiman 

---
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