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/06/20 05:34:07 UTC
Review Request 67670: Added a unit test for disappeared profiles.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/
-----------------------------------------------------------
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
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On June 20, 2018, 3: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?
>
> Chun-Hung Hsiao wrote:
> 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?
I misread the comment, we here of course explicitly disable the default decline filter. Dropping.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205061
-----------------------------------------------------------
On June 21, 2018, 7:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 7:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On June 20, 2018, 1:14 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1066 (patched)
> > <https://reviews.apache.org/r/67670/diff/1/?file=2042689#file2042689line1066>
> >
> > Why is this required? It looks like this is the only driver for making this a `ROOT` test. If it is required we should document why; else let's make this a non-`ROOT` test.
> >
> > nit: We can remove the empty line below.
>
> Benjamin Bannier wrote:
> Did you make any changes for this in rev2? Reopening for easier tracking.
>
> Chun-Hung Hsiao wrote:
> Created https://issues.apache.org/jira/browse/MESOS-9016 for this.
>
> Chun-Hung Hsiao wrote:
> I forgot to sent out the reply yesterday :(
Dropping this. This will be resolved along with other tests when we address MESOS-9016.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205061
-----------------------------------------------------------
On June 21, 2018, 5:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 5:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On June 20, 2018, 1:14 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1066 (patched)
> > <https://reviews.apache.org/r/67670/diff/1/?file=2042689#file2042689line1066>
> >
> > Why is this required? It looks like this is the only driver for making this a `ROOT` test. If it is required we should document why; else let's make this a non-`ROOT` test.
> >
> > nit: We can remove the empty line below.
>
> Benjamin Bannier wrote:
> Did you make any changes for this in rev2? Reopening for easier tracking.
>
> Chun-Hung Hsiao wrote:
> Created https://issues.apache.org/jira/browse/MESOS-9016 for this.
I forgot to sent out the reply yesterday :(
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205061
-----------------------------------------------------------
On June 21, 2018, 5:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 5:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> 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
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On June 20, 2018, 3:14 p.m., Benjamin Bannier wrote:
> >
Does it make sense to combine this patch with e.g., https://reviews.apache.org/r/67663/?
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205061
-----------------------------------------------------------
On June 20, 2018, 7: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, 7: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
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On June 20, 2018, 1:14 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1066 (patched)
> > <https://reviews.apache.org/r/67670/diff/1/?file=2042689#file2042689line1066>
> >
> > Why is this required? It looks like this is the only driver for making this a `ROOT` test. If it is required we should document why; else let's make this a non-`ROOT` test.
> >
> > nit: We can remove the empty line below.
>
> Benjamin Bannier wrote:
> Did you make any changes for this in rev2? Reopening for easier tracking.
Created https://issues.apache.org/jira/browse/MESOS-9016 for this.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205061
-----------------------------------------------------------
On June 21, 2018, 5:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 5:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Benjamin Bannier <be...@mesosphere.io>.
> On June 20, 2018, 3:14 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1066 (patched)
> > <https://reviews.apache.org/r/67670/diff/1/?file=2042689#file2042689line1066>
> >
> > Why is this required? It looks like this is the only driver for making this a `ROOT` test. If it is required we should document why; else let's make this a non-`ROOT` test.
> >
> > nit: We can remove the empty line below.
Did you make any changes for this in rev2? Reopening for easier tracking.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205061
-----------------------------------------------------------
On June 21, 2018, 7:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 7:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205061
-----------------------------------------------------------
src/tests/storage_local_resource_provider_tests.cpp
Lines 1047 (patched)
<https://reviews.apache.org/r/67670/#comment288023>
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.
src/tests/storage_local_resource_provider_tests.cpp
Lines 1058 (patched)
<https://reviews.apache.org/r/67670/#comment288012>
Let's pass this to `StartMaster`.
src/tests/storage_local_resource_provider_tests.cpp
Lines 1066 (patched)
<https://reviews.apache.org/r/67670/#comment288011>
Why is this required? It looks like this is the only driver for making this a `ROOT` test. If it is required we should document why; else let's make this a non-`ROOT` test.
nit: We can remove the empty line below.
src/tests/storage_local_resource_provider_tests.cpp
Lines 1165-1168 (patched)
<https://reviews.apache.org/r/67670/#comment288016>
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?
src/tests/storage_local_resource_provider_tests.cpp
Lines 1180 (patched)
<https://reviews.apache.org/r/67670/#comment288017>
Let's `ASSERT` here that we were offered the 4GB `disk` we expect.
src/tests/storage_local_resource_provider_tests.cpp
Lines 1204-1205 (patched)
<https://reviews.apache.org/r/67670/#comment288019>
The point here seems to be to simulate a situation where an agent update and an operation cross each other's paths between master and agent (and them being incompatible).
Also: `s/hold/held/`
src/tests/storage_local_resource_provider_tests.cpp
Lines 1284 (patched)
<https://reviews.apache.org/r/67670/#comment288022>
`storagePool.disk().get()`?
- Benjamin Bannier
On June 20, 2018, 7: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, 7: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
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205050
-----------------------------------------------------------
Patch looks great!
Reviews applied: [65974, 65594, 65995, 65975, 65976, 65640, 67663, 65875, 67664, 67665, 67666, 67667, 67668, 67669, 67670]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
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
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205156
-----------------------------------------------------------
PASS: Mesos patch 67670 was successfully built and tested.
Reviews applied: `['65995', '65975', '65976', '65640', '67663', '65875', '67664', '67665', '67666', '67667', '67668', '67669', '67670']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67670
- Mesos Reviewbot Windows
On June 21, 2018, 5:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 5:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
> On July 10, 2018, 3:37 p.m., Benjamin Bannier wrote:
> > src/tests/storage_local_resource_provider_tests.cpp
> > Lines 1212-1217 (patched)
> > <https://reviews.apache.org/r/67670/diff/2/?file=2043805#file2043805line1212>
> >
> > Shouldn't we update before we poll?
I advance the clock first to make sure that there is only one poll. Otherwise there might be a race condition and the advance might generate an additional call. Let me add some comments about it.
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205912
-----------------------------------------------------------
On June 21, 2018, 5:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 5:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205912
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/storage_local_resource_provider_tests.cpp
Lines 1212-1217 (patched)
<https://reviews.apache.org/r/67670/#comment288838>
Shouldn't we update before we poll?
- Benjamin Bannier
On June 21, 2018, 7:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 7:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/
-----------------------------------------------------------
(Updated July 10, 2018, 9:15 p.m.)
Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
Changes
-------
Added a comment to explain why we update the profile mapping after polling.
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 (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
Diff: https://reviews.apache.org/r/67670/diff/3/
Changes: https://reviews.apache.org/r/67670/diff/2-3/
Testing
-------
sudo make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205192
-----------------------------------------------------------
Patch looks great!
Reviews applied: [65974, 65594, 65995, 65975, 65976, 65640, 67663, 65875, 67664, 67665, 67666, 67667, 67668, 67669, 67670]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On June 21, 2018, 5:02 a.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67670/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 5:02 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/2/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/
-----------------------------------------------------------
(Updated June 21, 2018, 5:02 a.m.)
Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
Changes
-------
Addressed most Benjamin's comments.
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 (updated)
-----
src/tests/storage_local_resource_provider_tests.cpp 1afe8a8e0413ef225e952cd9cd6376e5d82774e5
Diff: https://reviews.apache.org/r/67670/diff/2/
Changes: https://reviews.apache.org/r/67670/diff/1-2/
Testing
-------
sudo make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 67670: Added a unit test for disappeared profiles.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67670/#review205048
-----------------------------------------------------------
PASS: Mesos patch 67670 was successfully built and tested.
Reviews applied: `['65995', '65975', '65976', '65640', '67663', '65875', '67664', '67665', '67666', '67667', '67668', '67669', '67670']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67670
- Mesos Reviewbot Windows
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
>
>