Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-19 Thread Greg Mann 

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


Ship it!




Ship It!

- Greg Mann


On Jan. 19, 2018, 11:07 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65057/
> ---
> 
> (Updated Jan. 19, 2018, 11:07 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8363 and MESOS-8420
> https://issues.apache.org/jira/browse/MESOS-8363
> https://issues.apache.org/jira/browse/MESOS-8420
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> `StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
> verifies that operation status updates are resent to the master after
> being dropped en route to it.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65057/diff/6/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
>  --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-19 Thread Gaston Kleiman 

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

(Updated Jan. 19, 2018, 3:07 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


Changes
---

Removed unnecessary decline.


Bugs: MESOS-8363 and MESOS-8420
https://issues.apache.org/jira/browse/MESOS-8363
https://issues.apache.org/jira/browse/MESOS-8420


Repository: mesos


Description
---

This patch adds
`StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
verifies that operation status updates are resent to the master after
being dropped en route to it.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


Diff: https://reviews.apache.org/r/65057/diff/6/

Changes: https://reviews.apache.org/r/65057/diff/5-6/


Testing
---

`sudo bin/mesos-tests.sh 
--gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
 --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-19 Thread Gaston Kleiman 


> On Jan. 18, 2018, 11:20 a.m., Greg Mann wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 2226 (patched)
> > 
> >
> > Do we need this, or just being careful? If it is needed, it might not 
> > be if we add a long filter to the accept call.

At the end of the test I do the following in order to make sure that the agent 
doesn't resend the status update once it has been acknowledged:

```
  // The master acknowledged the operation status update, so the SLRP shouldn't
  // send further operation status updates.
  EXPECT_NO_FUTURE_PROTOBUFS(UpdateOperationStatusMessage(), _, _);

  Clock::advance(slave::STATUS_UPDATE_RETRY_INTERVAL_MIN);
  Clock::settle();
```

That happens after the master has received the status update, so it can (and 
will) send another offer.

I moved the decline to that part and added a comment.


- Gaston


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


On Jan. 18, 2018, 6:19 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65057/
> ---
> 
> (Updated Jan. 18, 2018, 6:19 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8363 and MESOS-8420
> https://issues.apache.org/jira/browse/MESOS-8363
> https://issues.apache.org/jira/browse/MESOS-8420
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> `StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
> verifies that operation status updates are resent to the master after
> being dropped en route to it.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65057/diff/5/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
>  --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-18 Thread Gaston Kleiman 

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

(Updated Jan. 18, 2018, 6:19 p.m.)


Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


Changes
---

Addressed feedback.


Bugs: MESOS-8363 and MESOS-8420
https://issues.apache.org/jira/browse/MESOS-8363
https://issues.apache.org/jira/browse/MESOS-8420


Repository: mesos


Description
---

This patch adds
`StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
verifies that operation status updates are resent to the master after
being dropped en route to it.


Diffs (updated)
-

  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


Diff: https://reviews.apache.org/r/65057/diff/5/

Changes: https://reviews.apache.org/r/65057/diff/4-5/


Testing
---

`sudo bin/mesos-tests.sh 
--gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
 --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-18 Thread Greg Mann 

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




src/tests/storage_local_resource_provider_tests.cpp
Lines 2132 (patched)


Maybe "To accomplish this:"



src/tests/storage_local_resource_provider_tests.cpp
Lines 2134 (patched)


s/message//



src/tests/storage_local_resource_provider_tests.cpp
Lines 2172-2179 (patched)


Could you also add a note here regarding why the order of these two is 
reversed? Chun-Hung and I have such a comment in ours if you want to borrow it 
for consistency :)



src/tests/storage_local_resource_provider_tests.cpp
Lines 2189-2191 (patched)


Could you leave a more verbose comment here as to why this is necessary? 
Chun-Hung and my patches have one if you care to borrow for consistency :)



src/tests/storage_local_resource_provider_tests.cpp
Lines 2226 (patched)


Do we need this, or just being careful? If it is needed, it might not be if 
we add a long filter to the accept call.



src/tests/storage_local_resource_provider_tests.cpp
Lines 2278 (patched)


Benjamin suggested on my RR that we figure out the issue in 
`Slave::~Slave()` that creates the need for this resume. Looking at some other 
paused tests in the codebase, many of them do not need to resume.

I agree that it's a good idea for us to fix this in 'cluster.cpp' and 
eliminate the need for this resume.


- Greg Mann


On Jan. 10, 2018, 12:25 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65057/
> ---
> 
> (Updated Jan. 10, 2018, 12:25 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8363 and MESOS-8420
> https://issues.apache.org/jira/browse/MESOS-8363
> https://issues.apache.org/jira/browse/MESOS-8420
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds
> `StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate` that
> verifies that operation status updates are resent to the master after
> being dropped en route to it.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65057/diff/4/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
>  --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-09 Thread Mesos Reviewbot Windows 

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



PASS: Mesos patch 65057 was successfully built and tested.

Reviews applied: `['65029', '65021', '65022', '65023', '65024', '65025', 
'65026', '64992', '64994', '64998', '65000', '65057']`

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

- Mesos Reviewbot Windows


On Jan. 10, 2018, 5:55 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65057/
> ---
> 
> (Updated Jan. 10, 2018, 5:55 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.
> 
> 
> Bugs: MESOS-8363 and MESOS-8420
> https://issues.apache.org/jira/browse/MESOS-8363
> https://issues.apache.org/jira/browse/MESOS-8420
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds ROOT_RetryOperationStatusUpdate that operation status
> updates are resent to the master after being dropped en route to it.
> 
> 
> Diffs
> -
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> bbfe95e9818f25fdd5405db3ad2fe355e023f743 
> 
> 
> Diff: https://reviews.apache.org/r/65057/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo bin/mesos-tests.sh 
> --gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
>  --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Review Request 65057: Tested that op status updates dropped en route to master are resent.

2018-01-09 Thread Gaston Kleiman 

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

Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jie Yu.


Bugs: 8420
https://issues.apache.org/jira/browse/8420


Repository: mesos


Description
---

This patch adds ROOT_RetryOperationStatusUpdate that operation status
updates are resent to the master after being dropped en route to it.


Diffs
-

  src/tests/storage_local_resource_provider_tests.cpp 
bbfe95e9818f25fdd5405db3ad2fe355e023f743 


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


Testing
---

`sudo bin/mesos-tests.sh 
--gtest_filter='StorageLocalResourceProviderTest.ROOT_RetryOperationStatusUpdate'
 --gtest_repeat=100 --gtest_break_on_failure` on GNU/Linux


Thanks,

Gaston Kleiman