Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/#review204997 --- Ship it! Ship It! - Benjamin Bannier On June 18, 2018, 8:31 p.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65976/ > --- > > (Updated June 18, 2018, 8:31 p.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-8825 > https://issues.apache.org/jira/browse/MESOS-8825 > > > Repository: mesos > > > Description > --- > > Allowed profiles to be missing from `DiskProfileAdaptor`. > > > Diffs > - > > src/resource_provider/storage/provider.cpp > b90a4b81838fec410a97a10ce44a811bb81c87eb > src/resource_provider/storage/uri_disk_profile_adaptor.cpp > 614590ef7d1c0cc1df99b8e57c7fbd496793b5a7 > > > Diff: https://reviews.apache.org/r/65976/diff/5/ > > > Testing > --- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/ --- (Updated June 18, 2018, 6:31 p.m.) Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. Changes --- Added the checks back as suggested by Benjamin. Bugs: MESOS-8825 https://issues.apache.org/jira/browse/MESOS-8825 Repository: mesos Description --- Allowed profiles to be missing from `DiskProfileAdaptor`. Diffs (updated) - src/resource_provider/storage/provider.cpp b90a4b81838fec410a97a10ce44a811bb81c87eb src/resource_provider/storage/uri_disk_profile_adaptor.cpp 614590ef7d1c0cc1df99b8e57c7fbd496793b5a7 Diff: https://reviews.apache.org/r/65976/diff/5/ Changes: https://reviews.apache.org/r/65976/diff/4-5/ Testing --- sudo make check Thanks, Chun-Hung Hsiao
Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
> On April 20, 2018, 1:39 p.m., Benjamin Bannier wrote: > > LGTM, but I wonder whether it would make sense to keep the assertions and > > reject outdated resources more generally based on e.g., resource versions. > > Chun-Hung Hsiao wrote: > Hmm good point. I thought about it a bit. > I changed these assertions for accepted pending operations, which we have > already verified the resource versions. > The current update logic is the following. > > When SLRP receives changes in the set of known profiles: > 1. Query the disk profile adaptor to update the ProfileInfo for each > profile. > 2. Wait for all related pending operations to finish. Some of them may > want to use outdated profiles. > 3. Reconcile the storage pool. > > If we want to keep the assertion, the logic would look like the following. > > When SLRP receives changes in the set of known profiles: > 1. Wait for all related operations to finish. They are allowed to use the > outdated profiles, so new volumes with those profiles might be created > successfully. > 2. Query the disk profile adaptor to update the ProfileInfo for each > profile. > 3. Reconcile the storage pool. > > Personally I prefer the former, since it doesn't make sense to create new > volumes for old profiles. Hmm I took anotther look at the code and you're probably right that we don't need to remove the assertions since these checks are done synchronously when SLRP accepts the operations. Let me work on a unit test to to ensure that. - Chun-Hung --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/#review201631 --- On June 14, 2018, 12:07 a.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65976/ > --- > > (Updated June 14, 2018, 12:07 a.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-8825 > https://issues.apache.org/jira/browse/MESOS-8825 > > > Repository: mesos > > > Description > --- > > Allowed profiles to be missing from `DiskProfileAdaptor`. > > > Diffs > - > > src/resource_provider/storage/provider.cpp > b90a4b81838fec410a97a10ce44a811bb81c87eb > src/resource_provider/storage/uri_disk_profile_adaptor.cpp > 614590ef7d1c0cc1df99b8e57c7fbd496793b5a7 > > > Diff: https://reviews.apache.org/r/65976/diff/4/ > > > Testing > --- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
> On April 20, 2018, 1:39 p.m., Benjamin Bannier wrote: > > LGTM, but I wonder whether it would make sense to keep the assertions and > > reject outdated resources more generally based on e.g., resource versions. Hmm good point. I thought about it a bit. I changed these assertions for accepted pending operations, which we have already verified the resource versions. The current update logic is the following. When SLRP receives changes in the set of known profiles: 1. Query the disk profile adaptor to update the ProfileInfo for each profile. 2. Wait for all related pending operations to finish. Some of them may want to use outdated profiles. 3. Reconcile the storage pool. If we want to keep the assertion, the logic would look like the following. When SLRP receives changes in the set of known profiles: 1. Wait for all related operations to finish. They are allowed to use the outdated profiles, so new volumes with those profiles might be created successfully. 2. Query the disk profile adaptor to update the ProfileInfo for each profile. 3. Reconcile the storage pool. Personally I prefer the former, since it doesn't make sense to create new volumes for old profiles. - Chun-Hung --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/#review201631 --- On June 14, 2018, 12:07 a.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65976/ > --- > > (Updated June 14, 2018, 12:07 a.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-8825 > https://issues.apache.org/jira/browse/MESOS-8825 > > > Repository: mesos > > > Description > --- > > Allowed profiles to be missing from `DiskProfileAdaptor`. > > > Diffs > - > > src/resource_provider/storage/provider.cpp > b90a4b81838fec410a97a10ce44a811bb81c87eb > src/resource_provider/storage/uri_disk_profile_adaptor.cpp > 614590ef7d1c0cc1df99b8e57c7fbd496793b5a7 > > > Diff: https://reviews.apache.org/r/65976/diff/4/ > > > Testing > --- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/ --- (Updated June 14, 2018, 12:07 a.m.) Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. Changes --- Rebased. Bugs: MESOS-8825 https://issues.apache.org/jira/browse/MESOS-8825 Repository: mesos Description --- Allowed profiles to be missing from `DiskProfileAdaptor`. Diffs (updated) - src/resource_provider/storage/provider.cpp b90a4b81838fec410a97a10ce44a811bb81c87eb src/resource_provider/storage/uri_disk_profile_adaptor.cpp 614590ef7d1c0cc1df99b8e57c7fbd496793b5a7 Diff: https://reviews.apache.org/r/65976/diff/4/ Changes: https://reviews.apache.org/r/65976/diff/3-4/ Testing --- sudo make check Thanks, Chun-Hung Hsiao
Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/#review201631 --- LGTM, but I wonder whether it would make sense to keep the assertions and reject outdated resources more generally based on e.g., resource versions. - Benjamin Bannier On April 12, 2018, 5:37 a.m., Chun-Hung Hsiao wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65976/ > --- > > (Updated April 12, 2018, 5:37 a.m.) > > > Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-8492 > https://issues.apache.org/jira/browse/MESOS-8492 > > > Repository: mesos > > > Description > --- > > Allowed profiles to be missing from `DiskProfileAdaptor`. > > > Diffs > - > > src/resource_provider/storage/provider.cpp > a07620d1c4bf618f669575b3e183bf598da905a6 > src/resource_provider/storage/uri_disk_profile_adaptor.cpp > 300ea12687de487737ce91066ab4e74d9b3430e6 > > > Diff: https://reviews.apache.org/r/65976/diff/3/ > > > Testing > --- > > sudo make check > > > Thanks, > > Chun-Hung Hsiao > >
Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/ --- (Updated April 12, 2018, 3:37 a.m.) Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. Changes --- Rebased. Bugs: MESOS-8492 https://issues.apache.org/jira/browse/MESOS-8492 Repository: mesos Description --- Allowed profiles to be missing from `DiskProfileAdaptor`. Diffs (updated) - src/resource_provider/storage/provider.cpp a07620d1c4bf618f669575b3e183bf598da905a6 src/resource_provider/storage/uri_disk_profile_adaptor.cpp 300ea12687de487737ce91066ab4e74d9b3430e6 Diff: https://reviews.apache.org/r/65976/diff/3/ Changes: https://reviews.apache.org/r/65976/diff/2-3/ Testing --- sudo make check Thanks, Chun-Hung Hsiao
Re: Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/ --- (Updated March 20, 2018, 3:21 a.m.) Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. Changes --- Rebased. Bugs: MESOS-8492 https://issues.apache.org/jira/browse/MESOS-8492 Repository: mesos Description --- Allowed profiles to be missing from `DiskProfileAdaptor`. Diffs (updated) - src/resource_provider/storage/provider.cpp bb19ed4b6b1b8f5f6b327461a737517497c8c38e src/resource_provider/storage/uri_disk_profile_adaptor.cpp 665798fdb085ea34f93bd287fe6f9ab29a265cbf Diff: https://reviews.apache.org/r/65976/diff/2/ Changes: https://reviews.apache.org/r/65976/diff/1-2/ Testing --- sudo make check Thanks, Chun-Hung Hsiao
Review Request 65976: Allowed profiles to be missing from `DiskProfileAdaptor`.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65976/ --- Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu. Bugs: MESOS-8492 https://issues.apache.org/jira/browse/MESOS-8492 Repository: mesos Description --- Allowed profiles to be missing from `DiskProfileAdaptor`. Diffs - src/resource_provider/storage/provider.cpp 63dde512fd8cc9f68f5f48a96869eb09b23b6f4a src/resource_provider/storage/uri_disk_profile.cpp 665798fdb085ea34f93bd287fe6f9ab29a265cbf Diff: https://reviews.apache.org/r/65976/diff/1/ Testing --- sudo make check Thanks, Chun-Hung Hsiao
