----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65594/#review197572 -----------------------------------------------------------
src/resource_provider/storage/provider.cpp Line 1304 (original), 1329 (patched) <https://reviews.apache.org/r/65594/#comment278311> This flag is a bit confusing. When we will set this flag to `true`? Do we need to set it to true when `reconcileStoragePools` is called (currently, it's not)? What if `GetCapacity` reports shrinked capacity? Should we drop `CreateVolume` in that case? src/resource_provider/storage/provider.cpp Lines 2968-2969 (patched) <https://reviews.apache.org/r/65594/#comment278314> Do you need a dispatch here? src/resource_provider/storage/provider.cpp Lines 2970-2971 (patched) <https://reviews.apache.org/r/65594/#comment278315> This is a bit confusing to me initially, and a bit hard to follow. I think the goal here is trying to prevent concurrent reconciliation. However, currently in the code, we used two ways to enforce that: 1) the loop for detecting profile changes (to prevent two reconcileProfiles run concurrently 2) operationSequence to prevent reconcileProfiles and reconcileStoragePool run concurrently Maybe we should use a unified way. src/resource_provider/storage/provider.cpp Lines 3087 (patched) <https://reviews.apache.org/r/65594/#comment278343> Do you want to add some CHECK here to check resource `has_disk`, `has_source` and `has_profile`? - Jie Yu On Feb. 13, 2018, 10:18 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65594/ > ----------------------------------------------------------- > > (Updated Feb. 13, 2018, 10:18 p.m.) > > > Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-8492 > https://issues.apache.org/jira/browse/MESOS-8492 > > > Repository: mesos > > > Description > ------- > > SLRP now checkpoints profiles associated with storage pools, and no > longer assumes that profiles won't be removed by the disk profile > adaptor. It will reject volume/block creation requests that use > missing profiles, but existing volumes/blocks will continue to work. > When a volume/block with a missing profiles is destroyed, it will be > converted to an empty resource, and SLRP will update the sizes of > storage pools for currently known profiles. > > > Diffs > ----- > > src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b > src/resource_provider/storage/provider.cpp > 33abc0e05a804969ae14da9cb9c58698ba1aaea5 > src/resource_provider/storage/uri_disk_profile.cpp > 665798fdb085ea34f93bd287fe6f9ab29a265cbf > > > Diff: https://reviews.apache.org/r/65594/diff/3/ > > > Testing > ------- > > sudo make check > > NOTE: This patch does not change the URI disk profile daptor to allow missing > profiles from upstream. > > > Thanks, > > Chun-Hung Hsiao > >
