You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@mesosphere.io> on 2018/03/08 23:39:51 UTC
Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
Bugs: MESOS-8492
https://issues.apache.org/jira/browse/MESOS-8492
Repository: mesos
Description
-------
The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.
Diffs
-----
src/tests/storage_local_resource_provider_tests.cpp c2fcbf32936ef1cb6fd313eac7e3c9c5de99c1fc
Diff: https://reviews.apache.org/r/65995/diff/1/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review198919
-----------------------------------------------------------
Ship it!
Ship It!
- Gaston Kleiman
On March 8, 2018, 3:39 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated March 8, 2018, 3:39 p.m.)
>
>
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp c2fcbf32936ef1cb6fd313eac7e3c9c5de99c1fc
>
>
> Diff: https://reviews.apache.org/r/65995/diff/1/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On June 18, 2018, 12:08 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 2561-2562 (patched)
> > <https://reviews.apache.org/r/65995/diff/7/?file=2040455#file2040455line2562>
> >
> > If there is a reason to `settle` before we `advance`, we should add a comment, otherwise I would expect a sequence of first `advance`, then `settle`.
> >
> > We don't seem to be very disciplined to _always_ `settle` after and `advance` in this file, we could clean that up in a follow-up.
>
> Chun-Hung Hsiao wrote:
> Yes there's a reason to do `settle` before `advance`: we need to ensure that the allocater gets the RP resouces before doing any allocation. I'll add a comment here.
>
> However, it seems to me that it needs not to always `settle` after `advance`. IMO `settle` enforces many synchronizations, and sometimes enforces too many for our unit tests to reveal certain schedules that might expose data races.
> Instead, I'd prefer more specific synchronizations after `advance`. For example,
> ```
> Clock::advance(masterFlags.allocation_interval);
>
> AWAIT_READY(offers);
> ```
> The above `advance` is for triggering an offer, and thus waiting for the offer should be sufficient.
>
> WDYT?
Hmm sorry I made a mistake on the previous `settle`. Actually there's already a comment there to explain why we do the `settle` first.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review204917
-----------------------------------------------------------
On June 14, 2018, 12:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated June 14, 2018, 12:02 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8825
> https://issues.apache.org/jira/browse/MESOS-8825
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
>
>
> Diff: https://reviews.apache.org/r/65995/diff/7/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On June 18, 2018, 2:08 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 2561-2562 (patched)
> > <https://reviews.apache.org/r/65995/diff/7/?file=2040455#file2040455line2562>
> >
> > If there is a reason to `settle` before we `advance`, we should add a comment, otherwise I would expect a sequence of first `advance`, then `settle`.
> >
> > We don't seem to be very disciplined to _always_ `settle` after and `advance` in this file, we could clean that up in a follow-up.
>
> Chun-Hung Hsiao wrote:
> Yes there's a reason to do `settle` before `advance`: we need to ensure that the allocater gets the RP resouces before doing any allocation. I'll add a comment here.
>
> However, it seems to me that it needs not to always `settle` after `advance`. IMO `settle` enforces many synchronizations, and sometimes enforces too many for our unit tests to reveal certain schedules that might expose data races.
> Instead, I'd prefer more specific synchronizations after `advance`. For example,
> ```
> Clock::advance(masterFlags.allocation_interval);
>
> AWAIT_READY(offers);
> ```
> The above `advance` is for triggering an offer, and thus waiting for the offer should be sufficient.
>
> WDYT?
>
> Chun-Hung Hsiao wrote:
> Hmm sorry I made a mistake on the previous `settle`. Actually there's already a comment there to explain why we do the `settle` first.
Makes sense, dropping.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review204917
-----------------------------------------------------------
On June 14, 2018, 2:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated June 14, 2018, 2:02 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8825
> https://issues.apache.org/jira/browse/MESOS-8825
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
>
>
> Diff: https://reviews.apache.org/r/65995/diff/7/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On June 18, 2018, 12:08 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 2561-2562 (patched)
> > <https://reviews.apache.org/r/65995/diff/7/?file=2040455#file2040455line2562>
> >
> > If there is a reason to `settle` before we `advance`, we should add a comment, otherwise I would expect a sequence of first `advance`, then `settle`.
> >
> > We don't seem to be very disciplined to _always_ `settle` after and `advance` in this file, we could clean that up in a follow-up.
Yes there's a reason to do `settle` before `advance`: we need to ensure that the allocater gets the RP resouces before doing any allocation. I'll add a comment here.
However, it seems to me that it needs not to always `settle` after `advance`. IMO `settle` enforces many synchronizations, and sometimes enforces too many for our unit tests to reveal certain schedules that might expose data races.
Instead, I'd prefer more specific synchronizations after `advance`. For example,
```
Clock::advance(masterFlags.allocation_interval);
AWAIT_READY(offers);
```
The above `advance` is for triggering an offer, and thus waiting for the offer should be sufficient.
WDYT?
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review204917
-----------------------------------------------------------
On June 14, 2018, 12:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated June 14, 2018, 12:02 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8825
> https://issues.apache.org/jira/browse/MESOS-8825
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
>
>
> Diff: https://reviews.apache.org/r/65995/diff/7/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review204917
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/storage_local_resource_provider_tests.cpp
Lines 2561-2562 (patched)
<https://reviews.apache.org/r/65995/#comment287714>
If there is a reason to `settle` before we `advance`, we should add a comment, otherwise I would expect a sequence of first `advance`, then `settle`.
We don't seem to be very disciplined to _always_ `settle` after and `advance` in this file, we could clean that up in a follow-up.
src/tests/storage_local_resource_provider_tests.cpp
Lines 2718-2719 (patched)
<https://reviews.apache.org/r/65995/#comment287715>
Ditto.
- Benjamin Bannier
On June 14, 2018, 2:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated June 14, 2018, 2:02 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8825
> https://issues.apache.org/jira/browse/MESOS-8825
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
>
>
> Diff: https://reviews.apache.org/r/65995/diff/7/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
(Updated June 19, 2018, 9:16 p.m.)
Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and Jie Yu.
Changes
-------
Reverted to revision 7 since revision 8 is flaky: `Clock::settle()` won't wait for `UPDATE_STATE` calls in HTTP pipes.
Summary (updated)
-----------------
Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.
Bugs: MESOS-8825
https://issues.apache.org/jira/browse/MESOS-8825
Repository: mesos
Description (updated)
-------
The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
Diff: https://reviews.apache.org/r/65995/diff/9/
Changes: https://reviews.apache.org/r/65995/diff/8-9/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Ensured wanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
(Updated June 19, 2018, 4:31 a.m.)
Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and Jie Yu.
Summary (updated)
-----------------
Ensured wanted offers in `RetryOperationStatusUpdate*` SLRP tests.
Bugs: MESOS-8825
https://issues.apache.org/jira/browse/MESOS-8825
Repository: mesos
Description (updated)
-------
The two SLRP tests assume that SLRP will send out a RAW resource before
the test framework registers, and thus the framework would receive the
RAW resource in its first offer, but this assumption is not guaranteed.
This patch ensures the assumption by settling the clock in advance.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
Diff: https://reviews.apache.org/r/65995/diff/8/
Changes: https://reviews.apache.org/r/65995/diff/7-8/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
(Updated June 14, 2018, 12:02 a.m.)
Review request for mesos, Benjamin Bannier, Gastón Kleiman, Greg Mann, and Jie Yu.
Changes
-------
Reverted the previous change since it doesn't work.
Bugs: MESOS-8825
https://issues.apache.org/jira/browse/MESOS-8825
Repository: mesos
Description
-------
The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
Diff: https://reviews.apache.org/r/65995/diff/7/
Changes: https://reviews.apache.org/r/65995/diff/6-7/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
(Updated April 24, 2018, 1:12 a.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jie Yu.
Changes
-------
Addressed Benjamin's comment and removed unnecessary `std::bind`.
Bugs: MESOS-8492
https://issues.apache.org/jira/browse/MESOS-8492
Repository: mesos
Description
-------
The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp ccb114aacc904998dfeef4128f6d38dfb3c865c7
Diff: https://reviews.apache.org/r/65995/diff/6/
Changes: https://reviews.apache.org/r/65995/diff/5-6/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On April 20, 2018, 11:26 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 2573-2574 (patched)
> > <https://reviews.apache.org/r/65995/diff/5/?file=1996908#file1996908line2573>
> >
> > I feel this leads to slightly involved state transitions below (e.g., requiring explicit clock manipulations to restore sane state).
> >
> > How about introducing a
> >
> > auto isNotRaw = [isRaw](const Resource& r) { return !isRaw(r); };
> >
> > and then making dedicated declines,
> >
> > EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
> > std::bind(isNotRaw, lambda::_1))))
> > .WillRepeatedly(DeclineOffers());
> >
> > We should be able to remove the new clock manipulations then.
I could do the dedicated decline. However it doesn't resolve the issue I described in the comments for the explicit clock manipulation. We still need to ensure that a new offer is sent after tho unwanted offer has been declined.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review201624
-----------------------------------------------------------
On April 12, 2018, 3:31 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated April 12, 2018, 3:31 a.m.)
>
>
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 2872f1aec1a7b94fc302a533f5ae9e1be9658087
>
>
> Diff: https://reviews.apache.org/r/65995/diff/5/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On April 20, 2018, 11:26 a.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 2573-2574 (patched)
> > <https://reviews.apache.org/r/65995/diff/5/?file=1996908#file1996908line2573>
> >
> > I feel this leads to slightly involved state transitions below (e.g., requiring explicit clock manipulations to restore sane state).
> >
> > How about introducing a
> >
> > auto isNotRaw = [isRaw](const Resource& r) { return !isRaw(r); };
> >
> > and then making dedicated declines,
> >
> > EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
> > std::bind(isNotRaw, lambda::_1))))
> > .WillRepeatedly(DeclineOffers());
> >
> > We should be able to remove the new clock manipulations then.
>
> Chun-Hung Hsiao wrote:
> I could do the dedicated decline. However it doesn't resolve the issue I described in the comments for the explicit clock manipulation. We still need to ensure that a new offer is sent after tho unwanted offer has been declined.
Note that `isNotRow` cannot be implemented as above. We need
```
r.disk().has_source() &&
r.disk().source().has_profile() &&
r.disk().source().type() != Resource::DiskInfo::Source::RAW;
```
I'll change it to `hasSourceType` like what we have in other tests.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review201624
-----------------------------------------------------------
On April 24, 2018, 1:12 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated April 24, 2018, 1:12 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8825
> https://issues.apache.org/jira/browse/MESOS-8825
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp ccb114aacc904998dfeef4128f6d38dfb3c865c7
>
>
> Diff: https://reviews.apache.org/r/65995/diff/6/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review201624
-----------------------------------------------------------
src/tests/storage_local_resource_provider_tests.cpp
Lines 2573-2574 (patched)
<https://reviews.apache.org/r/65995/#comment283013>
I feel this leads to slightly involved state transitions below (e.g., requiring explicit clock manipulations to restore sane state).
How about introducing a
auto isNotRaw = [isRaw](const Resource& r) { return !isRaw(r); };
and then making dedicated declines,
EXPECT_CALL(sched, resourceOffers(&driver, OffersHaveAnyResource(
std::bind(isNotRaw, lambda::_1))))
.WillRepeatedly(DeclineOffers());
We should be able to remove the new clock manipulations then.
src/tests/storage_local_resource_provider_tests.cpp
Lines 2737-2738 (patched)
<https://reviews.apache.org/r/65995/#comment283014>
Ditto.
- Benjamin Bannier
On April 12, 2018, 5:31 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated April 12, 2018, 5:31 a.m.)
>
>
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 2872f1aec1a7b94fc302a533f5ae9e1be9658087
>
>
> Diff: https://reviews.apache.org/r/65995/diff/5/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
(Updated April 12, 2018, 3:31 a.m.)
Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
Changes
-------
Rebased.
Bugs: MESOS-8492
https://issues.apache.org/jira/browse/MESOS-8492
Repository: mesos
Description
-------
The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 2872f1aec1a7b94fc302a533f5ae9e1be9658087
Diff: https://reviews.apache.org/r/65995/diff/5/
Changes: https://reviews.apache.org/r/65995/diff/4-5/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
(Updated March 13, 2018, 9:58 p.m.)
Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
Changes
-------
Addressed Gaston's comment.
Bugs: MESOS-8492
https://issues.apache.org/jira/browse/MESOS-8492
Repository: mesos
Description
-------
The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 264d42b57fe1a8ea04c1de0a10112878c7b45d39
Diff: https://reviews.apache.org/r/65995/diff/3/
Changes: https://reviews.apache.org/r/65995/diff/2-3/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review199122
-----------------------------------------------------------
src/tests/storage_local_resource_provider_tests.cpp
Lines 2892 (patched)
<https://reviews.apache.org/r/65995/#comment279432>
nit: s/decline/declined/
- Gaston Kleiman
On March 13, 2018, 2:45 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 2:45 p.m.)
>
>
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp 264d42b57fe1a8ea04c1de0a10112878c7b45d39
>
>
> Diff: https://reviews.apache.org/r/65995/diff/2/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/
-----------------------------------------------------------
(Updated March 13, 2018, 9:45 p.m.)
Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
Changes
-------
Addressed the flakiness in the tests.
Bugs: MESOS-8492
https://issues.apache.org/jira/browse/MESOS-8492
Repository: mesos
Description
-------
The two SLRP tests assume that SLRP will send out a RAW resource in its
first `UPDATE_STATE` message, and expect that the test framework would
receive an offer containing the RAW resource in its first offer. However
this behavior is not guaranteed and should not be relied on. This patch
makes the tests decline unwanted offers by default so they no longer
rely on SLRP's internal behavior.
Diffs (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 264d42b57fe1a8ea04c1de0a10112878c7b45d39
Diff: https://reviews.apache.org/r/65995/diff/2/
Changes: https://reviews.apache.org/r/65995/diff/1-2/
Testing
-------
sudo make check
Ran the two tests in repitition.
Thanks,
Chun-Hung Hsiao
Re: Review Request 65995: Declined unwanted offers in
`RetryOperationStatusUpdate*` SLRP tests.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65995/#review199036
-----------------------------------------------------------
Ship it!
Ship It!
- Greg Mann
On March 8, 2018, 11:39 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65995/
> -----------------------------------------------------------
>
> (Updated March 8, 2018, 11:39 p.m.)
>
>
> Review request for mesos, Gaston Kleiman, Greg Mann, and Jie Yu.
>
>
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The two SLRP tests assume that SLRP will send out a RAW resource in its
> first `UPDATE_STATE` message, and expect that the test framework would
> receive an offer containing the RAW resource in its first offer. However
> this behavior is not guaranteed and should not be relied on. This patch
> makes the tests decline unwanted offers by default so they no longer
> rely on SLRP's internal behavior.
>
>
> Diffs
> -----
>
> src/tests/storage_local_resource_provider_tests.cpp c2fcbf32936ef1cb6fd313eac7e3c9c5de99c1fc
>
>
> Diff: https://reviews.apache.org/r/65995/diff/1/
>
>
> Testing
> -------
>
> sudo make check
> Ran the two tests in repitition.
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>