> On June 20, 2018, 1:14 p.m., Benjamin Bannier wrote:
> >
> 
> Benjamin Bannier wrote:
>     Does it make sense to combine this patch with e.g., 
> https://reviews.apache.org/r/67663/?

This is an end-to-end test involving not just the module but other 
modifications, such as r65976, r67664 and r67669, so I prefer keep this patch 
split. I'll commit this with them atomically.


> On June 20, 2018, 1:14 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1047 (patched)
> > <https://reviews.apache.org/r/67670/diff/1/?file=2042689#file2042689line1047>
> >
> >     If this number depends on e.g., the allocation interval or the number 
> > of allocation cycles we perform below, it might be worthwhile to document 
> > that. I am not sure if there's any dependency.

This should be sufficiently greater than two times the allocation interval so 
it won't be triggered when we advance the clock to get another offer.

Instead, I could add a proper synchronization to decouple this time from the 
allocation interval, e.g., returning a promise future in the mock function. Let 
me do the latter.


> On June 20, 2018, 1:14 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1058 (patched)
> > <https://reviews.apache.org/r/67670/diff/1/?file=2042689#file2042689line1058>
> >
> >     Let's pass this to `StartMaster`.

Oops again.


> On June 20, 2018, 1:14 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1165-1168 (patched)
> > <https://reviews.apache.org/r/67670/diff/1/?file=2042689#file2042689line1165>
> >
> >     This relies strongly on the way the master interprets a zero time, and  
> > how the allocation interval is related to the default value the master 
> > would currently use. Can we set a value calculated from 
> > `masterFlags.allocation_interval` instead to decouple this?

I prefer leaving this as is for consistency since we set 0 refuse seconds in 
other tests as well. What is the value in your mind? Like just setting it to 
`masterFlags.allocation_interval`? But then for event-based allocations the 
resources might still be refused?


- Chun-Hung


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


On June 20, 2018, 5:34 a.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
> 
> (Updated June 20, 2018, 5:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8995
>     https://issues.apache.org/jira/browse/MESOS-8995
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `ROOT_ProfileDisappeared` tests if a `CREATE_VOLUME` consuming a RAW
> disk with a disappeared profile will be dropped, and if the disk space
> freed by a `DESTROY_VOLUME` destroying a volume with a disappeared
> profile will be recovered with a newly appeared profile.
> 
> 
> Diffs
> -----
> 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 1afe8a8e0413ef225e952cd9cd6376e5d82774e5 
> 
> 
> Diff: https://reviews.apache.org/r/67670/diff/1/
> 
> 
> Testing
> -------
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Reply via email to