mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 45121: Implemented deletion for persistent volumes.
Date Mon, 28 Mar 2016 15:48:29 GMT


> On March 26, 2016, 12:37 a.m., Jie Yu wrote:
> > src/slave/slave.cpp, line 2375
> > <https://reviews.apache.org/r/45121/diff/1/?file=1308847#file1308847line2375>
> >
> >     I would suggest we don't use CHECK here. We can just LOG(ERROR) if the deletion
fails. Given the TODO above, we could leak disk space anyway.
> 
> Neil Conway wrote:
>     I used `CHECK_SOME` because we use the same error-handling strategy when creating
the persistent volume fails, or if the slave fails to checkpoint resources to disk successfully.
Are you sure we want to handle `rmdir` errors differently?

We cannot proceed when creating persistent volume fails because the subsequent 'mount' will
fail.

For the deletion case, if it fails, we just leak some data in the volume. The subsequent operation
will not fail. Since we'll be leaking data anyway (the slave crash case), I suggest that we
don't use CHECK_SOME here.


- Jie


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


On March 27, 2016, 6:08 p.m., Neil Conway wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45121/
> -----------------------------------------------------------
> 
> (Updated March 27, 2016, 6:08 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-2408
>     https://issues.apache.org/jira/browse/MESOS-2408
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Prior to this commit, destroying a persistent volume would remove
> the Mesos-level metadata about the volume, but wouldn't destroy
> any of the volume's filesystem content. We now remove the volume
> from the slave's filesystem, essentially via "rm -r".
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp f383605a52f31d7b805ad6153adc409dcb40f83a 
> 
> Diff: https://reviews.apache.org/r/45121/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>


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