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...@apache.org> on 2018/05/10 19:17:17 UTC

Re: Review Request 65995: Declined unwanted offers in `RetryOperationStatusUpdate*` SLRP tests.


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