> On Feb. 12, 2019, 2 p.m., Benjamin Bannier wrote: > > src/resource_provider/storage/provider.cpp > > Lines 3285-3291 (patched) > > <https://reviews.apache.org/r/69893/diff/2/?file=2124904#file2124904line3287> > > > > I am wondering whether it would make sense to try to remove data from > > as many volumes as possible instead of erroring out on the first failure. > > Not sure as users are left in a weird intermediate state anyway. > > Chun-Hung Hsiao wrote: > Good point, especially since we don't crash the agent or SLRP. Let me do > that.
After an offline discussion, the conclusion is that failures won't result in any data leakage since the PVs will still be there and the framework will have to retry, so stopping early is fine. The volumes will be in an intermediate state that some of the data might have been removed but some not, but it doesn't sound a valid use case to expect a PV to have data after a failed `DESTROY`. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69893/#review212738 ----------------------------------------------------------- On Feb. 12, 2019, 5:19 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69893/ > ----------------------------------------------------------- > > (Updated Feb. 12, 2019, 5:19 a.m.) > > > Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu. > > > Bugs: MESOS-9544 > https://issues.apache.org/jira/browse/MESOS-9544 > > > Repository: mesos > > > Description > ------- > > This patch limits SLRP to only support persistent volumes on MOUNT > disks, and makes it clean up data in persistent volumes when processing > `DESTROY` operations. > > NOTE: Persistent volumes backed by CSI disks that are created before > upgrading to a Mesos version that does not include this fix are subject > to data leakage. To ensure data security, these persistent volume must > be consumed by a task at least once after the upgrade before being > destroyed. > > > Diffs > ----- > > src/resource_provider/storage/provider.cpp > 09a710d668a5a7460b6c4e4fa32d3829dca7ac55 > src/resource_provider/storage/provider_process.hpp > 36187fb4a25c49653530ee286fa9c1663177fbc6 > > > Diff: https://reviews.apache.org/r/69893/diff/2/ > > > Testing > ------- > > `make check` > > More testing done later in chain. > > > Thanks, > > Chun-Hung Hsiao > >
