mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zhitao Li <zhitaoli...@gmail.com>
Subject Re: Review Request 66220: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.
Date Tue, 17 Apr 2018 17:29:45 GMT


> On April 13, 2018, 9:43 a.m., Greg Mann wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 455-459 (patched)
> > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line455>
> >
> >     Is this enforced somewhere in validation code? Can we check for expected behavior
when a GROW/SHRINK operation is submitted for a MOUNT volume, rather than simply returning?

I added validation in r/66050 so we drop shrink volume operation on MOUNT. There is no logical
path for triggering `GROW` on `MOUNT` disk type so I'm not writing validation.

For testing, we can either keep the current structure and check that `GROW`/`SHRINK` do not
work on `MOUNT` (operation will be dropped), or take Chun's suggestion to not test them for
`MOUNT`. Please let me know.


> On April 13, 2018, 9:43 a.m., Greg Mann wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 492 (patched)
> > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line492>
> >
> >     Is this necessary? Since the clock is paused, is it really possible that we'll
get more offers than expected?

This is very common in our tests (there are more than 500 line matches with this comment through
`git grep`).

My guess is that `clock::advance` can guarantee at least one offer being triggered but there
might be some race condition cases which would make the test flaky if we do not guard against
this?


> On April 13, 2018, 9:43 a.m., Greg Mann wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 549 (patched)
> > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line549>
> >
> >     Is this necessary?

See above comment.


> On April 13, 2018, 9:43 a.m., Greg Mann wrote:
> > src/tests/persistent_volume_tests.cpp
> > Lines 553-554 (patched)
> > <https://reviews.apache.org/r/66220/diff/4/?file=1996762#file1996762line553>
> >
> >     Is this necessary? Awaiting on the task status updates may be sufficient?

See above comment.


- Zhitao


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


On April 11, 2018, 2:19 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66220/
> -----------------------------------------------------------
> 
> (Updated April 11, 2018, 2:19 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
>     https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.
> 
> 
> Diffs
> -----
> 
>   src/tests/persistent_volume_tests.cpp 4edf781711d9efdb994114aeb6289b6af750b87a 
> 
> 
> Diff: https://reviews.apache.org/r/66220/diff/4/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message