mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Neil Conway <neil.con...@gmail.com>
Subject Re: Review Request 45121: Implemented deletion for persistent volumes.
Date Mon, 28 Mar 2016 15:57:15 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?
> 
> Jie Yu wrote:
>     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.

True, although for MOUNT disks we might expose data from the previous volume to a newly-created
volume on the same disk. Overall I agree that using `LOG(ERROR)` is probably best for now.


- Neil


-----------------------------------------------------------
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